Re: [Xen-devel] [PATCH v2 2/4] x86/hvm: take a reference on ioreq server emulating domain

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.03.18 at 17:58, <paul.durrant@xxxxxxxxxx> wrote:
> @@ -305,6 +305,7 @@ static int hvm_ioreq_server_add_vcpu(struct 
> hvm_ioreq_server *s,
>                                       bool is_default, struct vcpu *v)
>  {
>      struct hvm_ioreq_vcpu *sv;
> +    domid_t domid;
>      int rc;
>  
>      sv = xzalloc(struct hvm_ioreq_vcpu);
> @@ -315,7 +316,9 @@ static int hvm_ioreq_server_add_vcpu(struct 
> hvm_ioreq_server *s,
>  
>      spin_lock(&s->lock);
>  
> -    rc = alloc_unbound_xen_event_channel(v->domain, v->vcpu_id, s->domid,
> +    domid = s->emulator->domain_id;
> +
> +    rc = alloc_unbound_xen_event_channel(v->domain, v->vcpu_id, domid,
>                                           NULL);
>      if ( rc < 0 )
>          goto fail2;
> @@ -324,9 +327,9 @@ static int hvm_ioreq_server_add_vcpu(struct 
> hvm_ioreq_server *s,
>  
>      if ( v->vcpu_id == 0 && s->bufioreq.va != NULL )
>      {
> -        struct domain *d = s->domain;
> +        struct domain *d = s->target;
>  
> -        rc = alloc_unbound_xen_event_channel(v->domain, 0, s->domid, NULL);
> +        rc = alloc_unbound_xen_event_channel(v->domain, 0, domid, NULL);

Especially for this second call site it would probably be more clear
at the first glance which domain is meant if you didn't latch
s->emulator->domain_id into a local variable.

> @@ -602,12 +605,17 @@ static int hvm_ioreq_server_init(struct 
> hvm_ioreq_server *s,
>                                   struct domain *d, bool is_default,
>                                   int bufioreq_handling, ioservid_t id)
>  {
> +    struct domain *currd = current->domain;
>      struct vcpu *v;
>      int rc;
>  
>      s->id = id;
> -    s->domain = d;
> -    s->domid = current->domain->domain_id;
> +    s->target = d;
> +
> +    if ( !get_domain(currd) )
> +        return -EACCES;

Use get_knownalive_domain() here, which cannot fail?

> @@ -651,6 +660,8 @@ static void hvm_ioreq_server_deinit(struct 
> hvm_ioreq_server *s,
>      hvm_ioreq_server_remove_all_vcpus(s);
>      hvm_ioreq_server_unmap_pages(s, is_default);
>      hvm_ioreq_server_free_rangesets(s, is_default);
> +
> +    put_domain(s->emulator);
>  }

Could you clarify in the description that this runs in the context of
XEN_DOMCTL_destroydomain and hence there's no risk of target
and emulator preventing each other from being cleaned up?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel