Re: [PATCH v2] KVM: Sanitize cpuid

["Roedel, Joerg" <Joerg.Roedel@amd.com>]

On Wed, May 18, 2011 at 05:56:07AM -0400, Avi Kivity wrote:
> Instead of blacklisting known-unsupported cpuid leaves, whitelist known-
> supported leaves.  This is more conservative and prevents us from reporting
> features we don't support.  Also whitelist a few more leaves while at it.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Looks good.

Acked-by: Joerg Roedel <joerg.roedel@amd.com>

> ---
> 
> v2: 0x80000008: fix overflow noted by Joerg; make code less subtle
> 
>  arch/x86/kvm/x86.c |   37 +++++++++++++++++++++++++++++++++++--
>  1 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 77c9d867..abd4d81 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2283,6 +2283,13 @@ static void do_cpuid_1_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  	entry->flags = 0;
>  }
>  
> +static bool supported_xcr0_bit(unsigned bit)
> +{
> +	u64 mask = ((u64)1 << bit);
> +
> +	return mask & (XSTATE_FP | XSTATE_SSE | XSTATE_YMM) & host_xcr0;
> +}
> +
>  #define F(x) bit(X86_FEATURE_##x)
>  
>  static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> @@ -2393,6 +2400,8 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		}
>  		break;
>  	}
> +	case 9:
> +		break;
>  	case 0xb: {
>  		int i, level_type;
>  
> @@ -2414,7 +2423,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  
>  		entry->flags |= KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
>  		for (i = 1; *nent < maxnent && i < 64; ++i) {
> -			if (entry[i].eax == 0)
> +			if (entry[i].eax == 0 || !supported_xcr0_bit(i))
>  				continue;
>  			do_cpuid_1_ent(&entry[i], function, i);
>  			entry[i].flags |=
> @@ -2451,6 +2460,24 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		entry->ecx &= kvm_supported_word6_x86_features;
>  		cpuid_mask(&entry->ecx, 6);
>  		break;
> +	case 0x80000008: {
> +		unsigned g_phys_as = (entry->eax >> 16) & 0xff;
> +		unsigned virt_as = max((entry->eax >> 8) & 0xff, 48U);
> +		unsigned phys_as = entry->eax & 0xff;
> +
> +		if (!g_phys_as)
> +			g_phys_as = phys_as;
> +		entry->eax = g_phys_as | (virt_as << 8);
> +		entry->ebx = entry->edx = 0;
> +		break;
> +	}
> +	case 0x80000019:
> +		entry->ecx = entry->edx = 0;
> +		break;
> +	case 0x8000001a:
> +		break;
> +	case 0x8000001d:
> +		break;
>  	/*Add support for Centaur's CPUID instruction*/
>  	case 0xC0000000:
>  		/*Just support up to 0xC0000004 now*/
> @@ -2460,10 +2487,16 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>  		entry->edx &= kvm_supported_word5_x86_features;
>  		cpuid_mask(&entry->edx, 5);
>  		break;
> +	case 3: /* Processor serial number */
> +	case 5: /* MONITOR/MWAIT */
> +	case 6: /* Thermal management */
> +	case 0xA: /* Architectural Performance Monitoring */
> +	case 0x80000007: /* Advanced power management */
>  	case 0xC0000002:
>  	case 0xC0000003:
>  	case 0xC0000004:
> -		/*Now nothing to do, reserved for the future*/
> +	default:
> +		entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
>  		break;
>  	}
>  
> -- 
> 1.7.5.1
>