Re: [PATCH] KVM: Sanitize cpuid

[Avi Kivity <avi@redhat.com>]

On 05/18/2011 10:19 AM, Roedel, Joerg wrote:
> >   		break;
> >  +	case 0x80000008: {
> >  +		u8 g_phys_as = entry->eax>>  16;
> >  +		u8 virt_as = max(entry->eax>>  8, 48U);
>
> Shouldn't that be 'max((entry->eax>>  8)&  0xff, 48U)'? Seems safer when
> the entry->eax contains a non-zero g_phys value.

Yes, this is a bug.  I originally had 'u8 virt_as = entry->eax >> 8', 
relying on the cast to u8, but missed it when updating.  Moral: don't be 
subtle.

> >  +		u8 phys_as = entry->eax;
> >  +
> >  +		if (!g_phys_as)
> >  +			g_phys_as = phys_as;
> >  +		entry->eax = g_phys_as | (virt_as<<  8);
>
> It is optional, but since we support nesting we can also encode
> g_phys_as in bits 23:16.

I'm relying on a zero value in 23:16 indicating you should use 7:0.

Thanks for the review, will post an updated patch.

-- 
error compiling committee.c: too many arguments to function