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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
