Re: [PATCH 15/16] vmx: nest: capability reporting MSRs

[Tim Deegan <Tim.Deegan@citrix.com>]

At 16:22 +0100 on 08 Sep (1283962943), Qing He wrote:
> handles VMX capability reporting MSRs.
> Some features are masked so L1 would see a rather
> simple configuration

As I said last time, would it be better to whitelist features that we
know are safely virtualized?

> Signed-off-by: Qing He <qing.he@intel.com>
> Signed-off-by: Eddie Dong <eddie.dong@intel.com>
> 
> ---
> 
> diff -r 694dcf6c3f06 xen/arch/x86/hvm/vmx/nest.c
> --- a/xen/arch/x86/hvm/vmx/nest.c	Wed Sep 08 19:47:14 2010 +0800
> +++ b/xen/arch/x86/hvm/vmx/nest.c	Wed Sep 08 19:47:39 2010 +0800
> @@ -1352,3 +1352,91 @@
>  
>      return bypass_l0;
>  }
> +
> +/*
> + * Capability reporting
> + */
> +int vmx_nest_msr_read_intercept(unsigned int msr, u64 *msr_content)
> +{
> +    u32 eax, edx;
> +    u64 data = 0;
> +    int r = 1;
> +    u32 mask = 0;
> +
> +    if ( !is_nested_avail(current->domain) )
> +        return 0;
> +
> +    switch (msr) {
> +    case MSR_IA32_VMX_BASIC:
> +        rdmsr(msr, eax, edx);
> +        data = edx;
> +        data = (data & ~0x1fff) | 0x1000;     /* request 4KB for guest VMCS */
> +        data &= ~(1 << 23);                   /* disable TRUE_xxx_CTLS */

Magic number - please use a macro to define it. 

> +        data = (data << 32) | VVMCS_REVISION; /* VVMCS revision */
> +        break;
> +    case MSR_IA32_VMX_PINBASED_CTLS:
> +#define REMOVED_PIN_CONTROL_CAP (PIN_BASED_PREEMPT_TIMER)

You define this mask but don't actually mask anything with it.

Cheers, 

Tim.

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