Re: [Xen-devel] [PATCH 13/17] vmx: nest: capability reporting MSRs

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

At 10:41 +0100 on 22 Apr (1271932885), Qing He wrote:
> handles VMX capability reporting MSRs.
> Some features are masked so L1 would see a rather
> simple configuration

Would it be better to whitelist features that we know are safely
virtualized? 

> Signed-off-by: Qing He <qing.he@xxxxxxxxx>
> 
> ---
>  arch/x86/hvm/vmx/nest.c        |   94 
> +++++++++++++++++++++++++++++++++++++++++
>  arch/x86/hvm/vmx/vmx.c         |   14 ++++--
>  include/asm-x86/hvm/vmx/nest.h |    5 ++
>  include/asm-x86/hvm/vmx/vmcs.h |    5 ++
>  include/asm-x86/msr-index.h    |    1 
>  5 files changed, 115 insertions(+), 4 deletions(-)
> 
> diff -r 25c338cbc024 -r 0f0e32a70c02 xen/arch/x86/hvm/vmx/nest.c
> --- a/xen/arch/x86/hvm/vmx/nest.c     Thu Apr 22 22:30:09 2010 +0800
> +++ b/xen/arch/x86/hvm/vmx/nest.c     Thu Apr 22 22:30:09 2010 +0800
> @@ -1200,3 +1200,97 @@
>  
>      return bypass_l0;
>  }
> +
> +/*
> + * Capability reporting
> + */
> +int vmx_nest_msr_read_intercept(struct cpu_user_regs *regs, u64 *msr_content)
> +{
> +    u32 eax, edx;
> +    u64 data = 0;
> +    int r = 1;
> +    u32 mask = 0;
> +
> +    if ( !current->domain->arch.hvm_domain.nesting_avail )
> +        return 0;
> +
> +    switch (regs->ecx) {
> +    case MSR_IA32_VMX_BASIC:
> +        rdmsr(regs->ecx, eax, edx);
> +        data = edx;
> +        data = (data & ~0x1fff) | 0x1000;     /* request 4KB for guest VMCS 
> */
> +        data &= ~(1 << 23);                   /* disable TRUE_xxx_CTLS */
> +        data = (data << 32) | VVMCS_REVISION; /* VVMCS revision */
> +        break;
> +    case MSR_IA32_VMX_PINBASED_CTLS:
> +#define REMOVED_PIN_CONTROL_CAP (PIN_BASED_PREEMPT_TIMER)

Did you mean to use this to mask the value below?

> +        rdmsr(regs->ecx, eax, edx);
> +        data = edx;
> +        data = (data << 32) | eax;
> +        break;
> +    case MSR_IA32_VMX_PROCBASED_CTLS:
> +        rdmsr(regs->ecx, eax, edx);
> +#define REMOVED_EXEC_CONTROL_CAP (CPU_BASED_TPR_SHADOW \
> +            | CPU_BASED_ACTIVATE_MSR_BITMAP            \
> +            | CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)
> +        data = edx & ~REMOVED_EXEC_CONTROL_CAP;
> +        data = (data << 32) | eax;
> +        break;
> +    case MSR_IA32_VMX_EXIT_CTLS:
> +        rdmsr(regs->ecx, eax, edx);
> +#define REMOVED_EXIT_CONTROL_CAP (VM_EXIT_SAVE_GUEST_PAT \
> +            | VM_EXIT_LOAD_HOST_PAT                      \
> +            | VM_EXIT_SAVE_GUEST_EFER                    \
> +            | VM_EXIT_LOAD_HOST_EFER                     \
> +            | VM_EXIT_SAVE_PREEMPT_TIMER)
> +        data = edx & ~REMOVED_EXIT_CONTROL_CAP;
> +        data = (data << 32) | eax;
> +        break;
> +    case MSR_IA32_VMX_ENTRY_CTLS:
> +        rdmsr(regs->ecx, eax, edx);
> +#define REMOVED_ENTRY_CONTROL_CAP (VM_ENTRY_LOAD_GUEST_PAT \
> +            | VM_ENTRY_LOAD_GUEST_EFER)
> +        data = edx & ~REMOVED_ENTRY_CONTROL_CAP;
> +        data = (data << 32) | eax;
> +        break;
> +    case MSR_IA32_VMX_PROCBASED_CTLS2:
> +        mask = 0;
> +
> +        rdmsr(regs->ecx, eax, edx);
> +        data = edx & mask;
> +        data = (data << 32) | eax;
> +        break;
> +
> +    /* pass through MSRs */
> +    case IA32_FEATURE_CONTROL_MSR:
> +    case MSR_IA32_VMX_MISC:
> +    case MSR_IA32_VMX_CR0_FIXED0:
> +    case MSR_IA32_VMX_CR0_FIXED1:
> +    case MSR_IA32_VMX_CR4_FIXED0:
> +    case MSR_IA32_VMX_CR4_FIXED1:
> +    case MSR_IA32_VMX_VMCS_ENUM:
> +        rdmsr(regs->ecx, eax, edx);
> +        data = edx;
> +        data = (data << 32) | eax;
> +        gdprintk(XENLOG_INFO,
> +            "nest: pass through VMX cap reporting register, %lx\n",
> +            regs->ecx);
> +        break;
> +    default:
> +        r = 0;
> +        break;
> +    }
> +
> +    if (r == 1)
> +        gdprintk(XENLOG_DEBUG, "nest: intercepted msr access: %lx: %lx\n",
> +            regs->ecx, data);

These debug printks should go. 

> +
> +    *msr_content = data;
> +    return r;
> +}
> +
> +int vmx_nest_msr_write_intercept(struct cpu_user_regs *regs, u64 msr_content)
> +{
> +    /* silently ignore for now */
> +    return 1;
> +}

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel