Re: [Qemu-devel] [QEMU][RFC PATCH 3/6] memory: Add xen memory hook

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/22/2012 11:01 AM, Julien Grall wrote:
> QEMU will now register all memory range (PIO and MMIO) in Xen.
> We distinct two phases in memory registered :
>    - initialization
>    - running
>
> For all range registered during the initialization, QEMU will
> check with XenStore if it is authorized to use them.
> After the initialization, QEMU can register all range. Indeed,
> the new ranges will be for PCI Bar.
>
> Signed-off-by: Julien Grall<julien.grall@citrix.com>
> ---
>   exec.c    |    9 ++++++
>   ioport.c  |   17 ++++++++++++
>   xen-all.c |   83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 109 insertions(+), 0 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 780f63f..42d8c56 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3557,12 +3557,21 @@ static void core_commit(MemoryListener *listener)
>   static void core_region_add(MemoryListener *listener,
>                               MemoryRegionSection *section)
>   {
> +    if (xen_enabled()) {
> +       xen_map_iorange(section->offset_within_address_space,
> +                       section->size, 1);
> +    }
> +
>       cpu_register_physical_memory_log(section, section->readonly);
>   }
>
>   static void core_region_del(MemoryListener *listener,
>                               MemoryRegionSection *section)
>   {
> +    if (xen_enabled()) {
> +       xen_unmap_iorange(section->offset_within_address_space,
> +                       section->size, 1);
> +    }
>   }
>
>   static void core_region_nop(MemoryListener *listener,
> diff --git a/ioport.c b/ioport.c
> index 78a3b89..073ed75 100644
> --- a/ioport.c
> +++ b/ioport.c
> @@ -28,6 +28,7 @@
>   #include "ioport.h"
>   #include "trace.h"
>   #include "memory.h"
> +#include "hw/xen.h"
>
>   /***********************************************************/
>   /* IO Port */
> @@ -155,6 +156,11 @@ int register_ioport_read(pio_addr_t start, int length, int size,
>                        i);
>           ioport_opaque[i] = opaque;
>       }
> +
> +    if (xen_enabled()) {
> +        xen_map_iorange(start, length, 0);
> +    }
> +
>       return 0;
>   }
>
> @@ -175,7 +181,13 @@ int register_ioport_write(pio_addr_t start, int length, int size,
>                        i);
>           ioport_opaque[i] = opaque;
>       }
> +
> +    if (xen_enabled()) {
> +        xen_map_iorange(start, length, 0);
> +    }
> +
>       return 0;
> +
>   }


This is the opposite direction we need to head.

I really don't think this series is the right way to handle things.  I don't 
want to see random hooks throughout QEMU to intercept for APIs affectively 
disabling large chunks of QEMU in the process.

You should look at (1) creating only the devices you want (2) use a clean 
interface to interact with those devices.

That would mean having a Xen specific AddressSpaceOps for ioports or something 
like that.  Not having hooks in areas of code like this.

Regards,

Anthony Liguori