Re: [Xen-devel] [PATCH v4 11/12] x86/hvm/ioreq: defer mapping gfns until they are actually requsted

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@xxxxxxxxxx]
> Sent: 07 September 2017 13:00
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Ian Jackson <Ian.Jackson@xxxxxxxxxx>;
> Wei Liu <wei.liu2@xxxxxxxxxx>; Andrew Cooper
> <Andrew.Cooper3@xxxxxxxxxx>; George Dunlap
> <George.Dunlap@xxxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>; Konrad
> Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>; Stefano Stabellini
> <sstabellini@xxxxxxxxxx>; Tim (Xen.org) <tim@xxxxxxx>
> Subject: Re: [PATCH v4 11/12] x86/hvm/ioreq: defer mapping gfns until they
> are actually requsted
> 
> On Tue, Sep 05, 2017 at 12:37:15PM +0100, Paul Durrant wrote:
> > A subsequent patch will introduce a new scheme to allow an emulator to
> > map ioreq server pages directly from Xen rather than the guest P2M.
> >
> > This patch lays the groundwork for that change by deferring mapping of
> > gfns until their values are requested by an emulator. To that end, the
> > pad field of the xen_dm_op_get_ioreq_server_info structure is re-
> purposed
> > to a flags field and new flag, XEN_DMOP_no_gfns, defined which modifies
> the
> > behaviour of XEN_DMOP_get_ioreq_server_info to allow the caller to
> avoid
> > requesting the gfn values.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> > ---
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> >
> > v3:
> >  - Updated in response to review comments from Wei and Roger.
> >  - Added a HANDLE_BUFIOREQ macro to make the code neater.
> >  - This patch no longer introduces a security vulnerability since there
> >    is now an explicit limit on the number of ioreq servers that may be
> >    created for any one domain.
> > ---
> >  tools/libs/devicemodel/core.c                   |  8 +++++
> >  tools/libs/devicemodel/include/xendevicemodel.h |  6 ++--
> >  xen/arch/x86/hvm/dm.c                           |  9 ++++--
> >  xen/arch/x86/hvm/ioreq.c                        | 41 
> > +++++++++++++------------
> >  xen/include/asm-x86/hvm/domain.h                |  2 +-
> >  xen/include/public/hvm/dm_op.h                  | 32 +++++++++++--------
> >  6 files changed, 59 insertions(+), 39 deletions(-)
> >
> > diff --git a/tools/libs/devicemodel/core.c b/tools/libs/devicemodel/core.c
> > index fcb260d29b..28958934bf 100644
> > --- a/tools/libs/devicemodel/core.c
> > +++ b/tools/libs/devicemodel/core.c
> > @@ -188,6 +188,14 @@ int xendevicemodel_get_ioreq_server_info(
> >
> >      data->id = id;
> >
> > +    /*
> > +     * If the caller is not requesting gfn values then instruct the
> > +     * hypercall not to retrieve them as this may cause them to be
> > +     * mapped.
> > +     */
> > +    if (!ioreq_gfn && !bufioreq_gfn)
> > +        data->flags |= XEN_DMOP_no_gfns;
> > +
> 
> Sorry for not having noticed this earlier.
> 
> This is a slight change to a stable API. The new functionality is an
> extension of the old. I would suggest you bump the minor number of this
> library as well.
> 

I don't believe there is an API change here. The code always coped with NULL 
being passed, it just wasn't documented. Or is there something else I'm missing?

> The rest looks good to me.

Thanks,

  Paul

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