On Tue, Sep 7, 2010 at 11:53 AM, Alexander Graf <ag...@suse.de> 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 <ag...@suse.de>
> ---
> 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?
>
> 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.
> + 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.
>
> - 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().
>
> /* CFGDATA */
> - index = pci_host_data_register_mmio(&controller->pci_state, 0);
> + index = pci_host_data_register_mmio(&s->pci_state, 0);
> if (index < 0)
> - goto free;
> + return -1;
> cpu_register_physical_memory(registers + PCIE500_CFGDATA, 4, index);
>
> index = cpu_register_io_memory(e500_pci_reg_read,
> - e500_pci_reg_write, controller);
> + e500_pci_reg_write, s);
> if (index < 0)
> - goto free;
> + return -1;
> cpu_register_physical_memory(registers + PCIE500_REG_BASE,
> PCIE500_REG_SIZE, index);
> + return 0;
> +}
>
> - /* XXX load/save code not tested. */
> - register_savevm(&d->qdev, "ppce500_pci", ppce500_pci_id++,
> - 1, ppce500_pci_save, ppce500_pci_load, controller);
> +static int e500_host_bridge_initfn(PCIDevice *dev)
> +{
> + pci_config_set_vendor_id(dev->config, PCI_VENDOR_ID_FREESCALE);
> + pci_config_set_device_id(dev->config, PCI_DEVICE_ID_MPC8533E);
> + pci_config_set_class(dev->config, PCI_CLASS_PROCESSOR_POWERPC);
> +
> + return 0;
> +}
> +
> +static PCIDeviceInfo e500_host_bridge_info = {
> + .qdev.name = "e500-host-bridge",
> + .qdev.desc = "Host bridge",
> + .qdev.size = sizeof(PCIDevice),
> + .qdev.no_user = 1,
> + .init = e500_host_bridge_initfn,
> +};
>
> - return controller->pci_state.bus;
> +static SysBusDeviceInfo e500_pcihost_info = {
> + .init = e500_pcihost_initfn,
> + .qdev.name = "e500-pcihost",
> + .qdev.size = sizeof(PPCE500PCIState),
> + .qdev.no_user = 1,
> + .qdev.props = (Property[]) {
> + DEFINE_PROP_UINT64("base_addr", PPCE500PCIState, base_addr, 0),
> + DEFINE_PROP_END_OF_LIST(),
> + }
> +};
>
> -free:
> - printf("%s error\n", __func__);
> - qemu_free(controller);
> - return NULL;
> +static void e500_pci_register(void)
> +{
> + sysbus_register_withprop(&e500_pcihost_info);
> + pci_qdev_register(&e500_host_bridge_info);
> }
> +device_init(e500_pci_register);
> --
> 1.6.0.2
>
>
>
