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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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>

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?

> +{
> +    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