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

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Fri, Aug 25, 2017 at 10:46:48AM +0100, Paul Durrant wrote:
> > -----Original Message-----
> > From: Roger Pau Monne
> > > +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 */
> > > +        if ( !gfn_eq(iorp->gfn, INVALID_GFN) )
> > > +            return -EPERM;
> > > +
> > > +        return 0;
> > 
> > Shouldn't this return EEXIST? Page has already been allocated by a
> > previous call AFAICT, and it seems like a possible error/misbehavior
> > to try to do it twice.
> > 
> 
> The checks are there to prevent a caller from trying to mix the legacy and 
> new methods of mapping ioreq server pages so EPERM (i.e. 'operation not 
> permitted') seems like the correct error. I agree that it's not obvious, at 
> this inner level, that I do think this is right. I'm open to debate about 
> this though.

Oh, I was referring to the 'return 0;', not the 'return -EPERM;' (that
looks fine to me).

The 'return 0;' means that the page is already setup (if I understood
this correctly), that's why I was wondering whether it should return
-EEXIST instead.

Roger.

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