Re: [PATCH V3 11/13] target/i386: Add pebs-fmt CPU option

["Chen, Zide" <zide.chen@intel.com>]

On 4/22/2026 1:21 AM, Zhao Liu wrote:
> Hi Zide,
> 
> On Wed, Mar 04, 2026 at 10:07:10AM -0800, Zide Chen wrote:
>> Date: Wed, 4 Mar 2026 10:07:10 -0800
>> From: Zide Chen <zide.chen@intel.com>
>> Subject: [PATCH V3 11/13] target/i386: Add pebs-fmt CPU option
>> X-Mailer: git-send-email 2.53.0
>>
>> Similar to lbr-fmt, target/i386 does not support multi-bit CPU
>> properties, so the PEBS record format cannot be exposed as a
>> user-visible CPU feature.
>>
>> Add a pebs-fmt option to allow users to specify the PEBS format via the
>> command line.  Since the PEBS state is part of the vmstate, this option
>> is considered migratable.
>>
>> We do not support PEBS record format 0.  Although it is a valid format
>> on some very old CPUs, it is unlikely to be used in practice.  This
>> allows pebs-fmt=0 to be used to explicitly disable PEBS in the case of
>> migratable=off.
>>
>> If PEBS is not enabled, mark it as unavailable in IA32_MISC_ENABLE and
>> clear the PEBS-related bits in IA32_PERF_CAPABILITIES.
>>
>> If migratable=on on PEBS capable host and pmu is enabled:
>> - PEBS is disabled if pebs-fmt is not specified or pebs-fmt=0.
>> - PEBS is enabled if pebs-fmt is set to the same value as the host.
>>
>> When migratable=off, the behavior is similar, except that omitting
>> the pebs-fmt option does not disable PEBS.
>>
>> Signed-off-by: Zide Chen <zide.chen@intel.com>
> 
> ...
> 
>>      if (user_req != -1) {
>> +        if (!is_lbr_fmt && !(env->features[FEAT_1_EDX] & CPUID_DTS)) {
>> +            error_setg(errp, "vPMU: %s is unsupported without Debug Store", name);
>> +            return false;
>> +        }
>> +
> 
> This is a general code path to check user_req != -1... but
> 
>> diff --git a/target/i386/kvm/kvm-cpu.c b/target/i386/kvm/kvm-cpu.c
>> index 1d0047d037c7..60bf3899852a 100644
>> --- a/target/i386/kvm/kvm-cpu.c
>> +++ b/target/i386/kvm/kvm-cpu.c
>> @@ -231,6 +231,7 @@ static void kvm_cpu_instance_init(CPUState *cs)
>>      }
>>  
>>      cpu->lbr_fmt = -1;
>> +    cpu->pebs_fmt = -1;
> 
> -1 is only set for KVM, so other accelerators have lbr_fmt=0 by default.
> 
> This will cause make check fail:
> 
> stderr:
> qemu-system-x86_64: vPMU: pebs is unsupported without Debug Store
> Broken pipe
> ../tests/qtest/libqtest.c:201: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0)
> 
> (test program exited with status code -6)
> 
> So I think the initialization should be placed in the general code path
> as well (x86_cpu_initfn()) - and move lbr_fmt initialization back to
> general x86 codes.

Yes, the related changes in patch 9/13 should be reverted.

> 
> Thanks,
> Zhao
>