Re: [PATCH] x86: correct CPUID output for out of bounds input

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 24/08/16 16:31, Jan Beulich wrote:
> Another place where we should try to behave like real hardware; see
> the code comments.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -3358,6 +3358,31 @@ void hvm_cpuid(unsigned int input, unsig
>      if ( !edx )
>          edx = &dummy;
>  
> +    if ( input & 0xffff )
> +    {
> +        /*
> +         * Requests beyond the highest supported leaf within a group return
> +         * zero on AMD and the highest basic leaf output on others.
> +         */
> +        unsigned int lvl;
> +
> +        hvm_cpuid(input & 0xffff0000, &lvl, NULL, NULL, NULL);
> +        if ( ((lvl ^ input) >> 16) || input > lvl )

This logic isn't correct.  It doesn't cope in the Intel case when lvl
aliases the upper 16 bits of input, despite input being an unknown group.

When I considered the problem before, the only functioning logic I came
up with was to know that for Intel, input = 0x8000xxxx is the only
special case which doesn't collapse into the highest basic leaf.

> +        {
> +            if ( d->arch.x86_vendor == X86_VENDOR_AMD )
> +            {
> +                *eax = 0;
> +                *ebx = 0;
> +                *ecx = 0;
> +                *edx = 0;
> +                return;
> +            }
> +            if ( input >> 16 )
> +                hvm_cpuid(0, &lvl, NULL, NULL, NULL);

Is this really the right way round?  The AMD method of "reserved always
as zero" is the more sane default to take.

I have looked at the Transmeta and Cyrix CPUID docs, and they are
non-specific as to what reserved means.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel