On 1/29/2026 7:09 AM, Zide Chen wrote: > Detach x86_cpu_pmu_realize() from x86_cpu_realizefn() to keep the latter > focused and easier to follow. Introduce a dedicated helper, > x86_cpu_apply_lbr_pebs_fmt(), in preparation for adding PEBS format > support without duplicating code. > > Convert PERF_CAP_LBR_FMT into separate mask and shift macros to allow > x86_cpu_apply_lbr_pebs_fmt() to be shared with PEBS format handling. > > No functional change intended
Seems a period is missed for above line. Others look good to me. Reviewed-by: Dapeng Mi <[email protected]> > > Signed-off-by: Zide Chen <[email protected]> > --- > V2: > - New patch. > > target/i386/cpu.c | 94 +++++++++++++++++++++++++++++++---------------- > target/i386/cpu.h | 3 +- > 2 files changed, 65 insertions(+), 32 deletions(-) > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c > index 09180c718d58..54f04adb0b48 100644 > --- a/target/i386/cpu.c > +++ b/target/i386/cpu.c > @@ -9781,6 +9781,66 @@ static bool x86_cpu_update_smp_cache_topo(MachineState > *ms, X86CPU *cpu, > } > #endif > > +static bool x86_cpu_apply_lbr_pebs_fmt(X86CPU *cpu, uint64_t host_perf_cap, > + uint64_t user_req, bool is_lbr_fmt, > + Error **errp) > +{ > + CPUX86State *env = &cpu->env; > + uint64_t mask; > + unsigned shift; > + unsigned user_fmt; > + const char *name; > + > + if (is_lbr_fmt) { > + mask = PERF_CAP_LBR_FMT_MASK; > + shift = PERF_CAP_LBR_FMT_SHIFT; > + name = "lbr"; > + } else { > + return false; > + } > + > + if (user_req != -1) { > + env->features[FEAT_PERF_CAPABILITIES] &= ~(mask << shift); > + env->features[FEAT_PERF_CAPABILITIES] |= (user_req << shift); > + } > + > + user_fmt = (env->features[FEAT_PERF_CAPABILITIES] >> shift) & mask; > + > + if (user_fmt) { > + unsigned host_fmt = (host_perf_cap >> shift) & mask; > + > + if (!cpu->enable_pmu) { > + error_setg(errp, "vPMU: %s is unsupported without pmu=on", name); > + return false; > + } > + if (user_fmt != host_fmt) { > + error_setg(errp, "vPMU: the %s-fmt value (0x%x) does not match " > + "the host value (0x%x).", > + name, user_fmt, host_fmt); > + return false; > + } > + } > + > + return true; > +} > + > +static int x86_cpu_pmu_realize(X86CPU *cpu, Error **errp) > +{ > + uint64_t host_perf_cap = > + x86_cpu_get_supported_feature_word(NULL, FEAT_PERF_CAPABILITIES); > + > + /* > + * Override env->features[FEAT_PERF_CAPABILITIES].LBR_FMT > + * with user-provided setting. > + */ > + if (!x86_cpu_apply_lbr_pebs_fmt(cpu, host_perf_cap, > + cpu->lbr_fmt, true, errp)) { > + return -EINVAL; > + } > + > + return 0; > +} > + > static void x86_cpu_realizefn(DeviceState *dev, Error **errp) > { > CPUState *cs = CPU(dev); > @@ -9788,7 +9848,6 @@ static void x86_cpu_realizefn(DeviceState *dev, Error > **errp) > X86CPUClass *xcc = X86_CPU_GET_CLASS(dev); > CPUX86State *env = &cpu->env; > Error *local_err = NULL; > - unsigned guest_fmt; > > if (!kvm_enabled()) > cpu->enable_pmu = false; > @@ -9824,35 +9883,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error > **errp) > goto out; > } > > - /* > - * Override env->features[FEAT_PERF_CAPABILITIES].LBR_FMT > - * with user-provided setting. > - */ > - if (cpu->lbr_fmt != -1) { > - env->features[FEAT_PERF_CAPABILITIES] &= ~PERF_CAP_LBR_FMT; > - env->features[FEAT_PERF_CAPABILITIES] |= cpu->lbr_fmt; > - } > - > - /* > - * vPMU LBR is supported when 1) KVM is enabled 2) Option pmu=on and > - * 3)vPMU LBR format matches that of host setting. > - */ > - guest_fmt = env->features[FEAT_PERF_CAPABILITIES] & PERF_CAP_LBR_FMT; > - if (guest_fmt) { > - uint64_t host_perf_cap = > - x86_cpu_get_supported_feature_word(NULL, FEAT_PERF_CAPABILITIES); > - unsigned host_lbr_fmt = host_perf_cap & PERF_CAP_LBR_FMT; > - > - if (!cpu->enable_pmu) { > - error_setg(errp, "vPMU: LBR is unsupported without pmu=on"); > - return; > - } > - if (guest_fmt != host_lbr_fmt) { > - error_setg(errp, "vPMU: the lbr-fmt value (0x%x) does not match " > - "the host value (0x%x).", > - guest_fmt, host_lbr_fmt); > - return; > - } > + if (x86_cpu_pmu_realize(cpu, errp)) { > + return; > } > > if (x86_cpu_filter_features(cpu, cpu->check_cpuid || > cpu->enforce_cpuid)) { > @@ -10445,7 +10477,7 @@ static const Property x86_cpu_properties[] = { > #endif > DEFINE_PROP_INT32("node-id", X86CPU, node_id, CPU_UNSET_NUMA_NODE_ID), > DEFINE_PROP_BOOL("pmu", X86CPU, enable_pmu, false), > - DEFINE_PROP_UINT64_CHECKMASK("lbr-fmt", X86CPU, lbr_fmt, > PERF_CAP_LBR_FMT), > + DEFINE_PROP_UINT64_CHECKMASK("lbr-fmt", X86CPU, lbr_fmt, > PERF_CAP_LBR_FMT_MASK), > > DEFINE_PROP_UINT32("hv-spinlocks", X86CPU, hyperv_spinlock_attempts, > HYPERV_SPINLOCK_NEVER_NOTIFY), > diff --git a/target/i386/cpu.h b/target/i386/cpu.h > index 3e2222e105bc..aa3c24e0ba13 100644 > --- a/target/i386/cpu.h > +++ b/target/i386/cpu.h > @@ -420,7 +420,8 @@ typedef enum X86Seg { > #define ARCH_CAP_TSX_CTRL_MSR (1<<7) > > #define MSR_IA32_PERF_CAPABILITIES 0x345 > -#define PERF_CAP_LBR_FMT 0x3f > +#define PERF_CAP_LBR_FMT_MASK 0x3f > +#define PERF_CAP_LBR_FMT_SHIFT 0x0 > #define PERF_CAP_FULL_WRITE (1U << 13) > #define PERF_CAP_PEBS_BASELINE (1U << 14) >
