[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/4] PPC: Qdev'ify e500 pci
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH 2/4] PPC: Qdev'ify e500 pci |
Date: |
Wed, 8 Sep 2010 17:38:45 +0000 |
On Tue, Sep 7, 2010 at 9:33 PM, Alexander Graf <address@hidden> wrote:
>
> On 07.09.2010, at 20:21, Blue Swirl wrote:
>
>> On Tue, Sep 7, 2010 at 11:53 AM, Alexander Graf <address@hidden> wrote:
>>> The e500 PCI controller isn't qdev'ified yet. This leads to severe issues
>>> when running with -drive.
>>>
>>> To be able to use a virtio disk with an e500 VM, let's convert the PCI
>>> controller over to qdev.
>>>
>>> Signed-off-by: Alexander Graf <address@hidden>
>>> ---
>>> hw/ppce500_pci.c | 106
>>> +++++++++++++++++++++++++++++++++++++-----------------
>>> 1 files changed, 73 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/hw/ppce500_pci.c b/hw/ppce500_pci.c
>>> index 8ac99f2..3fa42d2 100644
>>> --- a/hw/ppce500_pci.c
>>> +++ b/hw/ppce500_pci.c
>>> @@ -73,11 +73,11 @@ struct pci_inbound {
>>> };
>>>
>>> struct PPCE500PCIState {
>>> + PCIHostState pci_state;
>>> struct pci_outbound pob[PPCE500_PCI_NR_POBS];
>>> struct pci_inbound pib[PPCE500_PCI_NR_PIBS];
>>> uint32_t gasket_time;
>>> - PCIHostState pci_state;
>>> - PCIDevice *pci_dev;
>>> + uint64_t base_addr;
>>> };
>>>
>>> typedef struct PPCE500PCIState PPCE500PCIState;
>>> @@ -221,7 +221,7 @@ static void ppce500_pci_save(QEMUFile *f, void *opaque)
>>> PPCE500PCIState *controller = opaque;
>>> int i;
>>>
>>> - pci_device_save(controller->pci_dev, f);
>>> + /* pci_device_save(controller->pci_dev, f); */
>>
>> Why, is loading/saving broken?
>
> It was never tested before and save/restore won't work for this combination
> anyways, because it requires KVM which doesn't implement full save/restore
> yet FWIW.
>
>>
>>>
>>> for (i = 0; i < PPCE500_PCI_NR_POBS; i++) {
>>> qemu_put_be32s(f, &controller->pob[i].potar);
>>> @@ -247,7 +247,7 @@ static int ppce500_pci_load(QEMUFile *f, void *opaque,
>>> int version_id)
>>> if (version_id != 1)
>>> return -EINVAL;
>>>
>>> - pci_device_load(controller->pci_dev, f);
>>> + /* pci_device_load(controller->pci_dev, f); */
>>>
>>> for (i = 0; i < PPCE500_PCI_NR_POBS; i++) {
>>> qemu_get_be32s(f, &controller->pob[i].potar);
>>> @@ -269,55 +269,95 @@ static int ppce500_pci_load(QEMUFile *f, void
>>> *opaque, int version_id)
>>>
>>> PCIBus *ppce500_pci_init(qemu_irq pci_irqs[4], target_phys_addr_t
>>> registers)
>>> {
>>> - PPCE500PCIState *controller;
>>> + DeviceState *dev;
>>> + PCIBus *b;
>>> + PCIHostState *h;
>>> + PPCE500PCIState *s;
>>> PCIDevice *d;
>>> - int index;
>>> static int ppce500_pci_id;
>>>
>>> - controller = qemu_mallocz(sizeof(PPCE500PCIState));
>>> + dev = qdev_create(NULL, "e500-pcihost");
>>> + h = FROM_SYSBUS(PCIHostState, sysbus_from_qdev(dev));
>>> + s = DO_UPCAST(PPCE500PCIState, pci_state, h);
>>> +
>>> + qdev_prop_set_uint64(dev, "base_addr", registers);
>>
>> This property should not be needed. You should simply use
>> sysbus_mmio_map() here. See for example grackle_pci.c.
>
> The base address can be different for different boards. I thought the idea of
> the qdev variables was to enable machine description files one day in which
> case we'll have to pass it?
Yes, but the addresses will be specified by sysbus_mmio_map(), there
is very rarely a need to create a property for the base address.
>>> + b = pci_register_bus(&s->pci_state.busdev.qdev, NULL,
>>> mpc85xx_pci_set_irq,
>>> + mpc85xx_pci_map_irq, pci_irqs, PCI_DEVFN(0x11,
>>> 0), 4);
>>> +
>>> + s->pci_state.bus = b;
>>> + qdev_init_nofail(dev);
>>> + d = pci_create_simple(b, 0, "e500-host-bridge");
>>> +
>>> + /* XXX load/save code not tested. */
>>> + register_savevm(&d->qdev, "ppce500_pci", ppce500_pci_id++,
>>> + 1, ppce500_pci_save, ppce500_pci_load, s);
>>
>> It would be nice if you also converted the device to VMState and vmsd.
>> A reset function would be cool too, if it's needed after Anthony's
>> reset changes.
>
> I agree 100%. Next time I'll touch the code will probably be to convert it to
> VMState too. For now, qdev got me -drive working, so it was the more pressing
> issue :).
>
>>
>>>
>>> - controller->pci_state.bus = pci_register_bus(NULL, "pci",
>>> - mpc85xx_pci_set_irq,
>>> - mpc85xx_pci_map_irq,
>>> - pci_irqs, PCI_DEVFN(0x11,
>>> 0),
>>> - 4);
>>> - d = pci_register_device(controller->pci_state.bus,
>>> - "host bridge", sizeof(PCIDevice),
>>> - 0, NULL, NULL);
>>> + return b;
>>> +}
>>>
>>> - pci_config_set_vendor_id(d->config, PCI_VENDOR_ID_FREESCALE);
>>> - pci_config_set_device_id(d->config, PCI_DEVICE_ID_MPC8533E);
>>> - pci_config_set_class(d->config, PCI_CLASS_PROCESSOR_POWERPC);
>>> +static int e500_pcihost_initfn(SysBusDevice *dev)
>>> +{
>>> + PCIHostState *h;
>>> + PPCE500PCIState *s;
>>> + target_phys_addr_t registers;
>>> + int index;
>>>
>>> - controller->pci_dev = d;
>>> + h = FROM_SYSBUS(PCIHostState, sysbus_from_qdev(dev));
>>> + s = DO_UPCAST(PPCE500PCIState, pci_state, h);
>>> + registers = (target_phys_addr_t)s->base_addr;
>>>
>>> /* CFGADDR */
>>> - index = pci_host_conf_register_mmio(&controller->pci_state, 0);
>>> + index = pci_host_conf_register_mmio(&s->pci_state, 0);
>>> if (index < 0)
>>> - goto free;
>>> + return -1;
>>> cpu_register_physical_memory(registers + PCIE500_CFGADDR, 4, index);
>>
>> Instead of cpu_register_physical_memory(), you should use
>> sysbus_register_mmio().
>
> Oh? Interesting. I'll change that then.
>
>
> Alex
>
>
- [Qemu-devel] [PATCH 4/4] PPC: Make e500 pci byte swap config data, (continued)
- [Qemu-devel] [PATCH 4/4] PPC: Make e500 pci byte swap config data, Alexander Graf, 2010/09/02
- [Qemu-devel] [PATCH 2/4] KVM: PPC: Add level based interrupt logic, Alexander Graf, 2010/09/02
- [Qemu-devel] [PATCH 3/4] PPC: Qdev'ify e500 pci, Alexander Graf, 2010/09/02
- [Qemu-devel] [PATCH 1/4] Fix compile of ivshmem on 32 bit hosts, Alexander Graf, 2010/09/02
- [Qemu-devel] [PULL 0/4] PPC updates, Alexander Graf, 2010/09/07
- [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, Alexander Graf, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, Andreas Färber, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, Alexander Graf, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, Andreas Färber, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, malc, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, Alexander Graf, 2010/09/07
- Re: [Qemu-devel] [PATCH 4/4] PPC: Change PPC maintainer, malc, 2010/09/07
- Re: [Qemu-devel] CoreAudio warnings (was: [PATCH 4/4] PPC: Change PPC maintainer), Andreas Färber, 2010/09/09
- Re: [Qemu-devel] CoreAudio warnings (was: [PATCH 4/4] PPC: Change PPC maintainer), malc, 2010/09/09
Re: [Qemu-devel] [PULL 0/4] PPC updates, Anthony Liguori, 2010/09/08