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.
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).
Signed-off-by: Bertrand Marquis <bertrand.marq...@arm.com>
---
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?
Cheers,
--
Julien Grall