[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 01/12] e500: add board config options
From: |
Michael Davidsaver |
Subject: |
Re: [Qemu-ppc] [PATCH 01/12] e500: add board config options |
Date: |
Wed, 22 Nov 2017 11:55:04 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 11/21/2017 09:28 PM, David Gibson wrote:
> On Sun, Nov 19, 2017 at 09:24:09PM -0600, Michael Davidsaver wrote:
>> allow board code to skip common NIC and guest image setup
>> and configure decrementor frequency.
>> Existing boards unchanged.
>>
>> Signed-off-by: Michael Davidsaver <address@hidden>
>
> So, it's spelled "decrementer".
>
> Other than that, the patch looks correct. However having a big common
> function for overall init with a pile of ad-hoc configuration
> parameters is usually not a great way to go. I think what we want
> instead is to eliminate ppce500_init(), instead doing the setup logic
> separately in each of the e500 machines. The large common slabs of
> code can be helpers in e500.c, but the overall logic - including most
> of the things controlled by the current params - would be under the
> individual machine's control.
I agree that ppce500_init() is unwieldy, and am willing to spend some
time on cleanup. I'm just not sure what to do.
I can see moving initialization of at least the serial, i2c, gpio, and
possibly MPIC and PCI host bridge into the e500-ccsr class.
I'm not sure what to do with all the device tree construction code in
e500.c, which has a bunch of hard coded register offsets. A comment
here summarizes the problem nicely.
> /* TODO: parameterize */
> #define MPC8544_CCSRBAR_SIZE 0x00100000ULL
> #define MPC8544_MPIC_REGS_OFFSET 0x40000ULL
It seems desirable to avoid having these offsets appear in two different
files, which could allow them to get out of sync.
I had the idea of using an Interface to split up device tree
construction, and was encouraged to find PnvXScomInterfaceClass which
seems be a way of combining device tree construction in a device class.
Is this the way to go?
Some guidance would be appreciated.
Michael
>> ---
>> hw/ppc/e500.c | 8 ++++++--
>> hw/ppc/e500.h | 3 +++
>> hw/ppc/e500plat.c | 1 +
>> hw/ppc/mpc8544ds.c | 1 +
>> 4 files changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
>> index 5cf0dabef3..9e7e1b29c4 100644
>> --- a/hw/ppc/e500.c
>> +++ b/hw/ppc/e500.c
>> @@ -826,7 +826,7 @@ void ppce500_init(MachineState *machine, PPCE500Params
>> *params)
>> env->mpic_iack = params->ccsrbar_base +
>> MPC8544_MPIC_REGS_OFFSET + 0xa0;
>>
>> - ppc_booke_timers_init(cpu, 400000000, PPC_TIMER_E500);
>> + ppc_booke_timers_init(cpu, params->decrementor_freq,
>> PPC_TIMER_E500);
>>
>> /* Register reset handler */
>> if (!i) {
>> @@ -899,7 +899,7 @@ void ppce500_init(MachineState *machine, PPCE500Params
>> *params)
>> if (!pci_bus)
>> printf("couldn't create PCI controller!\n");
>>
>> - if (pci_bus) {
>> + if (pci_bus && !params->tsec_nic) {
>> /* Register network interfaces. */
>> for (i = 0; i < nb_nics; i++) {
>> pci_nic_init_nofail(&nd_table[i], pci_bus, "virtio", NULL);
>> @@ -948,6 +948,10 @@ void ppce500_init(MachineState *machine, PPCE500Params
>> *params)
>> sysbus_mmio_get_region(s, 0));
>> }
>>
>> + if (params->skip_load) {
>> + return;
>> + }
>> +
>> /* Load kernel. */
>> if (machine->kernel_filename) {
>> kernel_base = cur_base;
>> diff --git a/hw/ppc/e500.h b/hw/ppc/e500.h
>> index 70ba1d8f4f..40f72f2de2 100644
>> --- a/hw/ppc/e500.h
>> +++ b/hw/ppc/e500.h
>> @@ -22,6 +22,9 @@ typedef struct PPCE500Params {
>> hwaddr pci_mmio_base;
>> hwaddr pci_mmio_bus_base;
>> hwaddr spin_base;
>> + uint32_t decrementor_freq; /* in Hz */
>> + bool skip_load;
>> + bool tsec_nic;
>> } PPCE500Params;
>>
>> void ppce500_init(MachineState *machine, PPCE500Params *params);
>> diff --git a/hw/ppc/e500plat.c b/hw/ppc/e500plat.c
>> index e59e80fb9e..3d07987bd1 100644
>> --- a/hw/ppc/e500plat.c
>> +++ b/hw/ppc/e500plat.c
>> @@ -47,6 +47,7 @@ static void e500plat_init(MachineState *machine)
>> .pci_mmio_base = 0xC00000000ULL,
>> .pci_mmio_bus_base = 0xE0000000ULL,
>> .spin_base = 0xFEF000000ULL,
>> + .decrementor_freq = 400000000,
>> };
>>
>> /* Older KVM versions don't support EPR which breaks guests when we
>> announce
>> diff --git a/hw/ppc/mpc8544ds.c b/hw/ppc/mpc8544ds.c
>> index 1717953ec7..6d9931c475 100644
>> --- a/hw/ppc/mpc8544ds.c
>> +++ b/hw/ppc/mpc8544ds.c
>> @@ -40,6 +40,7 @@ static void mpc8544ds_init(MachineState *machine)
>> .pci_mmio_bus_base = 0xC0000000ULL,
>> .pci_pio_base = 0xE1000000ULL,
>> .spin_base = 0xEF000000ULL,
>> + .decrementor_freq = 400000000,
>> };
>>
>> if (machine->ram_size > 0xc0000000) {
>
signature.asc
Description: OpenPGP digital signature
- [Qemu-ppc] [PATCH 00/12] Add MVME3100 PPC SBC, Michael Davidsaver, 2017/11/19
- [Qemu-ppc] [PATCH 04/12] e500: additional CCSR registers, Michael Davidsaver, 2017/11/19
- [Qemu-ppc] [PATCH 03/12] e500: note possible bug with host bridge, Michael Davidsaver, 2017/11/19
- [Qemu-ppc] [PATCH 05/12] e500: name openpic and pci host bridge, Michael Davidsaver, 2017/11/19
- [Qemu-ppc] [PATCH 07/12] qtest: add e500_i2c_create(), Michael Davidsaver, 2017/11/19