The processor tracing features in cpu_x86_cpuid() are hardcoded to a set
that should be safe on all processor that support PT virtualization.
But as an additional check, x86_cpu_filter_features() also checks
that the accelerator supports that safe subset, and if not it marks
CPUID_7_0_EBX_INTEL_PT as unavailable.

This check fails on accelerators other than KVM, but it is actually
unnecessary to do it because KVM is the only accelerator that uses the
safe subset.  Everything else just provides nonzero values for CPUID
leaf 0x14 (TCG/HVF because processor tracing is not supported; qtest
because nothing is able to read CPUID anyway).  Restricting the check
to KVM fixes a warning with the qtest accelerator:

    $ qemu-system-x86_64 -display none -cpu max,mmx=off -accel qtest
    qemu-system-x86_64: warning: TCG doesn't support requested feature: CPUID.07H:EBX.intel-pt [bit 25]

The warning also happens in the test-x86-cpuid-compat qtest.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2096
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/cpu.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index bca776e1fe9..7f908236767 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -6412,6 +6412,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             break;
         }
 
+        /*
+         * If these are changed, they should stay in sync with
+         * x86_cpu_filter_features().
+         */
         if (count == 0) {
             *eax = INTEL_PT_MAX_SUBLEAF;
             *ebx = INTEL_PT_MINIMAL_EBX;
@@ -7156,7 +7160,12 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
         mark_unavailable_features(cpu, w, unavailable_features, prefix);
     }
 
-    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
+    /*
+     * Check that KVM actually allows the processor tracing features that
+     * are advertised by cpu_x86_cpuid().  Keep these two in sync.
+     */
+    if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
+        kvm_enabled()) {
         uint32_t eax_0, ebx_0, ecx_0, edx_0_unused;
         uint32_t eax_1, ebx_1, ecx_1_unused, edx_1_unused;
 
-- 
2.43.0

On 21/02/2024 17.29, Paolo Bonzini wrote:
> The processor tracing features in cpu_x86_cpuid() are hardcoded to a set
> that should be safe on all processor that support PT virtualization.
> But as an additional check, x86_cpu_filter_features() also checks
> that the accelerator supports that safe subset, and if not it marks
> CPUID_7_0_EBX_INTEL_PT as unavailable.
> 
> This check fails on accelerators other than KVM, but it is actually
> unnecessary to do it because KVM is the only accelerator that uses the
> safe subset.  Everything else just provides nonzero values for CPUID
> leaf 0x14 (TCG/HVF because processor tracing is not supported; qtest
> because nothing is able to read CPUID anyway).  Restricting the check
> to KVM fixes a warning with the qtest accelerator:
> 
>      $ qemu-system-x86_64 -display none -cpu max,mmx=off -accel qtest
>      qemu-system-x86_64: warning: TCG doesn't support requested feature: CPUID.07H:EBX.intel-pt [bit 25]
> 
> The warning also happens in the test-x86-cpuid-compat qtest.
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2096
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   target/i386/cpu.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index bca776e1fe9..7f908236767 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -6412,6 +6412,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>               break;
>           }
>   
> +        /*
> +         * If these are changed, they should stay in sync with
> +         * x86_cpu_filter_features().
> +         */
>           if (count == 0) {
>               *eax = INTEL_PT_MAX_SUBLEAF;
>               *ebx = INTEL_PT_MINIMAL_EBX;
> @@ -7156,7 +7160,12 @@ static void x86_cpu_filter_features(X86CPU *cpu, bool verbose)
>           mark_unavailable_features(cpu, w, unavailable_features, prefix);
>       }
>   
> -    if (env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) {
> +    /*
> +     * Check that KVM actually allows the processor tracing features that
> +     * are advertised by cpu_x86_cpuid().  Keep these two in sync.
> +     */
> +    if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
> +        kvm_enabled()) {
>           uint32_t eax_0, ebx_0, ecx_0, edx_0_unused;
>           uint32_t eax_1, ebx_1, ecx_1_unused, edx_1_unused;
>   

Fixes: d047402436 ("target/i386: Call accel-agnostic x86_cpu_get_supported_cpuid()")
Reviewed-by: Thomas Huth <thuth@redhat.com>