[PATCH v3 4/7] perf/x86/uncore: Add L3 PMU support for Hygon family 18h model 6h

[PATCH v3 4/7] perf/x86/uncore: Add L3 PMU support for Hygon family 18h model 6h

[Fu Hao]

Adjust the L3 PMU slicemask and threadmask for Hygon family 18h
model 6h processor.

Signed-off-by: Fu Hao <fuhao@open-hieco.net>
Tested-by: Tingyin Duan <tingyin.duan@gmail.com>
---
 arch/x86/events/amd/uncore.c      | 48 ++++++++++++++++++++++++++++++-
 arch/x86/include/asm/perf_event.h |  8 ++++++
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
index dd956cfca..e71d9e784 100644
--- a/arch/x86/events/amd/uncore.c
+++ b/arch/x86/events/amd/uncore.c
@@ -308,6 +308,14 @@ amd_f17h_uncore_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 	       attr->mode : 0;
 }
 
+static umode_t
+hygon_f18h_m6h_uncore_is_visible(struct kobject *kobj, struct attribute *attr, int i)
+{
+	return boot_cpu_data.x86 == 0x18 &&
+	       boot_cpu_data.x86_model >= 0x6 && boot_cpu_data.x86_model <= 0xf	?
+	       attr->mode : 0;
+}
+
 static umode_t
 amd_f19h_uncore_is_visible(struct kobject *kobj, struct attribute *attr, int i)
 {
@@ -359,6 +367,8 @@ DEFINE_UNCORE_FORMAT_ATTR(enallslices,	enallslices,	"config:46");		   /* F19h L3
 DEFINE_UNCORE_FORMAT_ATTR(enallcores,	enallcores,	"config:47");		   /* F19h L3 */
 DEFINE_UNCORE_FORMAT_ATTR(sliceid,	sliceid,	"config:48-50");	   /* F19h L3 */
 DEFINE_UNCORE_FORMAT_ATTR(rdwrmask,	rdwrmask,	"config:8-9");		   /* PerfMonV2 UMC */
+DEFINE_UNCORE_FORMAT_ATTR(slicemask4,	slicemask,	"config:28-31");	   /* F18h L3 */
+DEFINE_UNCORE_FORMAT_ATTR(threadmask32,	threadmask,	"config:32-63");	   /* F18h L3 */
 
 /* Common DF and NB attributes */
 static struct attribute *amd_uncore_df_format_attr[] = {
@@ -388,6 +398,12 @@ static struct attribute *amd_f17h_uncore_l3_format_attr[] = {
 	NULL,
 };
 
+/* F18h M06h unique L3 attributes */
+static struct attribute *hygon_f18h_m6h_uncore_l3_format_attr[] = {
+	&format_attr_slicemask4.attr,	/* slicemask */
+	NULL,
+};
+
 /* F19h unique L3 attributes */
 static struct attribute *amd_f19h_uncore_l3_format_attr[] = {
 	&format_attr_coreid.attr,	/* coreid */
@@ -413,6 +429,12 @@ static struct attribute_group amd_f17h_uncore_l3_format_group = {
 	.is_visible = amd_f17h_uncore_is_visible,
 };
 
+static struct attribute_group hygon_f18h_m6h_uncore_l3_format_group = {
+	.name = "format",
+	.attrs = hygon_f18h_m6h_uncore_l3_format_attr,
+	.is_visible = hygon_f18h_m6h_uncore_is_visible,
+};
+
 static struct attribute_group amd_f19h_uncore_l3_format_group = {
 	.name = "format",
 	.attrs = amd_f19h_uncore_l3_format_attr,
@@ -442,6 +464,11 @@ static const struct attribute_group *amd_uncore_l3_attr_update[] = {
 	NULL,
 };
 
+static const struct attribute_group *hygon_uncore_l3_attr_update[] = {
+	&hygon_f18h_m6h_uncore_l3_format_group,
+	NULL,
+};
+
 static const struct attribute_group *amd_uncore_umc_attr_groups[] = {
 	&amd_uncore_attr_group,
 	&amd_uncore_umc_format_group,
@@ -820,6 +847,12 @@ static int amd_uncore_l3_event_init(struct perf_event *event)
 		mask = AMD64_L3_F19H_THREAD_MASK | AMD64_L3_EN_ALL_SLICES |
 		       AMD64_L3_EN_ALL_CORES;
 
+	if (boot_cpu_data.x86 == 0x18 &&
+	    boot_cpu_data.x86_model >= 0x6 &&
+	    boot_cpu_data.x86_model <= 0xf)
+		mask = ((config & HYGON_L3_SLICE_MASK) ? : HYGON_L3_SLICE_MASK) |
+		       ((config & HYGON_L3_THREAD_MASK) ? : HYGON_L3_THREAD_MASK);
+
 	hwc->config |= mask;
 
 	return 0;
@@ -877,7 +910,8 @@ int amd_uncore_l3_ctx_init(struct amd_uncore *uncore, unsigned int cpu)
 	pmu->rdpmc_base = RDPMC_BASE_LLC;
 	pmu->group = amd_uncore_ctx_gid(uncore, cpu);
 
-	if (boot_cpu_data.x86 >= 0x17) {
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
+	    boot_cpu_data.x86 >= 0x17) {
 		*l3_attr++ = &format_attr_event8.attr;
 		*l3_attr++ = &format_attr_umask8.attr;
 		*l3_attr++ = boot_cpu_data.x86 >= 0x19 ?
@@ -904,6 +938,18 @@ int amd_uncore_l3_ctx_init(struct amd_uncore *uncore, unsigned int cpu)
 		.module		= THIS_MODULE,
 	};
 
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON &&
+	    boot_cpu_data.x86 == 0x18) {
+		*l3_attr++ = &format_attr_event8.attr;
+		*l3_attr++ = &format_attr_umask8.attr;
+		if (boot_cpu_data.x86_model >= 0x6 && boot_cpu_data.x86_model <= 0xf) {
+			*l3_attr++ = &format_attr_threadmask32.attr;
+			pmu->pmu.attr_update = hygon_uncore_l3_attr_update;
+		} else {
+			*l3_attr++ = &format_attr_threadmask8.attr;
+		}
+	}
+
 	if (perf_pmu_register(&pmu->pmu, pmu->pmu.name, -1)) {
 		free_percpu(pmu->ctx);
 		pmu->ctx = NULL;
diff --git a/arch/x86/include/asm/perf_event.h b/arch/x86/include/asm/perf_event.h
index ff5acb8b1..404752601 100644
--- a/arch/x86/include/asm/perf_event.h
+++ b/arch/x86/include/asm/perf_event.h
@@ -89,6 +89,14 @@
 #define AMD64_L3_COREID_MASK				\
 	(0x7ULL << AMD64_L3_COREID_SHIFT)
 
+#define HYGON_L3_SLICE_SHIFT				28
+#define HYGON_L3_SLICE_MASK				\
+	(0xFULL << HYGON_L3_SLICE_SHIFT)
+
+#define HYGON_L3_THREAD_SHIFT				32
+#define HYGON_L3_THREAD_MASK				\
+	(0xFFFFFFFFULL << HYGON_L3_THREAD_SHIFT)
+
 #define X86_RAW_EVENT_MASK		\
 	(ARCH_PERFMON_EVENTSEL_EVENT |	\
 	 ARCH_PERFMON_EVENTSEL_UMASK |	\
-- 
2.34.1

Re: [PATCH v3 4/7] perf/x86/uncore: Add L3 PMU support for Hygon family 18h model 6h

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Registered PMUs are not unregistered if `cpuhp_setup_state()` fails during module initialization, leading to a Use-After-Free.
--

commit 98d69c16b51701dce640f5ac92cee0daa35f2cfb
Author: Fu Hao <fuhao@open-hieco.net>

perf/x86/uncore: Add L3 PMU support for Hygon family 18h model 6h

This patch adds L3 PMU support for the Hygon family 18h model 6h processor
by adjusting the slicemask and threadmask in the uncore driver.

> diff --git a/arch/x86/events/amd/uncore.c b/arch/x86/events/amd/uncore.c
> index 9293ce50574da..21f00edd3caa6 100644
> --- a/arch/x86/events/amd/uncore.c
> +++ b/arch/x86/events/amd/uncore.c

[ ... ]

> @@ -877,7 +910,8 @@ int amd_uncore_l3_ctx_init(struct amd_uncore *uncore, unsigned int cpu)
>  	pmu->rdpmc_base = RDPMC_BASE_LLC;
>  	pmu->group = amd_uncore_ctx_gid(uncore, cpu);
>  
> -	if (boot_cpu_data.x86 >= 0x17) {
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_AMD &&
> +	    boot_cpu_data.x86 >= 0x17) {
>  		*l3_attr++ = &format_attr_event8.attr;
>  		*l3_attr++ = &format_attr_umask8.attr;

[ ... ]

> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON &&
> +	    boot_cpu_data.x86 == 0x18) {
> +		*l3_attr++ = &format_attr_event8.attr;
> +		*l3_attr++ = &format_attr_umask8.attr;
> +		if (boot_cpu_data.x86_model >= 0x6 && boot_cpu_data.x86_model <= 0xf) {
> +			*l3_attr++ = &format_attr_threadmask32.attr;
> +			pmu->pmu.attr_update = hygon_uncore_l3_attr_update;
> +		} else {
> +			*l3_attr++ = &format_attr_threadmask8.attr;
> +		}
> +	}
> +
>  	if (perf_pmu_register(&pmu->pmu, pmu->pmu.name, -1)) {
>  		free_percpu(pmu->ctx);
>  		pmu->ctx = NULL;

[Severity: High]
This is a pre-existing issue, but registering the PMU here exposes a teardown
issue in the module initialization error path.

If cpuhp_setup_state() fails for the online state in amd_uncore_init(), the
error path jumps to fail_start and removes the hotplug states. This triggers
the amd_uncore_cpu_dead() teardown callback, which correctly frees the
per-CPU context by calling amd_uncore_ctx_free().

However, the error path does not unregister the PMU or free the uncore->pmus
array:

arch/x86/events/amd/uncore.c:amd_uncore_init() {
    ...
fail:
	for (i = 0; i < UNCORE_TYPE_MAX; i++) {
		uncore = &uncores[i];
		if (uncore->info) {
			free_percpu(uncore->info);
			uncore->info = NULL;
		}
	}

	return ret;
}

Since the initialization failed, the module will be unloaded, but the perf
subsystem retains references to the registered PMUs.

Could this lead to a use-after-free if the perf subsystem later accesses
these dangling PMUs (for instance, via perf stat) after the module memory
has been freed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1781523812.git.fuhao@open-hieco.net?part=4