Re: [Qemu-devel] [QEMU][RFC V2 10/10] xen: emulate IDE outside default device set

[Stefano Stabellini]

On Wed, 22 Aug 2012, Julien Grall wrote:
> IDE can be emulate in a different QEMU that the default.
> 
> This patch also fixes ide_get_geometry. When QEMU didn't emulate IDE,
                                                     ^doesn't
> it try to derefence a NULL bus.
      ^tries to dereference


> Signed-off-by: Julien Grall <address@hidden>
> ---
>  hw/ide/qdev.c |    8 +++++++-
>  hw/pc_piix.c  |   38 ++++++++++++++++++++++++--------------
>  hw/xen.h      |   10 ++++++++++
>  xen-all.c     |    8 +++++++-
>  4 files changed, 48 insertions(+), 16 deletions(-)
> 
> diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
> index 5ea9b8f..473acd7 100644
> --- a/hw/ide/qdev.c
> +++ b/hw/ide/qdev.c
> @@ -115,7 +115,13 @@ IDEDevice *ide_create_drive(IDEBus *bus, int unit, 
> DriveInfo *drive)
>  int ide_get_geometry(BusState *bus, int unit,
>                       int16_t *cyls, int8_t *heads, int8_t *secs)
>  {
> -    IDEState *s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
> +    IDEState *s = NULL;
> +
> +    if (!bus) {
> +        return -1;
> +    }
> +
> +    s = &DO_UPCAST(IDEBus, qbus, bus)->ifs[unit];
>  
>      if (s->drive_kind != IDE_HD || !s->bs) {
>          return -1;
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 6cb0a2a..b904100 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -148,6 +148,7 @@ static void pc_init1(MemoryRegion *system_memory,
>      MemoryRegion *pci_memory;
>      MemoryRegion *rom_memory;
>      void *fw_cfg = NULL;
> +    int register_default_dev = 0;
>  
>      pc_cpus_init(cpu_model);
>  
> @@ -242,23 +243,32 @@ static void pc_init1(MemoryRegion *system_memory,
>              pci_nic_init_nofail(nd, "e1000", NULL);
>      }
>  
> -    ide_drive_get(hd, MAX_IDE_BUS);
> -    if (pci_enabled) {
> -        PCIDevice *dev;
> -        if (xen_enabled()) {
> -            dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
> +    if (!xen_enabled() || xen_is_emulated_ide()) {
> +        xen_set_register_default_dev(0, &register_default_dev);
> +        ide_drive_get(hd, MAX_IDE_BUS);
> +        if (pci_enabled) {
> +            PCIDevice *dev;
> +            if (xen_enabled()) {
> +                dev = pci_piix3_xen_ide_init(pci_bus, hd, piix3_devfn + 1);
> +            } else {
> +                dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
> +            }
> +            idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
> +            idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
>          } else {
> -            dev = pci_piix3_ide_init(pci_bus, hd, piix3_devfn + 1);
> +            for (i = 0; i < MAX_IDE_BUS; i++) {
> +                ISADevice *dev;
> +                dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> +                                   ide_irq[i],
> +                                   hd[MAX_IDE_DEVS * i],
> +                                   hd[MAX_IDE_DEVS * i + 1]);
> +                idebus[i] = qdev_get_child_bus(&dev->qdev, "ide.0");
> +            }
>          }
> -        idebus[0] = qdev_get_child_bus(&dev->qdev, "ide.0");
> -        idebus[1] = qdev_get_child_bus(&dev->qdev, "ide.1");
> +        xen_set_register_default_dev(register_default_dev, NULL);
>      } else {
> -        for(i = 0; i < MAX_IDE_BUS; i++) {
> -            ISADevice *dev;
> -            dev = isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i],
> -                               ide_irq[i],
> -                               hd[MAX_IDE_DEVS * i], hd[MAX_IDE_DEVS * i + 
> 1]);
> -            idebus[i] = qdev_get_child_bus(&dev->qdev, "ide.0");
> +        for (i = 0; i < MAX_IDE_BUS; i++) {
> +            idebus[i] = NULL;
>          }
>      }

I think it would be better to have a non-xen specific option to
enable/disable ide emulation, something like hpet/no_hpet.


> diff --git a/hw/xen.h b/hw/xen.h
> index 3c8724f..3c89fb9 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -22,6 +22,7 @@ extern enum xen_mode xen_mode;
>  
>  extern int xen_allowed;
>  extern int xen_register_default_dev;
> +extern int xen_emulate_ide;
>  
>  static inline int xen_enabled(void)
>  {
> @@ -32,6 +33,15 @@ static inline int xen_enabled(void)
>  #endif
>  }
>  
> +static inline int xen_is_emulated_ide(void)
> +{
> +#if defined(CONFIG_XEN)
> +    return xen_emulate_ide;
> +#else
> +    return 1;
> +#endif
> +}
> +
>  static inline int xen_is_registered_default_dev(void)
>  {
>  #if defined(CONFIG_XEN)
> diff --git a/xen-all.c b/xen-all.c
> index f424cce..f091908 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -43,6 +43,8 @@ static uint32_t xen_dmid = ~0;
>  int xen_register_default_dev = 0;
>  static int xen_emulate_default_dev = 1;
>  
> +int xen_emulate_ide = 0;
> +
>  /* Compatibility with older version */
>  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
>  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
> @@ -1342,6 +1344,8 @@ int xen_hvm_init(void)
>                                         "xen_dmid", ~0);
>          xen_emulate_default_dev = 
> qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
>                                                      "xen_default_dev", 1);
> +        xen_emulate_ide = qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
> +                                            "xen_emulate_ide", 1);
>      }
>  
>      state = g_malloc0(sizeof (XenIOState));
> @@ -1437,12 +1441,14 @@ int xen_hvm_init(void)
>          fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
>          exit(1);
>      }
> -    xen_be_register("qdisk", &xen_blkdev_ops);
>  
>      if (xen_emulate_default_dev) {
>          xen_be_register("console", &xen_console_ops);
>          xen_be_register("vkbd", &xen_kbdmouse_ops);
>      }
> +    if (xen_emulate_ide) {
> +        xen_be_register("qdisk", &xen_blkdev_ops);
> +    }
>      xen_read_physmap(state);
>  
>      return 0;
> -- 
> Julien Grall
>