From: Ard Biesheuvel ardb@kernel.org
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes: 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs") Cc: stable@vger.kernel.org Signed-off-by: Ard Biesheuvel ardb@kernel.org --- arch/arm64/include/asm/assembler.h | 5 +++++ arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++ arch/arm64/kernel/pi/map_kernel.c | 6 ++++++ arch/arm64/mm/init.c | 2 +- 5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 3d8d534a7a77..ad63457a05c5 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -343,6 +343,11 @@ alternative_cb_end // Narrow PARange to fit the PS field in TCR_ELx ubfx \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3 mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX +#ifdef CONFIG_ARM64_LPA2 +alternative_if_not ARM64_HAS_VA52 + mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48 +alternative_else_nop_endif +#endif cmp \tmp0, \tmp1 csel \tmp0, \tmp1, \tmp0, hi bfi \tcr, \tmp0, \pos, #3 diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 37e4c02e0272..6f5137040ff6 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void) return;
safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1); - mmfr0 = read_cpuid(ID_AA64MMFR0_EL1); + mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1); mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
/* Verify VMID bits */ diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c index 22159251eb3a..c6b185b885f7 100644 --- a/arch/arm64/kernel/pi/idreg-override.c +++ b/arch/arm64/kernel/pi/idreg-override.c @@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val) id_aa64mmfr0_override.val |= (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT; id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT; + + /* + * Override PARange to 48 bits - the override will just be + * ignored if the actual PARange is smaller, but this is + * unlikely to be the case for LPA2 capable silicon. + */ + id_aa64mmfr0_override.val |= + ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT; + id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT; } #endif return true; diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c index f374a3e5a5fe..e57b043f324b 100644 --- a/arch/arm64/kernel/pi/map_kernel.c +++ b/arch/arm64/kernel/pi/map_kernel.c @@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) { u64 sctlr = read_sysreg(sctlr_el1); u64 tcr = read_sysreg(tcr_el1) | TCR_DS; + u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1); + u64 parange = cpuid_feature_extract_unsigned_field(mmfr0, + ID_AA64MMFR0_EL1_PARANGE_SHIFT); + + tcr &= ~TCR_IPS_MASK; + tcr |= parange << TCR_IPS_SHIFT;
asm(" msr sctlr_el1, %0 ;" " isb ;" diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index d21f67d67cf5..4db9887b2aef 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { extern u16 memstart_offset_seed; - u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1); + u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1); int parange = cpuid_feature_extract_unsigned_field( mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT); s64 range = linear_region_size -
On 11/11/24 14:05, Ard Biesheuvel wrote:
From: Ard Biesheuvel ardb@kernel.org
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes: 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs") Cc: stable@vger.kernel.org Signed-off-by: Ard Biesheuvel ardb@kernel.org
arch/arm64/include/asm/assembler.h | 5 +++++ arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++ arch/arm64/kernel/pi/map_kernel.c | 6 ++++++ arch/arm64/mm/init.c | 2 +- 5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 3d8d534a7a77..ad63457a05c5 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -343,6 +343,11 @@ alternative_cb_end // Narrow PARange to fit the PS field in TCR_ELx ubfx \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3 mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX +#ifdef CONFIG_ARM64_LPA2 +alternative_if_not ARM64_HAS_VA52
- mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
+alternative_else_nop_endif +#endif
I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.
cmp \tmp0, \tmp1 csel \tmp0, \tmp1, \tmp0, hi bfi \tcr, \tmp0, \pos, #3 diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 37e4c02e0272..6f5137040ff6 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void) return; safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
- mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
- mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?
mmfr1 = read_cpuid(ID_AA64MMFR1_EL1); /* Verify VMID bits */ diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c index 22159251eb3a..c6b185b885f7 100644 --- a/arch/arm64/kernel/pi/idreg-override.c +++ b/arch/arm64/kernel/pi/idreg-override.c @@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val) id_aa64mmfr0_override.val |= (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT; id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
/** Override PARange to 48 bits - the override will just be* ignored if the actual PARange is smaller, but this is* unlikely to be the case for LPA2 capable silicon.*/id_aa64mmfr0_override.val |=ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
Could these be used instead ?
SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?
} #endif return true; diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c index f374a3e5a5fe..e57b043f324b 100644 --- a/arch/arm64/kernel/pi/map_kernel.c +++ b/arch/arm64/kernel/pi/map_kernel.c @@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) { u64 sctlr = read_sysreg(sctlr_el1); u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
- u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
- u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
ID_AA64MMFR0_EL1_PARANGE_SHIFT);- tcr &= ~TCR_IPS_MASK;
Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.
- tcr |= parange << TCR_IPS_SHIFT;
Wondering if FIELD_PREP() could be used here.
asm(" msr sctlr_el1, %0 ;" " isb ;" diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index d21f67d67cf5..4db9887b2aef 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -280,7 +280,7 @@ void __init arm64_memblock_init(void) if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { extern u16 memstart_offset_seed;
u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Could this have a comment explaining the need for sanitized value ?
int parange = cpuid_feature_extract_unsigned_field( mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT); s64 range = linear_region_size -
Otherwise LGTM.
Hi Anshuman,
On Fri, 15 Nov 2024 at 07:05, Anshuman Khandual anshuman.khandual@arm.com wrote:
On 11/11/24 14:05, Ard Biesheuvel wrote:
From: Ard Biesheuvel ardb@kernel.org
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes: 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs") Cc: stable@vger.kernel.org Signed-off-by: Ard Biesheuvel ardb@kernel.org
arch/arm64/include/asm/assembler.h | 5 +++++ arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++ arch/arm64/kernel/pi/map_kernel.c | 6 ++++++ arch/arm64/mm/init.c | 2 +- 5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 3d8d534a7a77..ad63457a05c5 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -343,6 +343,11 @@ alternative_cb_end // Narrow PARange to fit the PS field in TCR_ELx ubfx \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3 mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX +#ifdef CONFIG_ARM64_LPA2 +alternative_if_not ARM64_HAS_VA52
mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48+alternative_else_nop_endif +#endif
I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.
It is the other way around, actually. This limit will always be applied on primary boot, which is why IPS is updated again in set_ttbr0_for_lpa2() [below]. Before booting the secondaries (or other subsequent invocations of this code, e.g., in the resume path), this MOV will be NOPed out if LPA2 support is enabled.
cmp \tmp0, \tmp1 csel \tmp0, \tmp1, \tmp0, hi bfi \tcr, \tmp0, \pos, #3diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 37e4c02e0272..6f5137040ff6 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void) return;
safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?
safe_mmfr1 exists because there is also mmfr1 in the same scope. No such distinction exists for mmfr0, so I opted for keeping the name.
mmfr1 = read_cpuid(ID_AA64MMFR1_EL1); /* Verify VMID bits */diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c index 22159251eb3a..c6b185b885f7 100644 --- a/arch/arm64/kernel/pi/idreg-override.c +++ b/arch/arm64/kernel/pi/idreg-override.c @@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val) id_aa64mmfr0_override.val |= (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT; id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
/** Override PARange to 48 bits - the override will just be* ignored if the actual PARange is smaller, but this is* unlikely to be the case for LPA2 capable silicon.*/id_aa64mmfr0_override.val |=ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;Could these be used instead ?
SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?
Yes, but 2 lines before, there is another occurrence of this idiom, and I did not want to deviate from that.
We could update both, or update the other one first in a separate patch, I suppose.
}#endif return true; diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c index f374a3e5a5fe..e57b043f324b 100644 --- a/arch/arm64/kernel/pi/map_kernel.c +++ b/arch/arm64/kernel/pi/map_kernel.c @@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) { u64 sctlr = read_sysreg(sctlr_el1); u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,ID_AA64MMFR0_EL1_PARANGE_SHIFT);tcr &= ~TCR_IPS_MASK;Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.
As explained above, TCR.IPS will be capped at 48 bits up to this point.
tcr |= parange << TCR_IPS_SHIFT;Wondering if FIELD_PREP() could be used here.
AIUI we'd end up with
tcr &= ~TCR_IPS_MASK; tcr |= FIELD_PREP(TCR_IPS_MASK, parange);
Is that really so much better?
asm(" msr sctlr_el1, %0 ;" " isb ;"diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index d21f67d67cf5..4db9887b2aef 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { extern u16 memstart_offset_seed;
u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);Could this have a comment explaining the need for sanitized value ?
Sure. Actually, it shouldn't make any difference here (unless we allow PARange to be narrowed even further, which might make sense if we care about enabling randomization of the linear map on systems where PARange is much larger than the size of the physical address space that is actually populated). However, for consistency, it is better to avoid the 52-bit PARange if LPA2 is disabled.
int parange = cpuid_feature_extract_unsigned_field( mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT); s64 range = linear_region_size -Otherwise LGTM.
Thanks!
On 11/15/24 14:30, Ard Biesheuvel wrote:
Hi Anshuman,
On Fri, 15 Nov 2024 at 07:05, Anshuman Khandual anshuman.khandual@arm.com wrote:
On 11/11/24 14:05, Ard Biesheuvel wrote:
From: Ard Biesheuvel ardb@kernel.org
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes: 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs") Cc: stable@vger.kernel.org Signed-off-by: Ard Biesheuvel ardb@kernel.org
arch/arm64/include/asm/assembler.h | 5 +++++ arch/arm64/kernel/cpufeature.c | 2 +- arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++ arch/arm64/kernel/pi/map_kernel.c | 6 ++++++ arch/arm64/mm/init.c | 2 +- 5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 3d8d534a7a77..ad63457a05c5 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -343,6 +343,11 @@ alternative_cb_end // Narrow PARange to fit the PS field in TCR_ELx ubfx \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3 mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX +#ifdef CONFIG_ARM64_LPA2 +alternative_if_not ARM64_HAS_VA52
mov \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48+alternative_else_nop_endif +#endif
I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.
It is the other way around, actually. This limit will always be
Right, missed the '_if_not' part.
applied on primary boot, which is why IPS is updated again in set_ttbr0_for_lpa2() [below]. Before booting the secondaries (or other subsequent invocations of this code, e.g., in the resume path), this MOV will be NOPed out if LPA2 support is enabled.
Understood.
cmp \tmp0, \tmp1 csel \tmp0, \tmp1, \tmp0, hi bfi \tcr, \tmp0, \pos, #3diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 37e4c02e0272..6f5137040ff6 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void) return;
safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?
safe_mmfr1 exists because there is also mmfr1 in the same scope. No such distinction exists for mmfr0, so I opted for keeping the name.
Fair enough.
mmfr1 = read_cpuid(ID_AA64MMFR1_EL1); /* Verify VMID bits */diff --git a/arch/arm64/kernel/pi/idreg-override.c b/arch/arm64/kernel/pi/idreg-override.c index 22159251eb3a..c6b185b885f7 100644 --- a/arch/arm64/kernel/pi/idreg-override.c +++ b/arch/arm64/kernel/pi/idreg-override.c @@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val) id_aa64mmfr0_override.val |= (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT; id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
/** Override PARange to 48 bits - the override will just be* ignored if the actual PARange is smaller, but this is* unlikely to be the case for LPA2 capable silicon.*/id_aa64mmfr0_override.val |=ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;Could these be used instead ?
SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?
Yes, but 2 lines before, there is another occurrence of this idiom, and I did not want to deviate from that.
We could update both, or update the other one first in a separate patch, I suppose.
Sure, have your choice, don't have a strong view on either method.
}#endif return true; diff --git a/arch/arm64/kernel/pi/map_kernel.c b/arch/arm64/kernel/pi/map_kernel.c index f374a3e5a5fe..e57b043f324b 100644 --- a/arch/arm64/kernel/pi/map_kernel.c +++ b/arch/arm64/kernel/pi/map_kernel.c @@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) { u64 sctlr = read_sysreg(sctlr_el1); u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,ID_AA64MMFR0_EL1_PARANGE_SHIFT);tcr &= ~TCR_IPS_MASK;Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.
As explained above, TCR.IPS will be capped at 48 bits up to this point.
Alright
tcr |= parange << TCR_IPS_SHIFT;Wondering if FIELD_PREP() could be used here.
AIUI we'd end up with
tcr &= ~TCR_IPS_MASK; tcr |= FIELD_PREP(TCR_IPS_MASK, parange);
Is that really so much better?
Not really, can be left unchanged.
asm(" msr sctlr_el1, %0 ;" " isb ;"diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c index d21f67d67cf5..4db9887b2aef 100644 --- a/arch/arm64/mm/init.c +++ b/arch/arm64/mm/init.c @@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) { extern u16 memstart_offset_seed;
u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);Could this have a comment explaining the need for sanitized value ?
Sure. Actually, it shouldn't make any difference here (unless we allow PARange to be narrowed even further, which might make sense if we care about enabling randomization of the linear map on systems where PARange is much larger than the size of the physical address space that is actually populated). However, for consistency, it is better to avoid the 52-bit PARange if LPA2 is disabled.
Got it, thanks for the explanation.
int parange = cpuid_feature_extract_unsigned_field( mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT); s64 range = linear_region_size -Otherwise LGTM.
Thanks!
linux-stable-mirror@lists.linaro.org
-
Anshuman Khandual -
Ard Biesheuvel -
Ard Biesheuvel