Re: [Xen-devel] [PATCH v5 3/9] ioreq-server: centralize access to ioreq structures

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 06 May 2014 13:35
> To: Paul Durrant
> Cc: Eddie Dong; Jun Nakajima; Kevin Tian; xen-devel@xxxxxxxxxxxxx; Keir
> (Xen.org)
> Subject: Re: [PATCH v5 3/9] ioreq-server: centralize access to ioreq 
> structures
> 
> >>> On 01.05.14 at 14:08, <paul.durrant@xxxxxxxxxx> wrote:
> > To simplify creation of the ioreq server abstraction in a subsequent patch,
> > this patch centralizes all use of the shared ioreq structure and the
> > buffered ioreq ring to the source module xen/arch/x86/hvm/hvm.c.
> >
> > The patch moves an rmb() from inside hvm_io_assist() to
> hvm_do_resume()
> > because the former may now be passed a data structure on stack, in which
> > case the barrier is unnecessary.
> >
> > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>
> 
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
> 

Thanks.

> with these minor comments:
> 
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -363,6 +363,26 @@ void hvm_migrate_pirqs(struct vcpu *v)
> >      spin_unlock(&d->event_lock);
> >  }
> >
> > +static ioreq_t *get_ioreq(struct vcpu *v)
> 
> const struct vcpu *? Or was it that this conflicts with a subsequent
> patch?
> 

I guess that could be done, although it wasn't const before it was moved.

  Paul

> > +{
> > +    struct domain *d = v->domain;
> > +    shared_iopage_t *p = d->arch.hvm_domain.ioreq.va;
> > +
> > +    ASSERT((v == current) || spin_is_locked(&d-
> >arch.hvm_domain.ioreq.lock));
> > +
> > +    return p ? &p->vcpu_ioreq[v->vcpu_id] : NULL;
> > +}
> > +
> > +bool_t hvm_io_pending(struct vcpu *v)
> > +{
> > +    ioreq_t *p = get_ioreq(v);
> 
> It would be particularly desirable since this one would logically want
> its parameter to be const-qualified.
> 
> > +bool_t hvm_has_dm(struct domain *d)
> > +{
> > +    return !!d->arch.hvm_domain.ioreq.va;
> > +}
> 
> Pretty certainly const here.
> 
> Jan


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