linux-kernel - Re: [PATCH V3] arm64: perf: Retain PMCR.X of PMCR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3b07bd45-42e7-0298-70d5-60d950e27304@arm.com>
Date:   Tue, 10 May 2022 10:26:52 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Srinivasarao Pathipati <quic_c_spathi@...cinc.com>,
        will@...nel.org, mark.rutland@....com, peterz@...radead.org,
        mingo@...hat.com, acme@...nel.org,
        alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
        namhyung@...nel.org, catalin.marinas@....com,
        linux-arm-kernel@...ts.infradead.org,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V3] arm64: perf: Retain PMCR.X of PMCR_EL0 during reset

On 2022-05-09 18:28, Srinivasarao Pathipati wrote:
> Preserve the bit PMCR.X of PMCR_EL0 during PMU reset to export
> PMU events. as it could be set before PMU initialization.

Reiterating my previous comment, if there are use-cases for enabling the 
event export bus under Linux, then it makes most sense to have an option 
to enable the event export bus. It shouldn't have to depend on whatever 
the bootloader may or may not have done.

What would it even mean to preserve a value of 0 instead of resetting it 
to 0?

Thanks,
Robin.

> Exporting events could result in additional power consumption
> so making it configurable.
> 
> Signed-off-by: Srinivasarao Pathipati <quic_c_spathi@...cinc.com>
> ---
> Changes since V2:
> 	Done below changes as per Will's comments
> 	- enabling pmcr_x now configurable with kernel parameters and
> 	  by default it is disabled.
> 	
> Changes since V1:
> 	- Preserving only PMCR_X bit as per Robin Murphy's comment.
> ---
>   Documentation/admin-guide/kernel-parameters.txt |  4 ++++
>   arch/arm64/kernel/perf_event.c                  | 13 +++++++++++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index de3da15..cf3410e 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5150,6 +5150,10 @@
>   			Useful for devices that are detected asynchronously
>   			(e.g. USB and MMC devices).
>   
> +	retain_pmcr_x
> +			[KNL] Retain export bit of PMCR_EL0 that is getting reset
> +			during pmu reset.
> +
>   	retain_initrd	[RAM] Keep initrd memory after extraction
>   
>   	rfkill.default_state=
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index cb69ff1..1ac251f 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -34,6 +34,7 @@
>   #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_ACCESS		0xEC
>   #define ARMV8_THUNDER_PERFCTR_L1I_CACHE_PREF_MISS		0xED
>   
> +static int export_pmu_events  __read_mostly;
>   /*
>    * ARMv8 Architectural defined events, not all of these may
>    * be supported on any given implementation. Unsupported events will
> @@ -1025,6 +1026,14 @@ static int armv8pmu_filter_match(struct perf_event *event)
>   	return evtype != ARMV8_PMUV3_PERFCTR_CHAIN;
>   }
>   
> +static int __init retain_pmcr_x(char *str)
> +{
> +	export_pmu_events = 1;
> +	return 0;
> +}
> +
> +early_param("retain_pmcr_x", retain_pmcr_x);
> +
>   static void armv8pmu_reset(void *info)
>   {
>   	struct arm_pmu *cpu_pmu = (struct arm_pmu *)info;
> @@ -1047,6 +1056,10 @@ static void armv8pmu_reset(void *info)
>   	if (armv8pmu_has_long_event(cpu_pmu))
>   		pmcr |= ARMV8_PMU_PMCR_LP;
>   
> +	/* Preserve PMCR_X to export PMU events */
> +	if (export_pmu_events)
> +		pmcr |= (armv8pmu_pmcr_read() & ARMV8_PMU_PMCR_X);
> +
>   	armv8pmu_pmcr_write(pmcr);
>   }
>