Re: [Xen-devel] [PATCH 4/9] x86/vmx: Support remote access to the MSR lists

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, May 22, 2018 at 12:20:41PM +0100, Andrew Cooper wrote:
> At the moment, all modifications of the MSR lists are in current context.
> However, future changes may need to put MSR_EFER into the lists from domctl
> hypercall context.
> 
> Plumb a struct vcpu parameter down through the infrastructure, and use
> vmx_vmcs_{enter,exit}() for safe access to the VMCS in vmx_add_msr().  Use
> assertions to ensure that access is either in current context, or while the
> vcpu is paused.
> 
> For now it is safe to require that remote accesses are under the domctl lock.
> This will remain safe if/when the global domctl lock becomes per-domain.

I'm not sure I see the point of this sentence. From my reading of the
above test accesses will always be safe regardless of whether the
domctl lock is global or per-domain?

> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index e4acdc1..8bf54c4 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -1301,13 +1301,15 @@ static struct vmx_msr_entry *locate_msr_entry(
>      return start;
>  }
>  
> -struct vmx_msr_entry *vmx_find_msr(uint32_t msr, enum vmx_msr_list_type type)
> +struct vmx_msr_entry *vmx_find_msr(struct vcpu *v, uint32_t msr,
> +                                   enum vmx_msr_list_type type)
>  {
> -    struct vcpu *curr = current;
> -    struct arch_vmx_struct *arch_vmx = &curr->arch.hvm_vmx;
> +    struct arch_vmx_struct *arch_vmx = &v->arch.hvm_vmx;
>      struct vmx_msr_entry *start = NULL, *ent, *end;
>      unsigned int total;
>  
> +    ASSERT(v == current || !vcpu_runnable(v));

I would rather do:

if ( v != current && vcpu_runnable(v) )
{
    ASSERT_UNREACHABLE();
    return NULL;
}

Thanks, Roger.

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