[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH 1/3] xen/arm: Sync sysregs and cpuinfo with Linux 5.18-rc3
Hi Julien, > On 3 May 2022, at 19:08, Julien Grall <julien@xxxxxxx> wrote: > > Hi Bertrand, > > On 03/05/2022 10:38, Bertrand Marquis wrote: >> Sync arm64 sysreg bit shift definitions with status of Linux kernel as >> of 5.18-rc3 version (linux commit b2d229d4ddb1). >> Sync ID registers sanitization with the status of Linux 5.18-rc3 and add >> sanitization of ISAR2 registers. > Please outline which specific commits you are actually backported. This would > help to know what changed, why and also keep track of the autorships. > > When possible, the changes should be separated to match each Linux commit we > backport. As those are exactly identical to the linux tree, one can easily use git blame on the linux source tree to find those information if it is needed. I checked a bit and this is not something that was required before (for example when the cpufeature was introduced). > >> Complete AA64ISAR2 and AA64MMFR1 with more fields. >> While there add a comment for MMFR bitfields as for other registers in >> the cpuinfo structure definition. > > AFAICT, this patch is doing 3 different things that are somewhat related: > - Sync cpufeature.c > - Update the headers with unused defines > - Complete the structure cpufeature.h > > All those changes seem to be independent, so I think they should be done > separately. This would help to keep the authorship right (your code vs Linux > code). This and the previous request to split using linux commit will actually end up in 10 patches or more. In the current, the change can easily be checked doing a diff with the mentioned Linux version, so I am not really thrilled to make it more complex. Please confirm that all this is really what you want. > >> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx> >> --- >> xen/arch/arm/arm64/cpufeature.c | 18 +++++- >> xen/arch/arm/include/asm/arm64/sysregs.h | 76 ++++++++++++++++++++---- >> xen/arch/arm/include/asm/cpufeature.h | 14 ++++- >> 3 files changed, 91 insertions(+), 17 deletions(-) >> diff --git a/xen/arch/arm/arm64/cpufeature.c >> b/xen/arch/arm/arm64/cpufeature.c >> index 6e5d30dc7b..d9039d37b2 100644 >> --- a/xen/arch/arm/arm64/cpufeature.c >> +++ b/xen/arch/arm/arm64/cpufeature.c >> @@ -143,6 +143,16 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = >> { >> ARM64_FTR_END, >> }; >> +static const struct arm64_ftr_bits ftr_id_aa64isar2[] = { >> + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_HIGHER_SAFE, >> ID_AA64ISAR2_CLEARBHB_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), >> + FTR_STRICT, FTR_EXACT, ID_AA64ISAR2_APA3_SHIFT, 4, 0), >> + ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH), > So we are using CONFIG_ARM64_PTR_AUTH. But this is not defined in Kconfig. I > realize there are more in cpufeature.c (somehow I didn't spot during > preview), but I don't think this is right to define CONFIG_* without an > associated entry in Kconfig. > > In one hand, I think it would be odd to add an entry in Kconfig because Xen > wouldn't properly work if selected. On the other hand, it is useful if when > we will implement pointer authentification. > > So maybe we should just add the Kconfig entry with a comment explaning why > they are not selected. Any thoughts? This is really right and a very good catch. I think it would make sense to introduce those in Kconfig in order to keep the code equivalent to Linux. So I would suggest here to add hidden entries like this: ARM64_PTR_AUTH def_bool n depends on ARM64 help Pointer authentication support. This feature is not supported by Xen. Cheers Bertrand > > Cheers, > > -- > Julien Grall
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |