linux-kernel - Re: [PATCH] arm64: cpufeature: Expose the real mpidr value to EL0

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 for Android: free password hash cracker in your pocket

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

Message-ID: <759c91b9-856e-a778-0e0a-e52240e5c8ce@arm.com>
Date:   Tue, 12 Sep 2023 09:31:43 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     "guojinhui.liam" <guojinhui.liam@...edance.com>,
        catalin.marinas@....com, will@...nel.org
Cc:     lizefan.x@...edance.com, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: cpufeature: Expose the real mpidr value to EL0

On 2023-09-12 04:52, guojinhui.liam wrote:
> In EL0, it can get the register midr's value to distinguish vendor.
> But it won't return real value of the register mpidr by using mrs
> in EL0. The register mpidr's value is useful to obtain the cpu
> topology information.

...except there's no guarantee that the MPIDR value is anything other 
than a unique identifier. Proper topology information is already exposed 
to userspace[1], as described by ACPI PPTT or Devicetree[2]. Userspace 
should be using that.

Not to mention that userspace fundamentally can't guarantee it won't be 
migrated at just the wrong point and read the MPIDR of a different CPU 
anyway. (This is why the MIDRs and REVIDRs are also reported via sysfs, 
such that userspace has a stable and reliable source of information in 
case it needs to consider potential errata.)

Thanks,
Robin.

[1] https://www.kernel.org/doc/html/latest/admin-guide/cputopology.html
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/cpu/cpu-topology.txt

> In some scenarios, the task scheduling in userspace can be
> optimized with CPU Die information.
> 
> Signed-off-by: guojinhui.liam <guojinhui.liam@...edance.com>
> ---
>   arch/arm64/include/asm/sysreg.h | 3 ---
>   arch/arm64/kernel/cpufeature.c  | 2 +-
>   2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 38296579a4fd..1885857c8a22 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -901,9 +901,6 @@
>   #define SYS_TFSR_EL1_TF0	(UL(1) << SYS_TFSR_EL1_TF0_SHIFT)
>   #define SYS_TFSR_EL1_TF1	(UL(1) << SYS_TFSR_EL1_TF1_SHIFT)
>   
> -/* Safe value for MPIDR_EL1: Bit31:RES1, Bit30:U:0, Bit24:MT:0 */
> -#define SYS_MPIDR_SAFE_VAL	(BIT(31))
> -
>   #define TRFCR_ELx_TS_SHIFT		5
>   #define TRFCR_ELx_TS_MASK		((0x3UL) << TRFCR_ELx_TS_SHIFT)
>   #define TRFCR_ELx_TS_VIRTUAL		((0x1UL) << TRFCR_ELx_TS_SHIFT)
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index b018ae12ff5f..6e18597fdcc3 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -3450,7 +3450,7 @@ static inline int emulate_id_reg(u32 id, u64 *valp)
>   		*valp = read_cpuid_id();
>   		break;
>   	case SYS_MPIDR_EL1:
> -		*valp = SYS_MPIDR_SAFE_VAL;
> +		*valp = read_cpuid_mpidr();
>   		break;
>   	case SYS_REVIDR_EL1:
>   		/* IMPLEMENTATION DEFINED values are emulated with 0 */