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

[Robin Murphy]

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@xxxxxxxxxxxxx>
> ---
>   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 */