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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Peter Maydell [mailto:peter.maydell@xxxxxxxxxx]
> Sent: 15 October 2014 18:30
> To: Paul Durrant
> Cc: QEMU Developers; xen-devel@xxxxxxxxxxxxxxxxxxxx; Stefano Stabellini;
> Paolo Bonzini; 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
> 
> 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?
> 

Good point. I was just using the Xen major, but that doesn't make much sense.

> > +    .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...
> 

Not really. The state is actually in Xen and so is saved by the Xen toolstack. 
I need the pre-save hook here because the pages shared between QEMU and Xen 
need re-inserting into the guest before the Xen toolstack saves the memory 
image.

> > @@ -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.
> 

I don't think so. As I said, there is no state to save but there is need for a 
callback before state is saved. Is there another way to achieve that? I could 
not find any 'clean' way to do it.

  Paul

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