Re: [Xen-devel] [PATCH v4 12/12] x86/hvm/ioreq: add a new mappable resource type...

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Sep 07, 2017 at 03:56:16PM +0100, Roger Pau Monné wrote:
> On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> > ... XENMEM_resource_ioreq_server
> > 
> > This patch adds support for a new resource type that can be mapped using
> > the XENMEM_acquire_resource memory op.
> > 
> > If an emulator makes use of this resource type then, instead of mapping
> > gfns, the IOREQ server will allocate pages from the heap. These pages
> > will never be present in the P2M of the guest at any point and so are
> > not vulnerable to any direct attack by the guest. They are only ever
> > accessible by Xen and any domain that has mapping privilege over the
> > guest (which may or may not be limited to the domain running the emulator).
> > 
> > NOTE: Use of the new resource type is not compatible with use of
> >       XEN_DMOP_get_ioreq_server_info unless the XEN_DMOP_no_gfns flag is
> >       set.
> > 
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> > Acked-by: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > ---
> > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > Cc: Tim Deegan <tim@xxxxxxx>
> > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > ---
> >  xen/arch/x86/hvm/ioreq.c        | 123 
> > +++++++++++++++++++++++++++++++++++++++-
> >  xen/arch/x86/mm.c               |  27 +++++++++
> >  xen/include/asm-x86/hvm/ioreq.h |   2 +
> >  xen/include/public/hvm/dm_op.h  |   4 ++
> >  xen/include/public/memory.h     |   3 +
> >  5 files changed, 158 insertions(+), 1 deletion(-)
> > 
> > diff --git a/xen/arch/x86/hvm/ioreq.c b/xen/arch/x86/hvm/ioreq.c
> > index 158bfbba32..f4c06a7a2a 100644
> > --- a/xen/arch/x86/hvm/ioreq.c
> > +++ b/xen/arch/x86/hvm/ioreq.c
> > @@ -250,6 +250,15 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server 
> > *s, bool buf)
> >      struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> >      int rc;
> >  
> > +    if ( iorp->page )
> > +    {
> > +        /* Make sure the page has not been allocated */
> > +        if ( gfn_eq(iorp->gfn, INVALID_GFN) )
> > +            return -EPERM;
> > +
> > +        return 0;
> > +    }
> > +
> >      if ( d->is_dying )
> >          return -EINVAL;
> >  
> > @@ -272,6 +281,57 @@ static int hvm_map_ioreq_gfn(struct hvm_ioreq_server 
> > *s, bool buf)
> >      return rc;
> >  }
> >  
> > +static int hvm_alloc_ioreq_mfn(struct hvm_ioreq_server *s, bool buf)
> > +{
> > +    struct domain *currd = current->domain;
> > +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
> > +
> > +    if ( iorp->page )
> > +    {
> > +        /* Make sure the page has not been mapped */
> 
> Maybe it's just me being slightly lost, but here you imply that
> iorp->gfn being != INVALID_GFN means the page is mapped, while above
> you mention allocated for the same check.
> 
> Is it because gfn has different usages depending on the context?
> 

Haha, I'm not the only one who got confused while reading the patch. ;-)

That's because the "page" can be obtained via two different methods.

"Allocated" means getting it from xen heap, "mapped" means mapping guest
page afaict.

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