Re: [Qemu-devel] [QEMU][RFC V2 07/10] xen: specify which device is part of default devices

[Stefano Stabellini]

On Wed, 22 Aug 2012, Julien Grall wrote:
> One major problem of QEMU disaggregation is that some devices needs to
> be "emulate" in each QEMU, but only one need to register it in Xen.
> 
> This patch introduces helpers that can be used in QEMU code (for
> instance hw/pc_piix.c) to specify if the device is part of default sets.
> 
> Signed-off-by: Julien Grall <address@hidden>
> ---
>  hw/pc_piix.c |    2 ++
>  hw/xen.h     |   20 ++++++++++++++++++++
>  xen-all.c    |   29 +++++++++++++++++++++++++++--
>  3 files changed, 49 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index 0c0096f..6cb0a2a 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -342,9 +342,11 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
>      if (xen_hvm_init() != 0) {
>          hw_error("xen hardware virtual machine initialisation failed");
>      }
> +    xen_set_register_default_dev(1,  NULL);
>      pc_init_pci_no_kvmclock(ram_size, boot_device,
>                              kernel_filename, kernel_cmdline,
>                              initrd_filename, cpu_model);
> +    xen_set_register_default_dev(0, NULL);
>      xen_vcpu_init();
>  }

Honestly I don't like this interface, I would rather have an explicit
list of "default devices" and then go through them in
xen_register_pcidev and xen_map_iorange.


>  #endif
> diff --git a/hw/xen.h b/hw/xen.h
> index 663731a..3c8724f 100644
> --- a/hw/xen.h
> +++ b/hw/xen.h
> @@ -21,6 +21,7 @@ extern uint32_t xen_domid;
>  extern enum xen_mode xen_mode;
>  
>  extern int xen_allowed;
> +extern int xen_register_default_dev;
>  
>  static inline int xen_enabled(void)
>  {
> @@ -31,6 +32,25 @@ static inline int xen_enabled(void)
>  #endif
>  }
>  
> +static inline int xen_is_registered_default_dev(void)
> +{
> +#if defined(CONFIG_XEN)
> +    return xen_register_default_dev;
> +#else
> +    return 1;
> +#endif
> +}
> +
> +static inline void xen_set_register_default_dev(int val, int *old)
> +{
> +#if defined(CONFIG_XEN)
> +    if (old) {
> +        *old = xen_register_default_dev;
> +    }
> +    xen_register_default_dev = val;
> +#endif
> +}
> +
>  int xen_pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num);
>  void xen_piix3_set_irq(void *opaque, int irq_num, int level);
>  void xen_piix_pci_write_config_client(uint32_t address, uint32_t val, int 
> len);
> diff --git a/xen-all.c b/xen-all.c
> index 485c312..afa9bcc 100644
> --- a/xen-all.c
> +++ b/xen-all.c
> @@ -39,6 +39,10 @@ static MemoryRegion *framebuffer;
>  static unsigned int serverid;
>  static uint32_t xen_dmid = ~0;
>  
> +/* Use to tell if we register pci/mmio/pio of default devices */
> +int xen_register_default_dev = 0;
> +static int xen_emulate_default_dev = 1;
>
>  /* Compatibility with older version */
>  #if __XEN_LATEST_INTERFACE_VERSION__ < 0x0003020a
>  static inline uint32_t xen_vcpu_eport(shared_iopage_t *shared_page, int i)
> @@ -176,6 +180,10 @@ void xen_piix3_set_irq(void *opaque, int irq_num, int 
> level)
>  
>  int xen_register_pcidev(PCIDevice *pci_dev)
>  {
> +    if (xen_register_default_dev && !xen_emulate_default_dev) {
> +        return 0;
> +    }
>
>      DPRINTF("register pci %x:%x.%x %s\n", 0, (pci_dev->devfn >> 3) & 0x1f,
>              pci_dev->devfn & 0x7, pci_dev->name);
>  
> @@ -214,6 +222,18 @@ static void xen_map_iorange(target_phys_addr_t addr, 
> uint64_t size,
>          return;
>      }
>  
> +    /* Handle the registration of all default io range */
> +    if (xen_register_default_dev) {
> +        /* Register ps/2 only if we emulate VGA */
> +        if (!strcmp(name, "i8042-data") || !strcmp(name, "i8042-cmd")) {
> +            if (display_type == DT_NOGRAPHIC) {
> +                return;
> +            }
> +        } else if (!xen_emulate_default_dev && strcmp(name, "serial")) {
> +            return;
> +        }
> +    }

It seems to be that you are acting upon xen_register_default_dev only in
xen_map_iorange and xen_register_pcidev, so it shouldn't be difficult to
have a real list of "default devices" instead.


>      DPRINTF("map %s %s 0x"TARGET_FMT_plx" - 0x"TARGET_FMT_plx"\n",
>              (is_mmio) ? "mmio" : "io", name, addr, addr + size - 1);
>  
> @@ -1300,6 +1320,8 @@ int xen_hvm_init(void)
>      if (!QTAILQ_EMPTY(&list->head)) {
>          xen_dmid = qemu_opt_get_number(QTAILQ_FIRST(&list->head),
>                                         "xen_dmid", ~0);
> +        xen_emulate_default_dev = 
> qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
> +                                                    "xen_default_dev", 1);
>      }
>  
>      state = g_malloc0(sizeof (XenIOState));
> @@ -1395,9 +1417,12 @@ int xen_hvm_init(void)
>          fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
>          exit(1);
>      }
> -    xen_be_register("console", &xen_console_ops);
> -    xen_be_register("vkbd", &xen_kbdmouse_ops);
>      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);
> +    }
>      xen_read_physmap(state);