Re: [Xen-devel] [PATCH v3 4/6] Pause/Unpause the domain before/after assigning PI hooks

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.08.16 at 05:56, <feng.wu@xxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -219,8 +219,19 @@ void vmx_pi_hooks_assign(struct domain *d)
>  
>      ASSERT(!d->arch.hvm_domain.vmx.vcpu_block);
>  
> +    /*
> +     * Pausing the domain can make sure the vCPU is not
> +     * running and hence calling the hooks simultaneously
> +     * when deassigning the PI hooks. This makes sure that
> +     * all the appropriate state of PI descriptor is actually
> +     * set up for all vCPus before leaving this function.
> +     */
> +    domain_pause(d);
> +
>      d->arch.hvm_domain.vmx.vcpu_block = vmx_vcpu_block;
>      d->arch.hvm_domain.vmx.pi_do_resume = vmx_pi_do_resume;
> +
> +    domain_unpause(d);
>  }

First of all I'm missing a word on whether the race mentioned in
the description and comment can actually happen. Device
(de)assignment should already be pretty much serialized (via
the domctl lock, and maybe also via the pcidevs one).

And then - isn't this overkill? Wouldn't a simple spin lock, taken
here and in the deassign counterpart, do?

Or wait - is the comment perhaps wrongly talking about deassign?

If so the change is still questionable, as the hooks get set before
the first device gets actually assigned to a guest (I remember
that I insisted on things getting done that way when those
original patches had been under review).

Jan


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