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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Paolo Bonzini [mailto:pbonzini@xxxxxxxxxx]
> Sent: 16 October 2014 08:37
> To: Peter Maydell; Paul Durrant
> Cc: QEMU Developers; xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini;
> Michael Tokarev; Stefan Hajnoczi; Stefan Weil; Olaf Hering; Gerd Hoffmann;
> Alexey Kardashevskiy; Alexander Graf
> Subject: Re: [PATCH v3 2/2] Xen: Use the ioreq-server API when available
> 
> Il 15/10/2014 19:30, Peter Maydell ha scritto:
> > On 15 October 2014 11:16, Paul Durrant <paul.durrant@xxxxxxxxxx> 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.
> >
> > This commit message doesn't mention it, but presumably this is
> > all x86-specific given it's in a file which is only used for
> > x86 Xen?
> >
> >> +static void xen_hvm_pre_save(void *opaque)
> >> +{
> >> +    XenIOState *state = opaque;
> >> +
> >> +    /* Stop servicing emulation requests */
> >> +    xen_set_ioreq_server_state(xen_xc, xen_domid, state->ioservid, 0);
> >> +    xen_destroy_ioreq_server(xen_xc, xen_domid, state->ioservid);
> >> +}
> >> +
> >> +static const VMStateDescription vmstate_xen_hvm = {
> >> +    .name = "xen-hvm",
> >> +    .version_id = 4,
> >> +    .minimum_version_id = 4,
> >
> > This is new in upstream so why's it starting at version 4?
> >
> >> +    .pre_save = xen_hvm_pre_save,
> >> +    .fields = (VMStateField[]) {
> >> +        VMSTATE_END_OF_LIST()
> >> +    },
> >
> > A vmstate which doesn't actually save any state? This seems
> > rather suspicious...
> >
> >> @@ -1060,12 +1185,19 @@ int xen_hvm_init(ram_addr_t
> *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
> >>      xen_ram_init(below_4g_mem_size, above_4g_mem_size, ram_size,
> ram_memory);
> >>
> >>
> qemu_add_vm_change_state_handler(xen_hvm_change_state_handler,
> state);
> >> +    vmstate_register(NULL, 0, &vmstate_xen_hvm, state);
> >
> > Is the new use of vmstate_register() really necessary?
> > Usually the state you're saving corresponds to some QOM
> > device whose vmsd field you can use instead.
> 
> In this case, it seems like a job for a vmstate change handler.
> 

I looked at that but it did not seem to give me the right semantic, whereas the 
pre-save callback gave me exactly the right semantic.

  Paul

> Paolo

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel