Re: [PATCH] target/i386: do not filter processor tracing features except on KVM

[Thomas Huth <thuth@redhat.com>]

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>