Re: [Xen-devel] [PATCH v4 6/8] ioreq-server: remove p2m entries when server is enabled

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Ian Campbell
> Sent: 07 April 2014 17:01
> To: Paul Durrant
> Cc: xen-devel@xxxxxxxxxxxxx; Ian Jackson; Stefano Stabellini; Jan Beulich
> Subject: Re: [PATCH v4 6/8] ioreq-server: remove p2m entries when server is
> enabled
> 
> On Wed, 2014-04-02 at 16:11 +0100, Paul Durrant wrote:
> > For secondary servers, add a hvm op to enable/disable the server. The
> > server will not accept IO until it is enabled and the act of enabling
> > the server removes its pages from the guest p2m, thus preventing the
> guest
> > from directly mapping the pages and synthesizing ioreqs.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> > Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> >  tools/libxc/xc_domain.c          |   24 ++++++++
> >  tools/libxc/xenctrl.h            |    5 ++
> >  xen/arch/x86/hvm/hvm.c           |  115
> +++++++++++++++++++++++++++++++++++++-
> >  xen/include/asm-x86/hvm/domain.h |    1 +
> >  xen/include/public/hvm/hvm_op.h  |   33 +++++++----
> >  5 files changed, 164 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> > index 8cec171..67829c5 100644
> > --- a/tools/libxc/xc_domain.c
> > +++ b/tools/libxc/xc_domain.c
> > @@ -1459,6 +1459,30 @@ int xc_hvm_destroy_ioreq_server(xc_interface
> *xch,
> >      return rc;
> >  }
> >
> > +int xc_hvm_set_ioreq_server_state(xc_interface *xch,
> > +                                  domid_t domid,
> > +                                  ioservid_t id,
> > +                                  int enabled)
> > +{
> > +    DECLARE_HYPERCALL;
> > +    DECLARE_HYPERCALL_BUFFER(xen_hvm_set_ioreq_server_state_t,
> arg);
> > +    int rc;
> > +
> > +    arg = xc_hypercall_buffer_alloc(xch, arg, sizeof(*arg));
> > +    if ( arg == NULL )
> > +        return -1;
> > +
> > +    hypercall.op     = __HYPERVISOR_hvm_op;
> > +    hypercall.arg[0] = HVMOP_set_ioreq_server_state;
> > +    hypercall.arg[1] = HYPERCALL_BUFFER_AS_ARG(arg);
> > +    arg->domid = domid;
> > +    arg->id = id;
> > +    arg->enabled = enabled;
> > +    rc = do_xen_hypercall(xch, &hypercall);
> > +    xc_hypercall_buffer_free(xch, arg);
> > +    return rc;
> > +}
> 
> Newlines for clarity please.
> 

Sure.

> I'm not really looking at the actual interface here. I'm assuming they
> are a pretty straight exposure of the underlying hypercall and relying
> on the hypervisor guys to agree that is a sane interface.
> > diff --git a/xen/include/public/hvm/hvm_op.h
> b/xen/include/public/hvm/hvm_op.h
> > index c6ceea5..a39290e 100644
> > --- a/xen/include/public/hvm/hvm_op.h
> > +++ b/xen/include/public/hvm/hvm_op.h
> > @@ -296,10 +296,10 @@
> DEFINE_XEN_GUEST_HANDLE(xen_hvm_get_ioreq_server_info_t);
> >
> >  #define HVMOP_map_io_range_to_ioreq_server 19
> >  struct xen_hvm_map_io_range_to_ioreq_server {
> > -    domid_t domid;                  /* IN - domain to be serviced */
> > -    ioservid_t id;                  /* IN - handle from
> HVMOP_register_ioreq_server */
> > -    int is_mmio;                    /* IN - MMIO or port IO? */
> > -    uint64_aligned_t start, end;    /* IN - inclusive start and end of 
> > range */
> > +    domid_t domid;               /* IN - domain to be serviced */
> > +    ioservid_t id;               /* IN - handle from 
> > HVMOP_register_ioreq_server
> */
> > +    int is_mmio;                 /* IN - MMIO or port IO? */
> > +    uint64_aligned_t start, end; /* IN - inclusive start and end of range 
> > */
> 
> There seems to be a lot of gratuitous whitespace changes in this patch.
> Shouldn't most of these be folded into the previous patch which
> introduced things so that happens with the correct indentation?
> 

Indeed they should. Emacs must have been playing tricks on me.

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