Re: [Xen-devel] [PATCH 4/6] x86/hvm: Move hvm_funcs.cpuid_intercept() handling into hvm_cpuid()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.11.16 at 13:31, <andrew.cooper3@xxxxxxxxxx> wrote:
> @@ -3676,6 +3671,12 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>              if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
>                  *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
>          }
> +
> +        /* SYSCALL is hidden outside of long mode on Intel. */
> +        if ( d->arch.x86_vendor == X86_VENDOR_INTEL &&
> +             !hvm_long_mode_enabled(v))
> +            *edx &= ~cpufeat_mask(X86_FEATURE_SYSCALL);

Original code also set the bit in its opposite path - don't you think
this should be retained (or otherwise the change be explained in
the description)?

> @@ -3700,6 +3701,14 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, 
> unsigned int *ebx,
>  
>          *ebx &= hvm_featureset[FEATURESET_e8b];
>          break;
> +
> +    case 0x8000001c:
> +        if ( !(v->arch.xcr0 & XSTATE_LWP) )
> +            *eax = 0;
> +        else if ( cpu_has_svm && cpu_has_lwp )
> +            /* Turn on available bit and other features specified in 
> lwp_cfg. */
> +            *eax = (*edx & v->arch.hvm_svm.guest_lwp_cfg) | 1;
> +        break;

Wouldn't it be better to do

+        if ( cpu_has_svm && cpu_has_lwp && (v->arch.xcr0 & XSTATE_LWP) )
+            /* Turn on available bit and other features specified in lwp_cfg. 
*/
+            *eax = (*edx & v->arch.hvm_svm.guest_lwp_cfg) | 1;
+        else
+            *eax = 0;

the more that you're (slightly) altering original behavior anyway?

Jan


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