x86/cpuid: Fix APIC bit clearing

[PATCH] x86/cpuid: Fix APIC bit clearing

Posted by fam@euphon.net 3 years, 8 months ago

From: Fam Zheng <famzheng@amazon.com>

The bug is obvious here, other places in this function used
"cpufeat_mask" correctly.

Signed-off-by: Fam Zheng <famzheng@amazon.com>
Fixes: 46df8a65 ("x86/cpuid: Effectively remove pv_cpuid() and hvm_cpuid()")
---
 xen/arch/x86/cpuid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 6a4a787b68..63a03ef1e5 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -1057,7 +1057,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
         {
             /* Fast-forward MSR_APIC_BASE.EN. */
             if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
-                res->d &= ~cpufeat_bit(X86_FEATURE_APIC);
+                res->d &= ~cpufeat_mask(X86_FEATURE_APIC);
 
             /*
              * PSE36 is not supported in shadow mode.  This bit should be
-- 
2.17.1

Re: [PATCH] x86/cpuid: Fix APIC bit clearing

Posted by Roger Pau Monné 3 years, 8 months ago

On Wed, Jul 29, 2020 at 05:33:41PM +0100, fam@euphon.net wrote:
> From: Fam Zheng <famzheng@amazon.com>
> 
> The bug is obvious here, other places in this function used
> "cpufeat_mask" correctly.
> 
> Signed-off-by: Fam Zheng <famzheng@amazon.com>
> Fixes: 46df8a65 ("x86/cpuid: Effectively remove pv_cpuid() and hvm_cpuid()")

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks!

Re: [PATCH] x86/cpuid: Fix APIC bit clearing

Posted by Andrew Cooper 3 years, 8 months ago

On 29/07/2020 17:33, fam@euphon.net wrote:
> From: Fam Zheng <famzheng@amazon.com>
>
> The bug is obvious here, other places in this function used
> "cpufeat_mask" correctly.
>
> Signed-off-by: Fam Zheng <famzheng@amazon.com>
> Fixes: 46df8a65 ("x86/cpuid: Effectively remove pv_cpuid() and hvm_cpuid()")
> ---
>  xen/arch/x86/cpuid.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index 6a4a787b68..63a03ef1e5 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -1057,7 +1057,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>          {
>              /* Fast-forward MSR_APIC_BASE.EN. */
>              if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
> -                res->d &= ~cpufeat_bit(X86_FEATURE_APIC);
> +                res->d &= ~cpufeat_mask(X86_FEATURE_APIC);
>  
>              /*
>               * PSE36 is not supported in shadow mode.  This bit should be

Oops.  Good spot.

However, the Fixes you identify was just code movement.  The bug was
actually introduced in b648feff8ea2c9bff250b4b262704fb100b1f9cf two
years earlier.

I've tweaked the Fixes line and committed.

~Andrew