Re: [Xen-devel] [PATCH 1/2] VMX: fix VMCS race on context-switch paths

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/02/17 10:28, Jan Beulich wrote:
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -552,6 +552,27 @@ static void vmx_load_vmcs(struct vcpu *v
>      local_irq_restore(flags);
>  }
>  
> +void vmx_vmcs_reload(struct vcpu *v)
> +{
> +    /*
> +     * As we're running with interrupts disabled, we can't acquire
> +     * v->arch.hvm_vmx.vmcs_lock here. However, with interrupts disabled
> +     * the VMCS can't be taken away from us anymore if we still own it.
> +     */
> +    ASSERT(!local_irq_is_enabled());
> +    if ( v->arch.hvm_vmx.vmcs_pa == this_cpu(current_vmcs) )
> +        return;
> +    ASSERT(!this_cpu(current_vmcs));
> +
> +    /*
> +     * Wait for the remote side to be done with the VMCS before loading
> +     * it here.
> +     */
> +    while ( v->arch.hvm_vmx.active_cpu != -1 )
> +        cpu_relax();

Doesn't this need a ACCESS_ONCE() read?

While the compiled code (using GCC 4.9) isn't an infinite loop, I am not
aware of anything which prevents a compiler hoisting the comparison out
as being constant.

Otherwise, everything else LGTM.

~Andrew

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