1. Patchew
  2. QEMU
  3. [PATCH v3 00/20] i386: Support CET for KVM
  4. View patch

[PATCH v3 08/20] i386/cpu: Drop pmu check in CPUID 0x1C encoding

Zhao Liu posted 20 patches 3 days, 4 hours ago
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>
[PATCH v3 08/20] i386/cpu: Drop pmu check in CPUID 0x1C encoding
Posted by Zhao Liu 3 days, 4 hours ago 
Since CPUID_7_0_EDX_ARCH_LBR will be masked off if pmu is disabled,
there's no need to check CPUID_7_0_EDX_ARCH_LBR feature with pmu.

Tested-by: Farrah Chen <farrah.chen@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
 target/i386/cpu.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 5b7a81fcdb1b..5cd335bb5574 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -8275,11 +8275,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         }
         break;
     }
-    case 0x1C:
-        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
-            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
-            *edx = 0;
+    case 0x1C: /* Last Branch Records Information Leaf */
+        *eax = 0;
+        *ebx = 0;
+        *ecx = 0;
+        *edx = 0;
+        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
+            break;
         }
+        x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
+        *edx = 0; /* EDX is reserved. */
         break;
     case 0x1D: {
         /* AMX TILE, for now hardcoded for Sapphire Rapids*/
-- 
2.34.1
Re: [PATCH v3 08/20] i386/cpu: Drop pmu check in CPUID 0x1C encoding
Posted by Xiaoyao Li 3 hours ago 
On 10/24/2025 2:56 PM, Zhao Liu wrote:
> Since CPUID_7_0_EDX_ARCH_LBR will be masked off if pmu is disabled,
> there's no need to check CPUID_7_0_EDX_ARCH_LBR feature with pmu.
> 
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>

> ---
>   target/i386/cpu.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 5b7a81fcdb1b..5cd335bb5574 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8275,11 +8275,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>           }
>           break;
>       }
> -    case 0x1C:
> -        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> -            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> -            *edx = 0;
> +    case 0x1C: /* Last Branch Records Information Leaf */
> +        *eax = 0;
> +        *ebx = 0;
> +        *ecx = 0;
> +        *edx = 0;

Could you help write a patch to move the initialization-to-0 operation 
out to the switch() handling as the common first handling. So that each 
case doesn't need to set them to 0 individually.

> +        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> +            break;
>           }
> +        x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> +        *edx = 0; /* EDX is reserved. */

Not the fault of this series. I think just presenting what KVM returns 
to guest (i.e., directly passthrough) isn't correct. Once leaf 0x1c gets 
more bits defined and KVM starts to support and report them, then the 
bits presented to guest get changed automatically between different KVM.

the leaf 0x1c needs to be configurable and QEMU needs to ensure the same 
configuration outputs the constant result of leaf 0x1c, to ensure safe 
migration.

It's not urgent though. KVM doesn't even support ArchLBR yet.

>           break;
>       case 0x1D: {
>           /* AMX TILE, for now hardcoded for Sapphire Rapids*/
Re: [PATCH v3 08/20] i386/cpu: Drop pmu check in CPUID 0x1C encoding
Posted by Zhao Liu 40 minutes ago 
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index 5b7a81fcdb1b..5cd335bb5574 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -8275,11 +8275,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> >           }
> >           break;
> >       }
> > -    case 0x1C:
> > -        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> > -            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> > -            *edx = 0;
> > +    case 0x1C: /* Last Branch Records Information Leaf */
> > +        *eax = 0;
> > +        *ebx = 0;
> > +        *ecx = 0;
> > +        *edx = 0;
> 
> Could you help write a patch to move the initialization-to-0 operation out
> to the switch() handling as the common first handling. So that each case
> doesn't need to set them to 0 individually.

Yes, this way could eliminate some redundant code, but explicitly
initializing each leaf currently helps prevent missing something.

Moreover, such cleanup would affect almost all CPUID leaves.
I'm afraid this would make it inconvenient for cherry-picking and
backporting. So the benefits are relatively limited. :-(

> > +        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> > +            break;
> >           }
> > +        x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> > +        *edx = 0; /* EDX is reserved. */
> 
> Not the fault of this series. I think just presenting what KVM returns to
> guest (i.e., directly passthrough) isn't correct. Once leaf 0x1c gets more
> bits defined and KVM starts to support and report them, then the bits
> presented to guest get changed automatically between different KVM.
> 
> the leaf 0x1c needs to be configurable and QEMU needs to ensure the same
> configuration outputs the constant result of leaf 0x1c, to ensure safe
> migration.
> 
> It's not urgent though. KVM doesn't even support ArchLBR yet.

I agree, the feature bits enumeration is necessary. Before KVM (or other
accelerators) supports the arch LBR, there's no need to make too many
logic changes - so in this series I try to not change functionality of
arch lbr as much as possible; Let's wait and see the new arch LBR series
from Zide in future.

Regards,
Zhao
Re: [PATCH v3 08/20] i386/cpu: Drop pmu check in CPUID 0x1C encoding
Posted by Chen, Zide 2 days, 17 hours ago 

On 10/23/2025 11:56 PM, Zhao Liu wrote:
> Since CPUID_7_0_EDX_ARCH_LBR will be masked off if pmu is disabled,
> there's no need to check CPUID_7_0_EDX_ARCH_LBR feature with pmu.
> 
> Tested-by: Farrah Chen <farrah.chen@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Reviewed-by: Zide Chen <zide.chen@intel.com>

> ---
>  target/i386/cpu.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 5b7a81fcdb1b..5cd335bb5574 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -8275,11 +8275,16 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>          }
>          break;
>      }
> -    case 0x1C:
> -        if (cpu->enable_pmu && (env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> -            x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> -            *edx = 0;
> +    case 0x1C: /* Last Branch Records Information Leaf */
> +        *eax = 0;
> +        *ebx = 0;
> +        *ecx = 0;
> +        *edx = 0;
> +        if (!(env->features[FEAT_7_0_EDX] & CPUID_7_0_EDX_ARCH_LBR)) {
> +            break;
>          }
> +        x86_cpu_get_supported_cpuid(0x1C, 0, eax, ebx, ecx, edx);
> +        *edx = 0; /* EDX is reserved. */
>          break;
>      case 0x1D: {
>          /* AMX TILE, for now hardcoded for Sapphire Rapids*/

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.