Re: [Qemu-devel] [PATCH v3 2/2] Xen: Use the ioreq-server API when available

[Stefano Stabellini]

On Wed, 15 Oct 2014, Paul Durrant wrote:
> The ioreq-server API added to Xen 4.5 offers better security than
> the existing Xen/QEMU interface because the shared pages that are
> used to pass emulation request/results back and forth are removed
> from the guest's memory space before any requests are serviced.
> This prevents the guest from mapping these pages (they are in a
> well known location) and attempting to attack QEMU by synthesizing
> its own request structures. Hence, this patch modifies configure
> to detect whether the API is available, and adds the necessary
> code to use the API if it is.
> 
> Signed-off-by: Paul Durrant <address@hidden>

The patch is OK, so you can add my Acked-by.
I have a couple of minor comments below. If you need to repost it then
would be nice if you could address them.


> Cc: Stefano Stabellini <address@hidden>
> Cc: Peter Maydell <address@hidden>
> Cc: Paolo Bonzini <address@hidden>
> Cc: Michael Tokarev <address@hidden>
> Cc: Stefan Hajnoczi <address@hidden>
> Cc: Stefan Weil <address@hidden>
> Cc: Olaf Hering <address@hidden>
> Cc: Gerd Hoffmann <address@hidden>
> Cc: Alexey Kardashevskiy <address@hidden>
> Cc: Alexander Graf <address@hidden>
> ---
>  configure                   |   29 ++++++
>  include/hw/xen/xen_common.h |  222 
> +++++++++++++++++++++++++++++++++++++++++++
>  trace-events                |    8 ++
>  xen-hvm.c                   |  174 +++++++++++++++++++++++++++++----
>  4 files changed, 412 insertions(+), 21 deletions(-)
> 

[...]

> diff --git a/xen-hvm.c b/xen-hvm.c
> index 05e522c..0bbbf2a 100644
> --- a/xen-hvm.c
> +++ b/xen-hvm.c
> @@ -62,9 +62,6 @@ static inline ioreq_t *xen_vcpu_ioreq(shared_iopage_t 
> *shared_page, int vcpu)
>  }
>  #  define FMT_ioreq_size "u"
>  #endif
> -#ifndef HVM_PARAM_BUFIOREQ_EVTCHN
> -#define HVM_PARAM_BUFIOREQ_EVTCHN 26
> -#endif
>  
>  #define BUFFER_IO_MAX_DELAY  100
>  
> @@ -78,6 +75,7 @@ typedef struct XenPhysmap {
>  } XenPhysmap;
>  
>  typedef struct XenIOState {
> +    ioservid_t ioservid;
>      shared_iopage_t *shared_page;
>      buffered_iopage_t *buffered_io_page;
>      QEMUTimer *buffered_io_timer;
> @@ -92,6 +90,8 @@ typedef struct XenIOState {
>  
>      struct xs_handle *xenstore;
>      MemoryListener memory_listener;
> +    MemoryListener io_listener;
> +    DeviceListener device_listener;
>      QLIST_HEAD(, XenPhysmap) physmap;
>      hwaddr free_phys_offset;
>      const XenPhysmap *log_for_dirtybit;
> @@ -442,12 +442,23 @@ static void xen_set_memory(struct MemoryListener 
> *listener,
>      bool log_dirty = memory_region_is_logging(section->mr);
>      hvmmem_type_t mem_type;
>  
> +    if (section->mr == &ram_memory) {
> +        return;
> +    } else {
> +        if (add) {
> +            xen_map_memory_section(xen_xc, xen_domid, state->ioservid,
> +                                   section);
> +        } else {
> +            xen_unmap_memory_section(xen_xc, xen_domid, state->ioservid,
> +                                     section);
> +        }
> +    }
>      if (!memory_region_is_ram(section->mr)) {
>          return;
>      }
>  
> -    if (!(section->mr != &ram_memory
> -          && ( (log_dirty && add) || (!log_dirty && !add)))) {
> +    if (!(log_dirty && add) && !(!log_dirty && !add)) {
>          return;

if (!((log_dirty && add) || (!log_dirty && !add)))



>      }
>  
> @@ -480,6 +491,7 @@ static void xen_region_add(MemoryListener *listener,
>                             MemoryRegionSection *section)
>  {
>      memory_region_ref(section->mr);
> +
>      xen_set_memory(listener, section, true);
>  }
>  
> @@ -487,9 +499,54 @@ static void xen_region_del(MemoryListener *listener,
>                             MemoryRegionSection *section)
>  {
>      xen_set_memory(listener, section, false);
> +
>      memory_region_unref(section->mr);
>  }

Useless changes?