Re: [Xen-devel] [PATCH v6 3/9] ioreq-server: create basic ioreq server abstraction.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.14 at 15:23, <paul.durrant@xxxxxxxxxx> wrote:
> @@ -446,16 +443,11 @@ void destroy_ring_for_helper(
>      }
>  }
>  
> -static void hvm_unmap_ioreq_page(
> -    struct domain *d, struct hvm_ioreq_page *iorp)
> +static void hvm_unmap_ioreq_page(struct hvm_ioreq_server *s, bool_t buf)
>  {
> -    spin_lock(&iorp->lock);
> -
> -    ASSERT(d->is_dying);
> +    struct hvm_ioreq_page *iorp = buf ? &s->bufioreq : &s->ioreq;
>  
>      destroy_ring_for_helper(&iorp->va, iorp->page);
> -
> -    spin_unlock(&iorp->lock);
>  }

Are you sure you can do without any locking (i.e. neither here nor in
the caller)? For the respective map function, afaict the locking simply
got moved to the caller. Remember that you need to also protect
yourself against hostile control domains...

> +static int hvm_set_ioreq_pfn(struct domain *d, bool_t buf,
> +                             unsigned long pfn)
> +{
> +    struct hvm_ioreq_server *s = d->arch.hvm_domain.ioreq_server;
> +    int rc;
> +
> +    spin_lock(&s->lock);
> +
> +    rc = hvm_map_ioreq_page(s, buf, pfn);
> +    if ( rc )
> +        goto fail;
> +
> +    if (!buf)

Coding style still...

Jan


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