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

From: Jonathan Cameron <Jonathan.Cameron@Huawei.com>
To: Robin Murphy <robin.murphy@arm.com>
Cc: guojinhui.liam <guojinhui.liam@bytedance.com>,
	<catalin.marinas@arm.com>, <will@kernel.org>,
	<lizefan.x@bytedance.com>, <linux-arm-kernel@lists.infradead.org>,
	<linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] arm64: cpufeature: Expose the real mpidr value to EL0
Date: Tue, 12 Sep 2023 11:51:16 +0100	[thread overview]
Message-ID: <20230912115116.000049b0@Huawei.com> (raw)
In-Reply-To: <759c91b9-856e-a778-0e0a-e52240e5c8ce@arm.com>

On Tue, 12 Sep 2023 09:31:43 +0100
Robin Murphy <robin.murphy@arm.com> wrote:

> 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

As a follow up question, is there some information that is missing from
current topology description?  (there is lots missing but I'm curious
as to what might matter for your use case!)

Jonathan

> 
> > In some scenarios, the task scheduling in userspace can be
> > optimized with CPU Die information.
> > 
> > Signed-off-by: guojinhui.liam <guojinhui.liam@bytedance.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 */  
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel