* [PATCH] perf/x86/intel: use hexidecimal value for cpuid @ 2023-03-08 3:15 Zhenyu Wang 2023-03-08 14:16 ` Peter Zijlstra 2023-03-12 13:26 ` [PATCH v2] perf/x86: " Zhenyu Wang 0 siblings, 2 replies; 8+ messages in thread From: Zhenyu Wang @ 2023-03-08 3:15 UTC (permalink / raw) To: linux-kernel Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kan Liang It's easier to use hexidecimal value instead of decimal for reading and following with SDM doc, also align with other cpuid calls. Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Kan Liang <kan.liang@linux.intel.com> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> --- arch/x86/events/intel/core.c | 2 +- arch/x86/events/intel/lbr.c | 2 +- arch/x86/events/intel/pt.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 14f0a746257d..5af084198c8f 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) * Check whether the Architectural PerfMon supports * Branch Misses Retired hw_event or not. */ - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); + cpuid(0xA, &eax.full, &ebx.full, &fixed_mask, &edx.full); if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) return -ENODEV; diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c index c3b0d15a9841..27048b7f23a0 100644 --- a/arch/x86/events/intel/lbr.c +++ b/arch/x86/events/intel/lbr.c @@ -1506,7 +1506,7 @@ void __init intel_pmu_arch_lbr_init(void) u64 lbr_nr; /* Arch LBR Capabilities */ - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); + cpuid(0x1C, &eax.full, &ebx.full, &ecx.full, &unused_edx); lbr_nr = fls(eax.split.lbr_depth_mask) * 8; if (!lbr_nr) diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c index 42a55794004a..da3c5d748365 100644 --- a/arch/x86/events/intel/pt.c +++ b/arch/x86/events/intel/pt.c @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) } for (i = 0; i < PT_CPUID_LEAVES; i++) { - cpuid_count(20, i, + cpuid_count(0x14, i, &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel: use hexidecimal value for cpuid 2023-03-08 3:15 [PATCH] perf/x86/intel: use hexidecimal value for cpuid Zhenyu Wang @ 2023-03-08 14:16 ` Peter Zijlstra 2023-03-09 1:48 ` Zhenyu Wang 2023-03-12 13:26 ` [PATCH v2] perf/x86: " Zhenyu Wang 1 sibling, 1 reply; 8+ messages in thread From: Peter Zijlstra @ 2023-03-08 14:16 UTC (permalink / raw) To: Zhenyu Wang Cc: linux-kernel, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kan Liang On Wed, Mar 08, 2023 at 11:15:01AM +0800, Zhenyu Wang wrote: > It's easier to use hexidecimal value instead of decimal for reading > and following with SDM doc, also align with other cpuid calls. *shrug*.. > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> > --- > arch/x86/events/intel/core.c | 2 +- > arch/x86/events/intel/lbr.c | 2 +- > arch/x86/events/intel/pt.c | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index 14f0a746257d..5af084198c8f 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) > * Check whether the Architectural PerfMon supports > * Branch Misses Retired hw_event or not. > */ > - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); > + cpuid(0xA, &eax.full, &ebx.full, &fixed_mask, &edx.full); > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) > return -ENODEV; But now the data type names and the cpuid calls are no longer related. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] perf/x86/intel: use hexidecimal value for cpuid 2023-03-08 14:16 ` Peter Zijlstra @ 2023-03-09 1:48 ` Zhenyu Wang 0 siblings, 0 replies; 8+ messages in thread From: Zhenyu Wang @ 2023-03-09 1:48 UTC (permalink / raw) To: Peter Zijlstra Cc: linux-kernel, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kan Liang [-- Attachment #1: Type: text/plain, Size: 1289 bytes --] On 2023.03.08 15:16:19 +0100, Peter Zijlstra wrote: > On Wed, Mar 08, 2023 at 11:15:01AM +0800, Zhenyu Wang wrote: > > It's easier to use hexidecimal value instead of decimal for reading > > and following with SDM doc, also align with other cpuid calls. > > *shrug*.. > > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> > > --- > > arch/x86/events/intel/core.c | 2 +- > > arch/x86/events/intel/lbr.c | 2 +- > > arch/x86/events/intel/pt.c | 2 +- > > 3 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > > index 14f0a746257d..5af084198c8f 100644 > > --- a/arch/x86/events/intel/core.c > > +++ b/arch/x86/events/intel/core.c > > @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) > > * Check whether the Architectural PerfMon supports > > * Branch Misses Retired hw_event or not. > > */ > > - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); > > + cpuid(0xA, &eax.full, &ebx.full, &fixed_mask, &edx.full); > > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) > > return -ENODEV; > > But now the data type names and the cpuid calls are no longer related. oh, right, missed that. How about we change those too e.g union cpuid_0xa_eax? [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] perf/x86: use hexidecimal value for cpuid 2023-03-08 3:15 [PATCH] perf/x86/intel: use hexidecimal value for cpuid Zhenyu Wang 2023-03-08 14:16 ` Peter Zijlstra @ 2023-03-12 13:26 ` Zhenyu Wang 2023-03-22 5:37 ` [PATCH v3] " Zhenyu Wang 1 sibling, 1 reply; 8+ messages in thread From: Zhenyu Wang @ 2023-03-12 13:26 UTC (permalink / raw) To: linux-kernel Cc: kvm, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kan Liang, CodyYao-oc It's easier to use hexidecimal value instead of decimal for reading and following with SDM doc, also align with other cpuid calls. This changes name for both cpuid data type and cpuid calls. Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> --- v2: - rename in cpuid data type as well arch/x86/events/intel/core.c | 10 +++++----- arch/x86/events/intel/lbr.c | 8 ++++---- arch/x86/events/intel/pt.c | 2 +- arch/x86/events/zhaoxin/core.c | 8 ++++---- arch/x86/include/asm/perf_event.h | 12 ++++++------ arch/x86/kvm/cpuid.c | 4 ++-- arch/x86/kvm/vmx/pmu_intel.c | 4 ++-- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index 14f0a746257d..7a1ebe0dd5cc 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -5170,7 +5170,7 @@ static __init void intel_arch_events_quirk(void) static __init void intel_nehalem_quirk(void) { - union cpuid10_ebx ebx; + union cpuid_0xa_ebx ebx; ebx.full = x86_pmu.events_maskl; if (ebx.split.no_branch_misses_retired) { @@ -5878,9 +5878,9 @@ __init int intel_pmu_init(void) struct attribute **td_attr = &empty_attrs; struct attribute **mem_attr = &empty_attrs; struct attribute **tsx_attr = &empty_attrs; - union cpuid10_edx edx; - union cpuid10_eax eax; - union cpuid10_ebx ebx; + union cpuid_0xa_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_ebx ebx; unsigned int fixed_mask; bool pmem = false; int version, i; @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) * Check whether the Architectural PerfMon supports * Branch Misses Retired hw_event or not. */ - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); + cpuid(0xA, &eax.full, &ebx.full, &fixed_mask, &edx.full); if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) return -ENODEV; diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c index c3b0d15a9841..2453a3a4c8bb 100644 --- a/arch/x86/events/intel/lbr.c +++ b/arch/x86/events/intel/lbr.c @@ -1497,16 +1497,16 @@ static bool is_arch_lbr_xsave_available(void) void __init intel_pmu_arch_lbr_init(void) { struct pmu *pmu = x86_get_pmu(smp_processor_id()); - union cpuid28_eax eax; - union cpuid28_ebx ebx; - union cpuid28_ecx ecx; + union cpuid_0x1c_eax eax; + union cpuid_0x1c_ebx ebx; + union cpuid_0x1c_ecx ecx; unsigned int unused_edx; bool arch_lbr_xsave; size_t size; u64 lbr_nr; /* Arch LBR Capabilities */ - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); + cpuid(0x1C, &eax.full, &ebx.full, &ecx.full, &unused_edx); lbr_nr = fls(eax.split.lbr_depth_mask) * 8; if (!lbr_nr) diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c index 42a55794004a..da3c5d748365 100644 --- a/arch/x86/events/intel/pt.c +++ b/arch/x86/events/intel/pt.c @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) } for (i = 0; i < PT_CPUID_LEAVES; i++) { - cpuid_count(20, i, + cpuid_count(0x14, i, &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c index 3e9acdaeed1e..ed02f73ac02d 100644 --- a/arch/x86/events/zhaoxin/core.c +++ b/arch/x86/events/zhaoxin/core.c @@ -504,9 +504,9 @@ static __init void zhaoxin_arch_events_quirk(void) __init int zhaoxin_pmu_init(void) { - union cpuid10_edx edx; - union cpuid10_eax eax; - union cpuid10_ebx ebx; + union cpuid_0xa_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_ebx ebx; struct event_constraint *c; unsigned int unused; int version; @@ -517,7 +517,7 @@ __init int zhaoxin_pmu_init(void) * Check whether the Architectural PerfMon supports * hw_event or not. */ - cpuid(10, &eax.full, &ebx.full, &unused, &edx.full); + cpuid(0xA, &eax.full, &ebx.full, &unused, &edx.full); if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT - 1) return -ENODEV; diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index 6496bdbcac98..c3cd60c2e73b 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -125,7 +125,7 @@ * Intel "Architectural Performance Monitoring" CPUID * detection/enumeration details: */ -union cpuid10_eax { +union cpuid_0xa_eax { struct { unsigned int version_id:8; unsigned int num_counters:8; @@ -135,7 +135,7 @@ union cpuid10_eax { unsigned int full; }; -union cpuid10_ebx { +union cpuid_0xa_ebx { struct { unsigned int no_unhalted_core_cycles:1; unsigned int no_instructions_retired:1; @@ -148,7 +148,7 @@ union cpuid10_ebx { unsigned int full; }; -union cpuid10_edx { +union cpuid_0xa_edx { struct { unsigned int num_counters_fixed:5; unsigned int bit_width_fixed:8; @@ -170,7 +170,7 @@ union cpuid10_edx { /* * Intel Architectural LBR CPUID detection/enumeration details: */ -union cpuid28_eax { +union cpuid_0x1c_eax { struct { /* Supported LBR depth values */ unsigned int lbr_depth_mask:8; @@ -183,7 +183,7 @@ union cpuid28_eax { unsigned int full; }; -union cpuid28_ebx { +union cpuid_0x1c_ebx { struct { /* CPL Filtering Supported */ unsigned int lbr_cpl:1; @@ -195,7 +195,7 @@ union cpuid28_ebx { unsigned int full; }; -union cpuid28_ecx { +union cpuid_0x1c_ecx { struct { /* Mispredict Bit Supported */ unsigned int lbr_mispred:1; diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 596061c1610e..76d9f49ffed3 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -936,8 +936,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) } break; case 0xa: { /* Architectural Performance Monitoring */ - union cpuid10_eax eax; - union cpuid10_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_edx edx; if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) { entry->eax = entry->ebx = entry->ecx = entry->edx = 0; diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index e5cec07ca8d9..34b21e38e33d 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -507,8 +507,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu); struct kvm_cpuid_entry2 *entry; - union cpuid10_eax eax; - union cpuid10_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_edx edx; u64 perf_capabilities; u64 counter_mask; int i; -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3] perf/x86: use hexidecimal value for cpuid 2023-03-12 13:26 ` [PATCH v2] perf/x86: " Zhenyu Wang @ 2023-03-22 5:37 ` Zhenyu Wang 2023-03-24 13:33 ` Liang, Kan 0 siblings, 1 reply; 8+ messages in thread From: Zhenyu Wang @ 2023-03-22 5:37 UTC (permalink / raw) To: linux-kernel Cc: kvm, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, Kan Liang, CodyYao-oc It's easier to use hexidecimal value instead of decimal for reading and following with SDM doc, also align with other cpuid calls. As new cpuid leaf would be added in future, this tries to convert current forms and would take it as convention for new leaf definition. This changes name for both cpuid type and cpuid calls. Cc: Peter Zijlstra <peterz@infradead.org> Cc: Ingo Molnar <mingo@redhat.com> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Namhyung Kim <namhyung@kernel.org> Cc: Kan Liang <kan.liang@linux.intel.com> Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> --- v3: - add more explanation in commit message for purpose of this - use lowercase number in call to align with current code v2: - rename in cpuid data type as well arch/x86/events/intel/core.c | 10 +++++----- arch/x86/events/intel/lbr.c | 8 ++++---- arch/x86/events/intel/pt.c | 2 +- arch/x86/events/zhaoxin/core.c | 8 ++++---- arch/x86/include/asm/perf_event.h | 12 ++++++------ arch/x86/kvm/cpuid.c | 4 ++-- arch/x86/kvm/vmx/pmu_intel.c | 4 ++-- 7 files changed, 24 insertions(+), 24 deletions(-) diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c index a3fb996a86a1..5487a39d4975 100644 --- a/arch/x86/events/intel/core.c +++ b/arch/x86/events/intel/core.c @@ -5170,7 +5170,7 @@ static __init void intel_arch_events_quirk(void) static __init void intel_nehalem_quirk(void) { - union cpuid10_ebx ebx; + union cpuid_0xa_ebx ebx; ebx.full = x86_pmu.events_maskl; if (ebx.split.no_branch_misses_retired) { @@ -5878,9 +5878,9 @@ __init int intel_pmu_init(void) struct attribute **td_attr = &empty_attrs; struct attribute **mem_attr = &empty_attrs; struct attribute **tsx_attr = &empty_attrs; - union cpuid10_edx edx; - union cpuid10_eax eax; - union cpuid10_ebx ebx; + union cpuid_0xa_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_ebx ebx; unsigned int fixed_mask; bool pmem = false; int version, i; @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) * Check whether the Architectural PerfMon supports * Branch Misses Retired hw_event or not. */ - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); + cpuid(0xa, &eax.full, &ebx.full, &fixed_mask, &edx.full); if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) return -ENODEV; diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c index c3b0d15a9841..616a6904af03 100644 --- a/arch/x86/events/intel/lbr.c +++ b/arch/x86/events/intel/lbr.c @@ -1497,16 +1497,16 @@ static bool is_arch_lbr_xsave_available(void) void __init intel_pmu_arch_lbr_init(void) { struct pmu *pmu = x86_get_pmu(smp_processor_id()); - union cpuid28_eax eax; - union cpuid28_ebx ebx; - union cpuid28_ecx ecx; + union cpuid_0x1c_eax eax; + union cpuid_0x1c_ebx ebx; + union cpuid_0x1c_ecx ecx; unsigned int unused_edx; bool arch_lbr_xsave; size_t size; u64 lbr_nr; /* Arch LBR Capabilities */ - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); + cpuid(0x1c, &eax.full, &ebx.full, &ecx.full, &unused_edx); lbr_nr = fls(eax.split.lbr_depth_mask) * 8; if (!lbr_nr) diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c index 42a55794004a..da3c5d748365 100644 --- a/arch/x86/events/intel/pt.c +++ b/arch/x86/events/intel/pt.c @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) } for (i = 0; i < PT_CPUID_LEAVES; i++) { - cpuid_count(20, i, + cpuid_count(0x14, i, &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c index 3e9acdaeed1e..1d071974f4db 100644 --- a/arch/x86/events/zhaoxin/core.c +++ b/arch/x86/events/zhaoxin/core.c @@ -504,9 +504,9 @@ static __init void zhaoxin_arch_events_quirk(void) __init int zhaoxin_pmu_init(void) { - union cpuid10_edx edx; - union cpuid10_eax eax; - union cpuid10_ebx ebx; + union cpuid_0xa_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_ebx ebx; struct event_constraint *c; unsigned int unused; int version; @@ -517,7 +517,7 @@ __init int zhaoxin_pmu_init(void) * Check whether the Architectural PerfMon supports * hw_event or not. */ - cpuid(10, &eax.full, &ebx.full, &unused, &edx.full); + cpuid(0xa, &eax.full, &ebx.full, &unused, &edx.full); if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT - 1) return -ENODEV; diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h index 8fc15ed5e60b..0d2d735c8167 100644 --- a/arch/x86/include/asm/perf_event.h +++ b/arch/x86/include/asm/perf_event.h @@ -125,7 +125,7 @@ * Intel "Architectural Performance Monitoring" CPUID * detection/enumeration details: */ -union cpuid10_eax { +union cpuid_0xa_eax { struct { unsigned int version_id:8; unsigned int num_counters:8; @@ -135,7 +135,7 @@ union cpuid10_eax { unsigned int full; }; -union cpuid10_ebx { +union cpuid_0xa_ebx { struct { unsigned int no_unhalted_core_cycles:1; unsigned int no_instructions_retired:1; @@ -148,7 +148,7 @@ union cpuid10_ebx { unsigned int full; }; -union cpuid10_edx { +union cpuid_0xa_edx { struct { unsigned int num_counters_fixed:5; unsigned int bit_width_fixed:8; @@ -170,7 +170,7 @@ union cpuid10_edx { /* * Intel Architectural LBR CPUID detection/enumeration details: */ -union cpuid28_eax { +union cpuid_0x1c_eax { struct { /* Supported LBR depth values */ unsigned int lbr_depth_mask:8; @@ -183,7 +183,7 @@ union cpuid28_eax { unsigned int full; }; -union cpuid28_ebx { +union cpuid_0x1c_ebx { struct { /* CPL Filtering Supported */ unsigned int lbr_cpl:1; @@ -195,7 +195,7 @@ union cpuid28_ebx { unsigned int full; }; -union cpuid28_ecx { +union cpuid_0x1c_ecx { struct { /* Mispredict Bit Supported */ unsigned int lbr_mispred:1; diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 599aebec2d52..57f43dc87538 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -967,8 +967,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) } break; case 0xa: { /* Architectural Performance Monitoring */ - union cpuid10_eax eax; - union cpuid10_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_edx edx; if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) { entry->eax = entry->ebx = entry->ecx = entry->edx = 0; diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c index e8a3be0b9df9..f4b165667ca9 100644 --- a/arch/x86/kvm/vmx/pmu_intel.c +++ b/arch/x86/kvm/vmx/pmu_intel.c @@ -512,8 +512,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu); struct kvm_cpuid_entry2 *entry; - union cpuid10_eax eax; - union cpuid10_edx edx; + union cpuid_0xa_eax eax; + union cpuid_0xa_edx edx; u64 perf_capabilities; u64 counter_mask; int i; -- 2.39.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] perf/x86: use hexidecimal value for cpuid 2023-03-22 5:37 ` [PATCH v3] " Zhenyu Wang @ 2023-03-24 13:33 ` Liang, Kan 2023-03-31 0:43 ` Zhenyu Wang 0 siblings, 1 reply; 8+ messages in thread From: Liang, Kan @ 2023-03-24 13:33 UTC (permalink / raw) To: Zhenyu Wang, linux-kernel Cc: kvm, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, CodyYao-oc On 2023-03-22 1:37 a.m., Zhenyu Wang wrote: > It's easier to use hexidecimal value instead of decimal for reading > and following with SDM doc, also align with other cpuid calls. As new > cpuid leaf would be added in future, this tries to convert current > forms and would take it as convention for new leaf definition. > > This changes name for both cpuid type and cpuid calls. It seems the patch touches 3 CPUIDs, 0xa, 0x1c and 0x14, right? The patch also crosses several different subsystems and drivers. I think it may be better to divide the patch by CPUID. Each patch to handle one CPUID. It's easier for review. Also, can you please add a macro for the CPUID leaf number? Please refer ARCH_PERFMON_EXT_LEAF (0x23). Thanks, Kan > > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > Cc: Jiri Olsa <jolsa@kernel.org> > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Kan Liang <kan.liang@linux.intel.com> > Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com> > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> > --- > v3: > - add more explanation in commit message for purpose of this > - use lowercase number in call to align with current code > > v2: > - rename in cpuid data type as well > > arch/x86/events/intel/core.c | 10 +++++----- > arch/x86/events/intel/lbr.c | 8 ++++---- > arch/x86/events/intel/pt.c | 2 +- > arch/x86/events/zhaoxin/core.c | 8 ++++---- > arch/x86/include/asm/perf_event.h | 12 ++++++------ > arch/x86/kvm/cpuid.c | 4 ++-- > arch/x86/kvm/vmx/pmu_intel.c | 4 ++-- > 7 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > index a3fb996a86a1..5487a39d4975 100644 > --- a/arch/x86/events/intel/core.c > +++ b/arch/x86/events/intel/core.c > @@ -5170,7 +5170,7 @@ static __init void intel_arch_events_quirk(void) > > static __init void intel_nehalem_quirk(void) > { > - union cpuid10_ebx ebx; > + union cpuid_0xa_ebx ebx; > > ebx.full = x86_pmu.events_maskl; > if (ebx.split.no_branch_misses_retired) { > @@ -5878,9 +5878,9 @@ __init int intel_pmu_init(void) > struct attribute **td_attr = &empty_attrs; > struct attribute **mem_attr = &empty_attrs; > struct attribute **tsx_attr = &empty_attrs; > - union cpuid10_edx edx; > - union cpuid10_eax eax; > - union cpuid10_ebx ebx; > + union cpuid_0xa_edx edx; > + union cpuid_0xa_eax eax; > + union cpuid_0xa_ebx ebx; > unsigned int fixed_mask; > bool pmem = false; > int version, i; > @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) > * Check whether the Architectural PerfMon supports > * Branch Misses Retired hw_event or not. > */ > - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); > + cpuid(0xa, &eax.full, &ebx.full, &fixed_mask, &edx.full); > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) > return -ENODEV; > > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c > index c3b0d15a9841..616a6904af03 100644 > --- a/arch/x86/events/intel/lbr.c > +++ b/arch/x86/events/intel/lbr.c > @@ -1497,16 +1497,16 @@ static bool is_arch_lbr_xsave_available(void) > void __init intel_pmu_arch_lbr_init(void) > { > struct pmu *pmu = x86_get_pmu(smp_processor_id()); > - union cpuid28_eax eax; > - union cpuid28_ebx ebx; > - union cpuid28_ecx ecx; > + union cpuid_0x1c_eax eax; > + union cpuid_0x1c_ebx ebx; > + union cpuid_0x1c_ecx ecx; > unsigned int unused_edx; > bool arch_lbr_xsave; > size_t size; > u64 lbr_nr; > > /* Arch LBR Capabilities */ > - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); > + cpuid(0x1c, &eax.full, &ebx.full, &ecx.full, &unused_edx); > > lbr_nr = fls(eax.split.lbr_depth_mask) * 8; > if (!lbr_nr) > diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c > index 42a55794004a..da3c5d748365 100644 > --- a/arch/x86/events/intel/pt.c > +++ b/arch/x86/events/intel/pt.c > @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) > } > > for (i = 0; i < PT_CPUID_LEAVES; i++) { > - cpuid_count(20, i, > + cpuid_count(0x14, i, > &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], > &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], > &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], > diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c > index 3e9acdaeed1e..1d071974f4db 100644 > --- a/arch/x86/events/zhaoxin/core.c > +++ b/arch/x86/events/zhaoxin/core.c > @@ -504,9 +504,9 @@ static __init void zhaoxin_arch_events_quirk(void) > > __init int zhaoxin_pmu_init(void) > { > - union cpuid10_edx edx; > - union cpuid10_eax eax; > - union cpuid10_ebx ebx; > + union cpuid_0xa_edx edx; > + union cpuid_0xa_eax eax; > + union cpuid_0xa_ebx ebx; > struct event_constraint *c; > unsigned int unused; > int version; > @@ -517,7 +517,7 @@ __init int zhaoxin_pmu_init(void) > * Check whether the Architectural PerfMon supports > * hw_event or not. > */ > - cpuid(10, &eax.full, &ebx.full, &unused, &edx.full); > + cpuid(0xa, &eax.full, &ebx.full, &unused, &edx.full); > > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT - 1) > return -ENODEV; > diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h > index 8fc15ed5e60b..0d2d735c8167 100644 > --- a/arch/x86/include/asm/perf_event.h > +++ b/arch/x86/include/asm/perf_event.h > @@ -125,7 +125,7 @@ > * Intel "Architectural Performance Monitoring" CPUID > * detection/enumeration details: > */ > -union cpuid10_eax { > +union cpuid_0xa_eax { > struct { > unsigned int version_id:8; > unsigned int num_counters:8; > @@ -135,7 +135,7 @@ union cpuid10_eax { > unsigned int full; > }; > > -union cpuid10_ebx { > +union cpuid_0xa_ebx { > struct { > unsigned int no_unhalted_core_cycles:1; > unsigned int no_instructions_retired:1; > @@ -148,7 +148,7 @@ union cpuid10_ebx { > unsigned int full; > }; > > -union cpuid10_edx { > +union cpuid_0xa_edx { > struct { > unsigned int num_counters_fixed:5; > unsigned int bit_width_fixed:8; > @@ -170,7 +170,7 @@ union cpuid10_edx { > /* > * Intel Architectural LBR CPUID detection/enumeration details: > */ > -union cpuid28_eax { > +union cpuid_0x1c_eax { > struct { > /* Supported LBR depth values */ > unsigned int lbr_depth_mask:8; > @@ -183,7 +183,7 @@ union cpuid28_eax { > unsigned int full; > }; > > -union cpuid28_ebx { > +union cpuid_0x1c_ebx { > struct { > /* CPL Filtering Supported */ > unsigned int lbr_cpl:1; > @@ -195,7 +195,7 @@ union cpuid28_ebx { > unsigned int full; > }; > > -union cpuid28_ecx { > +union cpuid_0x1c_ecx { > struct { > /* Mispredict Bit Supported */ > unsigned int lbr_mispred:1; > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 599aebec2d52..57f43dc87538 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -967,8 +967,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > } > break; > case 0xa: { /* Architectural Performance Monitoring */ > - union cpuid10_eax eax; > - union cpuid10_edx edx; > + union cpuid_0xa_eax eax; > + union cpuid_0xa_edx edx; > > if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) { > entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > index e8a3be0b9df9..f4b165667ca9 100644 > --- a/arch/x86/kvm/vmx/pmu_intel.c > +++ b/arch/x86/kvm/vmx/pmu_intel.c > @@ -512,8 +512,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) > struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); > struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu); > struct kvm_cpuid_entry2 *entry; > - union cpuid10_eax eax; > - union cpuid10_edx edx; > + union cpuid_0xa_eax eax; > + union cpuid_0xa_edx edx; > u64 perf_capabilities; > u64 counter_mask; > int i; ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] perf/x86: use hexidecimal value for cpuid 2023-03-24 13:33 ` Liang, Kan @ 2023-03-31 0:43 ` Zhenyu Wang 2023-03-31 13:26 ` Liang, Kan 0 siblings, 1 reply; 8+ messages in thread From: Zhenyu Wang @ 2023-03-31 0:43 UTC (permalink / raw) To: Liang, Kan Cc: linux-kernel, kvm, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, CodyYao-oc [-- Attachment #1: Type: text/plain, Size: 9267 bytes --] On 2023.03.24 09:33:19 -0400, Liang, Kan wrote: > sorry that I missed this one in my inbox... > On 2023-03-22 1:37 a.m., Zhenyu Wang wrote: > > It's easier to use hexidecimal value instead of decimal for reading > > and following with SDM doc, also align with other cpuid calls. As new > > cpuid leaf would be added in future, this tries to convert current > > forms and would take it as convention for new leaf definition. > > > > This changes name for both cpuid type and cpuid calls. > > It seems the patch touches 3 CPUIDs, 0xa, 0x1c and 0x14, right? > The patch also crosses several different subsystems and drivers. > I think it may be better to divide the patch by CPUID. Each patch to > handle one CPUID. It's easier for review. ok, I can do that. > > Also, can you please add a macro for the CPUID leaf number? > Please refer ARCH_PERFMON_EXT_LEAF (0x23). > As originally the purpose of this change is to use hex value in cpuid call and struct name, good to see that use for 0x23. If define every macro for these, e.g ARCH_PERFMON_LEAF(0xa), PT_LEAF(0x14), LBR_LEAF(0x1c), struct name needs change too? As in context of what source file you're reading, you already get idea what these cpuid numbers are for what kind of leaf... > > > > > Cc: Peter Zijlstra <peterz@infradead.org> > > Cc: Ingo Molnar <mingo@redhat.com> > > Cc: Arnaldo Carvalho de Melo <acme@kernel.org> > > Cc: Mark Rutland <mark.rutland@arm.com> > > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> > > Cc: Jiri Olsa <jolsa@kernel.org> > > Cc: Namhyung Kim <namhyung@kernel.org> > > Cc: Kan Liang <kan.liang@linux.intel.com> > > Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com> > > Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> > > --- > > v3: > > - add more explanation in commit message for purpose of this > > - use lowercase number in call to align with current code > > > > v2: > > - rename in cpuid data type as well > > > > arch/x86/events/intel/core.c | 10 +++++----- > > arch/x86/events/intel/lbr.c | 8 ++++---- > > arch/x86/events/intel/pt.c | 2 +- > > arch/x86/events/zhaoxin/core.c | 8 ++++---- > > arch/x86/include/asm/perf_event.h | 12 ++++++------ > > arch/x86/kvm/cpuid.c | 4 ++-- > > arch/x86/kvm/vmx/pmu_intel.c | 4 ++-- > > 7 files changed, 24 insertions(+), 24 deletions(-) > > > > diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c > > index a3fb996a86a1..5487a39d4975 100644 > > --- a/arch/x86/events/intel/core.c > > +++ b/arch/x86/events/intel/core.c > > @@ -5170,7 +5170,7 @@ static __init void intel_arch_events_quirk(void) > > > > static __init void intel_nehalem_quirk(void) > > { > > - union cpuid10_ebx ebx; > > + union cpuid_0xa_ebx ebx; > > > > ebx.full = x86_pmu.events_maskl; > > if (ebx.split.no_branch_misses_retired) { > > @@ -5878,9 +5878,9 @@ __init int intel_pmu_init(void) > > struct attribute **td_attr = &empty_attrs; > > struct attribute **mem_attr = &empty_attrs; > > struct attribute **tsx_attr = &empty_attrs; > > - union cpuid10_edx edx; > > - union cpuid10_eax eax; > > - union cpuid10_ebx ebx; > > + union cpuid_0xa_edx edx; > > + union cpuid_0xa_eax eax; > > + union cpuid_0xa_ebx ebx; > > unsigned int fixed_mask; > > bool pmem = false; > > int version, i; > > @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) > > * Check whether the Architectural PerfMon supports > > * Branch Misses Retired hw_event or not. > > */ > > - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); > > + cpuid(0xa, &eax.full, &ebx.full, &fixed_mask, &edx.full); > > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) > > return -ENODEV; > > > > diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c > > index c3b0d15a9841..616a6904af03 100644 > > --- a/arch/x86/events/intel/lbr.c > > +++ b/arch/x86/events/intel/lbr.c > > @@ -1497,16 +1497,16 @@ static bool is_arch_lbr_xsave_available(void) > > void __init intel_pmu_arch_lbr_init(void) > > { > > struct pmu *pmu = x86_get_pmu(smp_processor_id()); > > - union cpuid28_eax eax; > > - union cpuid28_ebx ebx; > > - union cpuid28_ecx ecx; > > + union cpuid_0x1c_eax eax; > > + union cpuid_0x1c_ebx ebx; > > + union cpuid_0x1c_ecx ecx; > > unsigned int unused_edx; > > bool arch_lbr_xsave; > > size_t size; > > u64 lbr_nr; > > > > /* Arch LBR Capabilities */ > > - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); > > + cpuid(0x1c, &eax.full, &ebx.full, &ecx.full, &unused_edx); > > > > lbr_nr = fls(eax.split.lbr_depth_mask) * 8; > > if (!lbr_nr) > > diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c > > index 42a55794004a..da3c5d748365 100644 > > --- a/arch/x86/events/intel/pt.c > > +++ b/arch/x86/events/intel/pt.c > > @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) > > } > > > > for (i = 0; i < PT_CPUID_LEAVES; i++) { > > - cpuid_count(20, i, > > + cpuid_count(0x14, i, > > &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], > > &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], > > &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], > > diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c > > index 3e9acdaeed1e..1d071974f4db 100644 > > --- a/arch/x86/events/zhaoxin/core.c > > +++ b/arch/x86/events/zhaoxin/core.c > > @@ -504,9 +504,9 @@ static __init void zhaoxin_arch_events_quirk(void) > > > > __init int zhaoxin_pmu_init(void) > > { > > - union cpuid10_edx edx; > > - union cpuid10_eax eax; > > - union cpuid10_ebx ebx; > > + union cpuid_0xa_edx edx; > > + union cpuid_0xa_eax eax; > > + union cpuid_0xa_ebx ebx; > > struct event_constraint *c; > > unsigned int unused; > > int version; > > @@ -517,7 +517,7 @@ __init int zhaoxin_pmu_init(void) > > * Check whether the Architectural PerfMon supports > > * hw_event or not. > > */ > > - cpuid(10, &eax.full, &ebx.full, &unused, &edx.full); > > + cpuid(0xa, &eax.full, &ebx.full, &unused, &edx.full); > > > > if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT - 1) > > return -ENODEV; > > diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h > > index 8fc15ed5e60b..0d2d735c8167 100644 > > --- a/arch/x86/include/asm/perf_event.h > > +++ b/arch/x86/include/asm/perf_event.h > > @@ -125,7 +125,7 @@ > > * Intel "Architectural Performance Monitoring" CPUID > > * detection/enumeration details: > > */ > > -union cpuid10_eax { > > +union cpuid_0xa_eax { > > struct { > > unsigned int version_id:8; > > unsigned int num_counters:8; > > @@ -135,7 +135,7 @@ union cpuid10_eax { > > unsigned int full; > > }; > > > > -union cpuid10_ebx { > > +union cpuid_0xa_ebx { > > struct { > > unsigned int no_unhalted_core_cycles:1; > > unsigned int no_instructions_retired:1; > > @@ -148,7 +148,7 @@ union cpuid10_ebx { > > unsigned int full; > > }; > > > > -union cpuid10_edx { > > +union cpuid_0xa_edx { > > struct { > > unsigned int num_counters_fixed:5; > > unsigned int bit_width_fixed:8; > > @@ -170,7 +170,7 @@ union cpuid10_edx { > > /* > > * Intel Architectural LBR CPUID detection/enumeration details: > > */ > > -union cpuid28_eax { > > +union cpuid_0x1c_eax { > > struct { > > /* Supported LBR depth values */ > > unsigned int lbr_depth_mask:8; > > @@ -183,7 +183,7 @@ union cpuid28_eax { > > unsigned int full; > > }; > > > > -union cpuid28_ebx { > > +union cpuid_0x1c_ebx { > > struct { > > /* CPL Filtering Supported */ > > unsigned int lbr_cpl:1; > > @@ -195,7 +195,7 @@ union cpuid28_ebx { > > unsigned int full; > > }; > > > > -union cpuid28_ecx { > > +union cpuid_0x1c_ecx { > > struct { > > /* Mispredict Bit Supported */ > > unsigned int lbr_mispred:1; > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > > index 599aebec2d52..57f43dc87538 100644 > > --- a/arch/x86/kvm/cpuid.c > > +++ b/arch/x86/kvm/cpuid.c > > @@ -967,8 +967,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) > > } > > break; > > case 0xa: { /* Architectural Performance Monitoring */ > > - union cpuid10_eax eax; > > - union cpuid10_edx edx; > > + union cpuid_0xa_eax eax; > > + union cpuid_0xa_edx edx; > > > > if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) { > > entry->eax = entry->ebx = entry->ecx = entry->edx = 0; > > diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c > > index e8a3be0b9df9..f4b165667ca9 100644 > > --- a/arch/x86/kvm/vmx/pmu_intel.c > > +++ b/arch/x86/kvm/vmx/pmu_intel.c > > @@ -512,8 +512,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) > > struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); > > struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu); > > struct kvm_cpuid_entry2 *entry; > > - union cpuid10_eax eax; > > - union cpuid10_edx edx; > > + union cpuid_0xa_eax eax; > > + union cpuid_0xa_edx edx; > > u64 perf_capabilities; > > u64 counter_mask; > > int i; [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] perf/x86: use hexidecimal value for cpuid 2023-03-31 0:43 ` Zhenyu Wang @ 2023-03-31 13:26 ` Liang, Kan 0 siblings, 0 replies; 8+ messages in thread From: Liang, Kan @ 2023-03-31 13:26 UTC (permalink / raw) To: Zhenyu Wang Cc: linux-kernel, kvm, Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim, CodyYao-oc On 2023-03-30 8:43 p.m., Zhenyu Wang wrote: > > On 2023.03.24 09:33:19 -0400, Liang, Kan wrote: >> > > sorry that I missed this one in my inbox... > >> On 2023-03-22 1:37 a.m., Zhenyu Wang wrote: >>> It's easier to use hexidecimal value instead of decimal for reading >>> and following with SDM doc, also align with other cpuid calls. As new >>> cpuid leaf would be added in future, this tries to convert current >>> forms and would take it as convention for new leaf definition. >>> >>> This changes name for both cpuid type and cpuid calls. >> >> It seems the patch touches 3 CPUIDs, 0xa, 0x1c and 0x14, right? >> The patch also crosses several different subsystems and drivers. >> I think it may be better to divide the patch by CPUID. Each patch to >> handle one CPUID. It's easier for review. > > ok, I can do that. > >> >> Also, can you please add a macro for the CPUID leaf number? >> Please refer ARCH_PERFMON_EXT_LEAF (0x23). >> > > As originally the purpose of this change is to use hex value in cpuid > call and struct name, good to see that use for 0x23. If define every > macro for these, e.g ARCH_PERFMON_LEAF(0xa), PT_LEAF(0x14), > LBR_LEAF(0x1c), struct name needs change too? As in context of what > source file you're reading, you already get idea what these cpuid > numbers are for what kind of leaf... > No, only the hex value is good enough for an union name. What I want is a consistent style for the leaf definition of the entire X86 perf code. For a union, e.g., cpuid_$hex_eax For the leaf, e.g., #define __meaning_name_macro __hex I think AMD has already done it. See EXT_PERFMON_DEBUG_FEATURES and union cpuid_0x80000022_ebx. If we have the same style, the code style will be consistent. Thanks, Kan >> >>> >>> Cc: Peter Zijlstra <peterz@infradead.org> >>> Cc: Ingo Molnar <mingo@redhat.com> >>> Cc: Arnaldo Carvalho de Melo <acme@kernel.org> >>> Cc: Mark Rutland <mark.rutland@arm.com> >>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com> >>> Cc: Jiri Olsa <jolsa@kernel.org> >>> Cc: Namhyung Kim <namhyung@kernel.org> >>> Cc: Kan Liang <kan.liang@linux.intel.com> >>> Cc: CodyYao-oc <CodyYao-oc@zhaoxin.com> >>> Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com> >>> --- >>> v3: >>> - add more explanation in commit message for purpose of this >>> - use lowercase number in call to align with current code >>> >>> v2: >>> - rename in cpuid data type as well >>> >>> arch/x86/events/intel/core.c | 10 +++++----- >>> arch/x86/events/intel/lbr.c | 8 ++++---- >>> arch/x86/events/intel/pt.c | 2 +- >>> arch/x86/events/zhaoxin/core.c | 8 ++++---- >>> arch/x86/include/asm/perf_event.h | 12 ++++++------ >>> arch/x86/kvm/cpuid.c | 4 ++-- >>> arch/x86/kvm/vmx/pmu_intel.c | 4 ++-- >>> 7 files changed, 24 insertions(+), 24 deletions(-) >>> >>> diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c >>> index a3fb996a86a1..5487a39d4975 100644 >>> --- a/arch/x86/events/intel/core.c >>> +++ b/arch/x86/events/intel/core.c >>> @@ -5170,7 +5170,7 @@ static __init void intel_arch_events_quirk(void) >>> >>> static __init void intel_nehalem_quirk(void) >>> { >>> - union cpuid10_ebx ebx; >>> + union cpuid_0xa_ebx ebx; >>> >>> ebx.full = x86_pmu.events_maskl; >>> if (ebx.split.no_branch_misses_retired) { >>> @@ -5878,9 +5878,9 @@ __init int intel_pmu_init(void) >>> struct attribute **td_attr = &empty_attrs; >>> struct attribute **mem_attr = &empty_attrs; >>> struct attribute **tsx_attr = &empty_attrs; >>> - union cpuid10_edx edx; >>> - union cpuid10_eax eax; >>> - union cpuid10_ebx ebx; >>> + union cpuid_0xa_edx edx; >>> + union cpuid_0xa_eax eax; >>> + union cpuid_0xa_ebx ebx; >>> unsigned int fixed_mask; >>> bool pmem = false; >>> int version, i; >>> @@ -5903,7 +5903,7 @@ __init int intel_pmu_init(void) >>> * Check whether the Architectural PerfMon supports >>> * Branch Misses Retired hw_event or not. >>> */ >>> - cpuid(10, &eax.full, &ebx.full, &fixed_mask, &edx.full); >>> + cpuid(0xa, &eax.full, &ebx.full, &fixed_mask, &edx.full); >>> if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT) >>> return -ENODEV; >>> >>> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c >>> index c3b0d15a9841..616a6904af03 100644 >>> --- a/arch/x86/events/intel/lbr.c >>> +++ b/arch/x86/events/intel/lbr.c >>> @@ -1497,16 +1497,16 @@ static bool is_arch_lbr_xsave_available(void) >>> void __init intel_pmu_arch_lbr_init(void) >>> { >>> struct pmu *pmu = x86_get_pmu(smp_processor_id()); >>> - union cpuid28_eax eax; >>> - union cpuid28_ebx ebx; >>> - union cpuid28_ecx ecx; >>> + union cpuid_0x1c_eax eax; >>> + union cpuid_0x1c_ebx ebx; >>> + union cpuid_0x1c_ecx ecx; >>> unsigned int unused_edx; >>> bool arch_lbr_xsave; >>> size_t size; >>> u64 lbr_nr; >>> >>> /* Arch LBR Capabilities */ >>> - cpuid(28, &eax.full, &ebx.full, &ecx.full, &unused_edx); >>> + cpuid(0x1c, &eax.full, &ebx.full, &ecx.full, &unused_edx); >>> >>> lbr_nr = fls(eax.split.lbr_depth_mask) * 8; >>> if (!lbr_nr) >>> diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c >>> index 42a55794004a..da3c5d748365 100644 >>> --- a/arch/x86/events/intel/pt.c >>> +++ b/arch/x86/events/intel/pt.c >>> @@ -235,7 +235,7 @@ static int __init pt_pmu_hw_init(void) >>> } >>> >>> for (i = 0; i < PT_CPUID_LEAVES; i++) { >>> - cpuid_count(20, i, >>> + cpuid_count(0x14, i, >>> &pt_pmu.caps[CPUID_EAX + i*PT_CPUID_REGS_NUM], >>> &pt_pmu.caps[CPUID_EBX + i*PT_CPUID_REGS_NUM], >>> &pt_pmu.caps[CPUID_ECX + i*PT_CPUID_REGS_NUM], >>> diff --git a/arch/x86/events/zhaoxin/core.c b/arch/x86/events/zhaoxin/core.c >>> index 3e9acdaeed1e..1d071974f4db 100644 >>> --- a/arch/x86/events/zhaoxin/core.c >>> +++ b/arch/x86/events/zhaoxin/core.c >>> @@ -504,9 +504,9 @@ static __init void zhaoxin_arch_events_quirk(void) >>> >>> __init int zhaoxin_pmu_init(void) >>> { >>> - union cpuid10_edx edx; >>> - union cpuid10_eax eax; >>> - union cpuid10_ebx ebx; >>> + union cpuid_0xa_edx edx; >>> + union cpuid_0xa_eax eax; >>> + union cpuid_0xa_ebx ebx; >>> struct event_constraint *c; >>> unsigned int unused; >>> int version; >>> @@ -517,7 +517,7 @@ __init int zhaoxin_pmu_init(void) >>> * Check whether the Architectural PerfMon supports >>> * hw_event or not. >>> */ >>> - cpuid(10, &eax.full, &ebx.full, &unused, &edx.full); >>> + cpuid(0xa, &eax.full, &ebx.full, &unused, &edx.full); >>> >>> if (eax.split.mask_length < ARCH_PERFMON_EVENTS_COUNT - 1) >>> return -ENODEV; >>> diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h >>> index 8fc15ed5e60b..0d2d735c8167 100644 >>> --- a/arch/x86/include/asm/perf_event.h >>> +++ b/arch/x86/include/asm/perf_event.h >>> @@ -125,7 +125,7 @@ >>> * Intel "Architectural Performance Monitoring" CPUID >>> * detection/enumeration details: >>> */ >>> -union cpuid10_eax { >>> +union cpuid_0xa_eax { >>> struct { >>> unsigned int version_id:8; >>> unsigned int num_counters:8; >>> @@ -135,7 +135,7 @@ union cpuid10_eax { >>> unsigned int full; >>> }; >>> >>> -union cpuid10_ebx { >>> +union cpuid_0xa_ebx { >>> struct { >>> unsigned int no_unhalted_core_cycles:1; >>> unsigned int no_instructions_retired:1; >>> @@ -148,7 +148,7 @@ union cpuid10_ebx { >>> unsigned int full; >>> }; >>> >>> -union cpuid10_edx { >>> +union cpuid_0xa_edx { >>> struct { >>> unsigned int num_counters_fixed:5; >>> unsigned int bit_width_fixed:8; >>> @@ -170,7 +170,7 @@ union cpuid10_edx { >>> /* >>> * Intel Architectural LBR CPUID detection/enumeration details: >>> */ >>> -union cpuid28_eax { >>> +union cpuid_0x1c_eax { >>> struct { >>> /* Supported LBR depth values */ >>> unsigned int lbr_depth_mask:8; >>> @@ -183,7 +183,7 @@ union cpuid28_eax { >>> unsigned int full; >>> }; >>> >>> -union cpuid28_ebx { >>> +union cpuid_0x1c_ebx { >>> struct { >>> /* CPL Filtering Supported */ >>> unsigned int lbr_cpl:1; >>> @@ -195,7 +195,7 @@ union cpuid28_ebx { >>> unsigned int full; >>> }; >>> >>> -union cpuid28_ecx { >>> +union cpuid_0x1c_ecx { >>> struct { >>> /* Mispredict Bit Supported */ >>> unsigned int lbr_mispred:1; >>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c >>> index 599aebec2d52..57f43dc87538 100644 >>> --- a/arch/x86/kvm/cpuid.c >>> +++ b/arch/x86/kvm/cpuid.c >>> @@ -967,8 +967,8 @@ static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function) >>> } >>> break; >>> case 0xa: { /* Architectural Performance Monitoring */ >>> - union cpuid10_eax eax; >>> - union cpuid10_edx edx; >>> + union cpuid_0xa_eax eax; >>> + union cpuid_0xa_edx edx; >>> >>> if (!static_cpu_has(X86_FEATURE_ARCH_PERFMON)) { >>> entry->eax = entry->ebx = entry->ecx = entry->edx = 0; >>> diff --git a/arch/x86/kvm/vmx/pmu_intel.c b/arch/x86/kvm/vmx/pmu_intel.c >>> index e8a3be0b9df9..f4b165667ca9 100644 >>> --- a/arch/x86/kvm/vmx/pmu_intel.c >>> +++ b/arch/x86/kvm/vmx/pmu_intel.c >>> @@ -512,8 +512,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu) >>> struct kvm_pmu *pmu = vcpu_to_pmu(vcpu); >>> struct lbr_desc *lbr_desc = vcpu_to_lbr_desc(vcpu); >>> struct kvm_cpuid_entry2 *entry; >>> - union cpuid10_eax eax; >>> - union cpuid10_edx edx; >>> + union cpuid_0xa_eax eax; >>> + union cpuid_0xa_edx edx; >>> u64 perf_capabilities; >>> u64 counter_mask; >>> int i; ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-03-31 13:26 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-03-08 3:15 [PATCH] perf/x86/intel: use hexidecimal value for cpuid Zhenyu Wang 2023-03-08 14:16 ` Peter Zijlstra 2023-03-09 1:48 ` Zhenyu Wang 2023-03-12 13:26 ` [PATCH v2] perf/x86: " Zhenyu Wang 2023-03-22 5:37 ` [PATCH v3] " Zhenyu Wang 2023-03-24 13:33 ` Liang, Kan 2023-03-31 0:43 ` Zhenyu Wang 2023-03-31 13:26 ` Liang, Kan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox