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

[Paul Durrant <Paul.Durrant@citrix.com>]

> -----Original Message-----
> From: Roger Pau Monne
> Sent: 07 September 2017 16:24
> To: Wei Liu <wei.liu2@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; xen-devel@lists.xenproject.org;
> Stefano Stabellini <sstabellini@kernel.org>; Andrew Cooper
> <Andrew.Cooper3@citrix.com>; Ian Jackson <Ian.Jackson@citrix.com>; Tim
> (Xen.org) <tim@xen.org>; Jan Beulich <jbeulich@suse.com>
> Subject: Re: [Xen-devel] [PATCH v4 12/12] x86/hvm/ioreq: add a new
> mappable resource type...
> 
> On Thu, Sep 07, 2017 at 03:59:05PM +0100, Wei Liu wrote:
> > 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:
> [...]
> > > > +        /* 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.
> 
> Oh, thanks. I think the comment should be slightly expanded then to
> give more context, or else someone else is likely to trip over this
> again IMHO.

Yes, Wei's explanation is correct. I will expand the comments in both places to explain they are mutually exclusive methods of getting hold of ioreq server pages.

  Paul

> 
> Roger.

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