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

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

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: 07 September 2017 15:51
> To: Paul Durrant <Paul.Durrant@citrix.com>
> Cc: xen-devel@lists.xenproject.org; Jan Beulich <jbeulich@suse.com>;
> Andrew Cooper <Andrew.Cooper3@citrix.com>; Ian Jackson
> <Ian.Jackson@citrix.com>; Konrad Rzeszutek Wilk
> <konrad.wilk@oracle.com>; Stefano Stabellini <sstabellini@kernel.org>; Tim
> (Xen.org) <tim@xen.org>; Wei Liu <wei.liu2@citrix.com>
> Subject: Re: [PATCH v4 12/12] x86/hvm/ioreq: add a new mappable resource
> type...
> 
> On Tue, Sep 05, 2017 at 12:37:16PM +0100, Paul Durrant wrote:
> >
> > +mfn_t hvm_get_ioreq_server_frame(struct domain *d, ioservid_t id,
> > +                                 unsigned int idx)
> > +{
> > +    struct hvm_ioreq_server *s;
> > +    mfn_t mfn = INVALID_MFN;
> > +
> > +    spin_lock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> > +
> > +    s = d->arch.hvm_domain.ioreq_server.server[id];
> > +
> 
> Check id < MAX_NR_IOREQ_SERVERS before getting s?

Oh, I should be using my new macro here. Good spot.

> 
> > +    if ( id >= MAX_NR_IOREQ_SERVERS || !s || IS_DEFAULT(s) )
> > +        goto out;
> > +
> > +    if ( hvm_ioreq_server_alloc_pages(s) )
> > +        goto out;
> > +
> > +    if ( idx == 0 )
> > +        mfn = _mfn(page_to_mfn(s->bufioreq.page));
> > +    else if ( idx == 1 )
> > +        mfn = _mfn(page_to_mfn(s->ioreq.page));
> 
> Does the caller care about the order? If so this should be documented?
> 

True. I should document that 0 is the buffered frame and 1+ are synchronous frames (of which there is only one at the moment but this will need to change if we support more vcpus).

> The rest looks sensible but I haven't reviewed in detail.
> 

Ok.

  Paul

> > +
> > + out:
> > +    spin_unlock_recursive(&d->arch.hvm_domain.ioreq_server.lock);
> > +
> > +    return mfn;
> > +}
> > +

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