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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
> bounces@xxxxxxxxxxxxx] On Behalf Of Paul Durrant
> Sent: 06 May 2014 13:41
> To: Jan Beulich
> Cc: Keir (Xen.org); Kevin Tian; Eddie Dong; Jun Nakajima; xen-
> devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH v5 3/9] ioreq-server: centralize access to
> ioreq structures
> 
> > -----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.
> 

Attempting this causes issues when using vcpu_runnable() in a subsequent patch, 
so I won't do it.

  Paul

>   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

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