Message ID | 1529403502-2843-2-git-send-email-vladimir.murzin@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Vladimir, On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: > diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h > index 39ec0b8..c506fb7 100644 > --- a/arch/arm64/include/asm/mmu_context.h > +++ b/arch/arm64/include/asm/mmu_context.h > @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > > phys_addr_t pgd_phys = virt_to_phys(pgdp); > > + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { > + /* > + * cpu_replace_ttbr1() is used when there's a boot CPU > + * up (i.e. cpufeature framework is not up yet) and > + * latter only when we enable CNP via cpufeature's > + * enable() callback. > + * Also we rely on the cpu_hwcap bit being set before > + * calling the enable() function. > + */ > + pgd_phys |= TTBR_CNP_BIT; > + } > + > replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it performs a phys_to_ttbr transformation of pgd_phys which masks out the bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need to tweak TTBR_BADDR_MASK_52 to start from bit 0. (cc'ing Kristina as she added this code, in case there is any issue with extending the mask)
On 27/07/18 12:35, Catalin Marinas wrote: > Hi Vladimir, > > On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: >> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h >> index 39ec0b8..c506fb7 100644 >> --- a/arch/arm64/include/asm/mmu_context.h >> +++ b/arch/arm64/include/asm/mmu_context.h >> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) >> >> phys_addr_t pgd_phys = virt_to_phys(pgdp); >> >> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { >> + /* >> + * cpu_replace_ttbr1() is used when there's a boot CPU >> + * up (i.e. cpufeature framework is not up yet) and >> + * latter only when we enable CNP via cpufeature's >> + * enable() callback. >> + * Also we rely on the cpu_hwcap bit being set before >> + * calling the enable() function. >> + */ >> + pgd_phys |= TTBR_CNP_BIT; >> + } >> + >> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); > > So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls > the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it > performs a phys_to_ttbr transformation of pgd_phys which masks out the > bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need > to tweak TTBR_BADDR_MASK_52 to start from bit 0. > > (cc'ing Kristina as she added this code, in case there is any issue with > extending the mask) > Something like bellow? diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 0bcc98d..e0b4b2f 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -524,11 +524,10 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU * ttbr: returns the TTBR value */ .macro phys_to_ttbr, ttbr, phys -#ifdef CONFIG_ARM64_PA_BITS_52 - orr \ttbr, \phys, \phys, lsr #46 - and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 -#else mov \ttbr, \phys +#ifdef CONFIG_ARM64_PA_BITS_52 + ubfx \ttbr, \ttbr, #48, #4 + orr \ttbr, \phys, \ttbr, lsl #2 #endif .endm diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h index 1bdeca8..1b9d0e9 100644 --- a/arch/arm64/include/asm/pgtable.h +++ b/arch/arm64/include/asm/pgtable.h @@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, #define kc_offset_to_vaddr(o) ((o) | VA_START) #ifdef CONFIG_ARM64_PA_BITS_52 -#define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) +#define phys_to_ttbr(addr) ((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52)) #else #define phys_to_ttbr(addr) (addr) #endif Vladimir
On Mon, Jul 30, 2018 at 11:08:27AM +0100, Vladimir Murzin wrote: > On 27/07/18 12:35, Catalin Marinas wrote: > > On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: > >> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h > >> index 39ec0b8..c506fb7 100644 > >> --- a/arch/arm64/include/asm/mmu_context.h > >> +++ b/arch/arm64/include/asm/mmu_context.h > >> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > >> > >> phys_addr_t pgd_phys = virt_to_phys(pgdp); > >> > >> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { > >> + /* > >> + * cpu_replace_ttbr1() is used when there's a boot CPU > >> + * up (i.e. cpufeature framework is not up yet) and > >> + * latter only when we enable CNP via cpufeature's > >> + * enable() callback. > >> + * Also we rely on the cpu_hwcap bit being set before > >> + * calling the enable() function. > >> + */ > >> + pgd_phys |= TTBR_CNP_BIT; > >> + } > >> + > >> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); > > > > So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls > > the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it > > performs a phys_to_ttbr transformation of pgd_phys which masks out the > > bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need > > to tweak TTBR_BADDR_MASK_52 to start from bit 0. > > Something like bellow? > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > index 0bcc98d..e0b4b2f 100644 > --- a/arch/arm64/include/asm/assembler.h > +++ b/arch/arm64/include/asm/assembler.h > @@ -524,11 +524,10 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU > * ttbr: returns the TTBR value > */ > .macro phys_to_ttbr, ttbr, phys > -#ifdef CONFIG_ARM64_PA_BITS_52 > - orr \ttbr, \phys, \phys, lsr #46 > - and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 > -#else > mov \ttbr, \phys > +#ifdef CONFIG_ARM64_PA_BITS_52 > + ubfx \ttbr, \ttbr, #48, #4 > + orr \ttbr, \phys, \ttbr, lsl #2 > #endif > .endm This would do, I don't have a better idea on how to write it. But I'd like a comment to say that this is moving bits 51:48 of the address to bits 5:2 of TTBR_ELx. > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > index 1bdeca8..1b9d0e9 100644 > --- a/arch/arm64/include/asm/pgtable.h > +++ b/arch/arm64/include/asm/pgtable.h > @@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > #define kc_offset_to_vaddr(o) ((o) | VA_START) > > #ifdef CONFIG_ARM64_PA_BITS_52 > -#define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) > +#define phys_to_ttbr(addr) ((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52)) With these changes: Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> Please repost the updated patches and cc Will as he's handling the upcoming merging window. Thanks.
On 06/19/2018 11:18 AM, Vladimir Murzin wrote: > Common Not Private (CNP) is a feature of ARMv8.2 extension which > allows translation table entries to be shared between different PEs in > the same inner shareable domain, so the hardware can use this fact to > optimise the caching of such entries in the TLB. > > CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to > the hardware that the translation table entries pointed to by this > TTBR are the same as every PE in the same inner shareable domain for > which the equivalent TTBR also has CNP bit set. In case CNP bit is set > but TTBR does not point at the same translation table entries for a > given ASID and VMID, then the system is mis-configured, so the results > of translations are UNPREDICTABLE. > > For kernel we postpone setting CNP till all cpus are up and rely on > cpufeature framework to 1) patch the code which is sensitive to CNP > and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be > reprogrammed as result of hibernation or cpuidle (via __enable_mmu). > For these two cases we restore CNP bit via __cpu_suspend_exit(). > > There are a few cases we need to care of changes in TTBR0_EL1: > - a switch to idmap > - software emulated PAN > > we rule out latter via Kconfig options and for the former we make > sure that CNP is set for non-zero ASIDs only. > > Reviewed-by: James Morse <james.morse@arm.com> > Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> > --- > arch/arm64/Kconfig | 13 +++++++++++++ > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/cpufeature.h | 6 ++++++ > arch/arm64/include/asm/mmu_context.h | 12 ++++++++++++ > arch/arm64/include/asm/pgtable-hwdef.h | 2 ++ > arch/arm64/kernel/cpufeature.c | 35 ++++++++++++++++++++++++++++++++++ > arch/arm64/kernel/suspend.c | 4 ++++ > arch/arm64/mm/context.c | 3 +++ > arch/arm64/mm/proc.S | 6 ++++++ > 9 files changed, 83 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 42c090c..70a4ad3 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -1124,6 +1124,19 @@ config ARM64_RAS_EXTN > and access the new registers if the system supports the extension. > Platform RAS features may additionally depend on firmware support. > > +config ARM64_CNP > + bool "Enable support for Common Not Private (CNP) translations" > + depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN > + help > + Common Not Private (CNP) allows translation table entries to > + be shared between different PEs in the same inner shareable > + domain, so the hardware can use this fact to optimise the > + caching of such entries in the TLB. > + > + Selecting this option allows the CNP feature to be detected > + at runtime, and does not affect PEs that do not implement > + this feature. > + > endmenu > > config ARM64_SVE > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > index 8a699c7..7219654 100644 > --- a/arch/arm64/include/asm/cpucaps.h > +++ b/arch/arm64/include/asm/cpucaps.h > @@ -49,7 +49,8 @@ > #define ARM64_HAS_CACHE_DIC 28 > #define ARM64_HW_DBM 29 > #define ARM64_SSBD 30 > +#define ARM64_HAS_CNP 31 > > -#define ARM64_NCAPS 31 > +#define ARM64_NCAPS 32 > > #endif /* __ASM_CPUCAPS_H */ > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 1717ba1..b5e2440 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -508,6 +508,12 @@ static inline bool system_supports_sve(void) > cpus_have_const_cap(ARM64_SVE); > } > > +static inline bool system_supports_cnp(void) > +{ > + return IS_ENABLED(CONFIG_ARM64_CNP) && > + cpus_have_const_cap(ARM64_HAS_CNP); > +} > + > #define ARM64_SSBD_UNKNOWN -1 > #define ARM64_SSBD_FORCE_DISABLE 0 > #define ARM64_SSBD_KERNEL 1 > diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h > index 39ec0b8..c506fb7 100644 > --- a/arch/arm64/include/asm/mmu_context.h > +++ b/arch/arm64/include/asm/mmu_context.h > @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > > phys_addr_t pgd_phys = virt_to_phys(pgdp); > > + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { > + /* > + * cpu_replace_ttbr1() is used when there's a boot CPU > + * up (i.e. cpufeature framework is not up yet) and > + * latter only when we enable CNP via cpufeature's > + * enable() callback. > + * Also we rely on the cpu_hwcap bit being set before > + * calling the enable() function. > + */ > + pgd_phys |= TTBR_CNP_BIT; > + } > + > replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); > > cpu_install_idmap(); > diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h > index fd208ea..1d7d8da 100644 > --- a/arch/arm64/include/asm/pgtable-hwdef.h > +++ b/arch/arm64/include/asm/pgtable-hwdef.h > @@ -211,6 +211,8 @@ > #define PHYS_MASK_SHIFT (CONFIG_ARM64_PA_BITS) > #define PHYS_MASK ((UL(1) << PHYS_MASK_SHIFT) - 1) > > +#define TTBR_CNP_BIT (UL(1) << 0) > + > /* > * TCR flags. > */ > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c > index d2856b1..e0129f2 100644 > --- a/arch/arm64/kernel/cpufeature.c > +++ b/arch/arm64/kernel/cpufeature.c > @@ -20,6 +20,7 @@ > > #include <linux/bsearch.h> > #include <linux/cpumask.h> > +#include <linux/crash_dump.h> > #include <linux/sort.h> > #include <linux/stop_machine.h> > #include <linux/types.h> > @@ -117,6 +118,7 @@ EXPORT_SYMBOL(cpu_hwcap_keys); > static bool __maybe_unused > cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused); > > +static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap); > > /* > * NOTE: Any changes to the visibility of features should be kept in > @@ -858,6 +860,21 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry, > return read_sanitised_ftr_reg(SYS_CTR_EL0) & BIT(CTR_DIC_SHIFT); > } > > +static bool __maybe_unused > +has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope) > +{ > +#ifdef CONFIG_CRASH_DUMP > + /* > + * Kdump isn't guaranteed to power-off all secondary CPUs, CNP > + * may share TLB entries with a CPU stuck in the crashed > + * kernel. > + */ > + if (elfcorehdr_size) > + return false; > +#endif Vladimir, If you are respinning with the changes mentioned by Catalin, could we use is_kdump_kernel() instead of relying on the variable ? Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> Suzuki
On 30/07/18 16:42, Catalin Marinas wrote: > On Mon, Jul 30, 2018 at 11:08:27AM +0100, Vladimir Murzin wrote: >> On 27/07/18 12:35, Catalin Marinas wrote: >>> On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: >>>> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h >>>> index 39ec0b8..c506fb7 100644 >>>> --- a/arch/arm64/include/asm/mmu_context.h >>>> +++ b/arch/arm64/include/asm/mmu_context.h >>>> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) >>>> >>>> phys_addr_t pgd_phys = virt_to_phys(pgdp); >>>> >>>> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { >>>> + /* >>>> + * cpu_replace_ttbr1() is used when there's a boot CPU >>>> + * up (i.e. cpufeature framework is not up yet) and >>>> + * latter only when we enable CNP via cpufeature's >>>> + * enable() callback. >>>> + * Also we rely on the cpu_hwcap bit being set before >>>> + * calling the enable() function. >>>> + */ >>>> + pgd_phys |= TTBR_CNP_BIT; >>>> + } >>>> + >>>> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); >>> >>> So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls >>> the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it >>> performs a phys_to_ttbr transformation of pgd_phys which masks out the >>> bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need >>> to tweak TTBR_BADDR_MASK_52 to start from bit 0. >> >> Something like bellow? >> >> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h >> index 0bcc98d..e0b4b2f 100644 >> --- a/arch/arm64/include/asm/assembler.h >> +++ b/arch/arm64/include/asm/assembler.h >> @@ -524,11 +524,10 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU >> * ttbr: returns the TTBR value >> */ >> .macro phys_to_ttbr, ttbr, phys >> -#ifdef CONFIG_ARM64_PA_BITS_52 >> - orr \ttbr, \phys, \phys, lsr #46 >> - and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 >> -#else >> mov \ttbr, \phys >> +#ifdef CONFIG_ARM64_PA_BITS_52 >> + ubfx \ttbr, \ttbr, #48, #4 >> + orr \ttbr, \phys, \ttbr, lsl #2 >> #endif >> .endm > > This would do, I don't have a better idea on how to write it. But I'd > like a comment to say that this is moving bits 51:48 of the address to > bits 5:2 of TTBR_ELx. The diff above is *copying* 51:48 into 5:2; it doesn't *move* it like the existing code does. That's pretty crucial, because without the BADDR masking operation it's going to leave the upper address bits in the ASID/VMID field, which looks like a recipe for disaster if the reserved TTBR1 happens to be at a high enough address (at least cpu_switch_mm might just be OK by virtue of using BFI rather than ORR for the ASID). >> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >> index 1bdeca8..1b9d0e9 100644 >> --- a/arch/arm64/include/asm/pgtable.h >> +++ b/arch/arm64/include/asm/pgtable.h >> @@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, >> #define kc_offset_to_vaddr(o) ((o) | VA_START) >> >> #ifdef CONFIG_ARM64_PA_BITS_52 >> -#define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) >> +#define phys_to_ttbr(addr) ((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52)) Ditto - by the look of things, this definitely stands to corrupt the VMID in update_vttbr(). If we really have no choice but to smuggle the CnP value in something which is logically an address, then I think we'd need to handle it more like this: #define TTBR_CNP (1) #define phys_to_ttbr(addr) \ ((((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) | (addr & TTBR_CNP)) Although in patch #2 we're only applying CnP *after* the BADDR conversion, so is changing this one even necessary? If I've understood the intent correctly, all that might be needed is something like the below (untested, of course). Robin. ----->8----- diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h index 0bcc98dbba56..5b0f8f80238f 100644 --- a/arch/arm64/include/asm/assembler.h +++ b/arch/arm64/include/asm/assembler.h @@ -527,6 +527,7 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU #ifdef CONFIG_ARM64_PA_BITS_52 orr \ttbr, \phys, \phys, lsr #46 and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 + bfi \ttbr, \addr, #0, #1 // CnP #else mov \ttbr, \phys #endif
On Mon, Jul 30, 2018 at 05:29:35PM +0100, Robin Murphy wrote: > On 30/07/18 16:42, Catalin Marinas wrote: > > On Mon, Jul 30, 2018 at 11:08:27AM +0100, Vladimir Murzin wrote: > > > On 27/07/18 12:35, Catalin Marinas wrote: > > > > On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: > > > > > diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h > > > > > index 39ec0b8..c506fb7 100644 > > > > > --- a/arch/arm64/include/asm/mmu_context.h > > > > > +++ b/arch/arm64/include/asm/mmu_context.h > > > > > @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > > > > > phys_addr_t pgd_phys = virt_to_phys(pgdp); > > > > > + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { > > > > > + /* > > > > > + * cpu_replace_ttbr1() is used when there's a boot CPU > > > > > + * up (i.e. cpufeature framework is not up yet) and > > > > > + * latter only when we enable CNP via cpufeature's > > > > > + * enable() callback. > > > > > + * Also we rely on the cpu_hwcap bit being set before > > > > > + * calling the enable() function. > > > > > + */ > > > > > + pgd_phys |= TTBR_CNP_BIT; > > > > > + } > > > > > + > > > > > replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); > > > > > > > > So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls > > > > the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it > > > > performs a phys_to_ttbr transformation of pgd_phys which masks out the > > > > bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need > > > > to tweak TTBR_BADDR_MASK_52 to start from bit 0. > > > > > > Something like bellow? > > > > > > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h > > > index 0bcc98d..e0b4b2f 100644 > > > --- a/arch/arm64/include/asm/assembler.h > > > +++ b/arch/arm64/include/asm/assembler.h > > > @@ -524,11 +524,10 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU > > > * ttbr: returns the TTBR value > > > */ > > > .macro phys_to_ttbr, ttbr, phys > > > -#ifdef CONFIG_ARM64_PA_BITS_52 > > > - orr \ttbr, \phys, \phys, lsr #46 > > > - and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 > > > -#else > > > mov \ttbr, \phys > > > +#ifdef CONFIG_ARM64_PA_BITS_52 > > > + ubfx \ttbr, \ttbr, #48, #4 > > > + orr \ttbr, \phys, \ttbr, lsl #2 > > > #endif > > > .endm > > > > This would do, I don't have a better idea on how to write it. But I'd > > like a comment to say that this is moving bits 51:48 of the address to > > bits 5:2 of TTBR_ELx. > > The diff above is *copying* 51:48 into 5:2; it doesn't *move* it like the > existing code does. That's pretty crucial, because without the BADDR masking > operation it's going to leave the upper address bits in the ASID/VMID field, > which looks like a recipe for disaster if the reserved TTBR1 happens to be > at a high enough address (at least cpu_switch_mm might just be OK by virtue > of using BFI rather than ORR for the ASID). Oh, very good point. > > > diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h > > > index 1bdeca8..1b9d0e9 100644 > > > --- a/arch/arm64/include/asm/pgtable.h > > > +++ b/arch/arm64/include/asm/pgtable.h > > > @@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, > > > #define kc_offset_to_vaddr(o) ((o) | VA_START) > > > #ifdef CONFIG_ARM64_PA_BITS_52 > > > -#define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) > > > +#define phys_to_ttbr(addr) ((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52)) > > Ditto - by the look of things, this definitely stands to corrupt the VMID in > update_vttbr(). If we really have no choice but to smuggle the CnP value in > something which is logically an address, then I think we'd need to handle it > more like this: > > #define TTBR_CNP (1) > #define phys_to_ttbr(addr) \ > ((((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) | (addr & TTBR_CNP)) > > Although in patch #2 we're only applying CnP *after* the BADDR conversion, > so is changing this one even necessary? If I've understood the intent > correctly, all that might be needed is something like the below (untested, > of course). Or we could keep phys_to_ttbr() as it is for both the asm and C and apply the CNP bit afterwards (as you've already noticed in patch 2). For cpu_replace_ttbr1(), we'd also need to change idmap_cpu_replace_ttbr1 to actually take the TTBR1_EL1 value directly so that it doesn't have to invoke phys_to_ttbr().
On 30/07/18 18:03, Catalin Marinas wrote: > On Mon, Jul 30, 2018 at 05:29:35PM +0100, Robin Murphy wrote: >> On 30/07/18 16:42, Catalin Marinas wrote: >>> On Mon, Jul 30, 2018 at 11:08:27AM +0100, Vladimir Murzin wrote: >>>> On 27/07/18 12:35, Catalin Marinas wrote: >>>>> On Tue, Jun 19, 2018 at 11:18:20AM +0100, Vladimir Murzin wrote: >>>>>> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h >>>>>> index 39ec0b8..c506fb7 100644 >>>>>> --- a/arch/arm64/include/asm/mmu_context.h >>>>>> +++ b/arch/arm64/include/asm/mmu_context.h >>>>>> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) >>>>>> phys_addr_t pgd_phys = virt_to_phys(pgdp); >>>>>> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { >>>>>> + /* >>>>>> + * cpu_replace_ttbr1() is used when there's a boot CPU >>>>>> + * up (i.e. cpufeature framework is not up yet) and >>>>>> + * latter only when we enable CNP via cpufeature's >>>>>> + * enable() callback. >>>>>> + * Also we rely on the cpu_hwcap bit being set before >>>>>> + * calling the enable() function. >>>>>> + */ >>>>>> + pgd_phys |= TTBR_CNP_BIT; >>>>>> + } >>>>>> + >>>>>> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); >>>>> >>>>> So the above code sets the TTBR_CNP_BIT (bit 0) in pgd_phys and calls >>>>> the idmap_cpu_replace_ttbr1() with this value. Looking at the latter, it >>>>> performs a phys_to_ttbr transformation of pgd_phys which masks out the >>>>> bottom 2 bits when CONFIG_ARM64_PA_BITS_52 is enabled. I think we need >>>>> to tweak TTBR_BADDR_MASK_52 to start from bit 0. >>>> >>>> Something like bellow? >>>> >>>> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h >>>> index 0bcc98d..e0b4b2f 100644 >>>> --- a/arch/arm64/include/asm/assembler.h >>>> +++ b/arch/arm64/include/asm/assembler.h >>>> @@ -524,11 +524,10 @@ USER(\label, ic ivau, \tmp2) // invalidate I line PoU >>>> * ttbr: returns the TTBR value >>>> */ >>>> .macro phys_to_ttbr, ttbr, phys >>>> -#ifdef CONFIG_ARM64_PA_BITS_52 >>>> - orr \ttbr, \phys, \phys, lsr #46 >>>> - and \ttbr, \ttbr, #TTBR_BADDR_MASK_52 >>>> -#else >>>> mov \ttbr, \phys >>>> +#ifdef CONFIG_ARM64_PA_BITS_52 >>>> + ubfx \ttbr, \ttbr, #48, #4 >>>> + orr \ttbr, \phys, \ttbr, lsl #2 >>>> #endif >>>> .endm >>> >>> This would do, I don't have a better idea on how to write it. But I'd >>> like a comment to say that this is moving bits 51:48 of the address to >>> bits 5:2 of TTBR_ELx. >> >> The diff above is *copying* 51:48 into 5:2; it doesn't *move* it like the >> existing code does. That's pretty crucial, because without the BADDR masking >> operation it's going to leave the upper address bits in the ASID/VMID field, >> which looks like a recipe for disaster if the reserved TTBR1 happens to be >> at a high enough address (at least cpu_switch_mm might just be OK by virtue >> of using BFI rather than ORR for the ASID). > > Oh, very good point. > >>>> diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h >>>> index 1bdeca8..1b9d0e9 100644 >>>> --- a/arch/arm64/include/asm/pgtable.h >>>> +++ b/arch/arm64/include/asm/pgtable.h >>>> @@ -770,7 +770,7 @@ static inline void update_mmu_cache(struct vm_area_struct *vma, >>>> #define kc_offset_to_vaddr(o) ((o) | VA_START) >>>> #ifdef CONFIG_ARM64_PA_BITS_52 >>>> -#define phys_to_ttbr(addr) (((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) >>>> +#define phys_to_ttbr(addr) ((addr) | (((addr) >> 46) & TTBR_BADDR_MASK_52)) >> >> Ditto - by the look of things, this definitely stands to corrupt the VMID in >> update_vttbr(). If we really have no choice but to smuggle the CnP value in >> something which is logically an address, then I think we'd need to handle it >> more like this: >> >> #define TTBR_CNP (1) >> #define phys_to_ttbr(addr) \ >> ((((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52) | (addr & TTBR_CNP)) >> >> Although in patch #2 we're only applying CnP *after* the BADDR conversion, >> so is changing this one even necessary? If I've understood the intent >> correctly, all that might be needed is something like the below (untested, >> of course). > > Or we could keep phys_to_ttbr() as it is for both the asm and C and > apply the CNP bit afterwards (as you've already noticed in patch 2). For > cpu_replace_ttbr1(), we'd also need to change idmap_cpu_replace_ttbr1 to > actually take the TTBR1_EL1 value directly so that it doesn't have to > invoke phys_to_ttbr(). > Does it represent your idea correctly? diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h index c506fb7..b6ba2f0 100644 --- a/arch/arm64/include/asm/mmu_context.h +++ b/arch/arm64/include/asm/mmu_context.h @@ -147,7 +147,8 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) extern ttbr_replace_func idmap_cpu_replace_ttbr1; ttbr_replace_func *replace_phys; - phys_addr_t pgd_phys = virt_to_phys(pgdp); + /* phys_to_ttbr() zeros lower 2 bits of ttbr with 52-bit PA */ + phys_addr_t ttbr1 = phys_to_ttbr(virt_to_phys(pgdp)); if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { /* @@ -158,13 +159,13 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) * Also we rely on the cpu_hwcap bit being set before * calling the enable() function. */ - pgd_phys |= TTBR_CNP_BIT; + ttbr1 |= TTBR_CNP_BIT; } replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); cpu_install_idmap(); - replace_phys(pgd_phys); + replace_phys(ttbr1); cpu_uninstall_idmap(); } diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index bde3a3b..2c75b0b 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -190,7 +190,7 @@ ENDPROC(cpu_do_switch_mm) .endm /* - * void idmap_cpu_replace_ttbr1(phys_addr_t new_pgd) + * void idmap_cpu_replace_ttbr1(phys_addr_t ttbr1) * * This is the low-level counterpart to cpu_replace_ttbr1, and should not be * called by anything else. It can only be executed from a TTBR0 mapping. @@ -200,8 +200,7 @@ ENTRY(idmap_cpu_replace_ttbr1) __idmap_cpu_set_reserved_ttbr1 x1, x3 - phys_to_ttbr x3, x0 - msr ttbr1_el1, x3 + msr ttbr1_el1, x0 isb restore_daif x2
On 30/07/18 17:24, Suzuki K Poulose wrote: > On 06/19/2018 11:18 AM, Vladimir Murzin wrote: >> Common Not Private (CNP) is a feature of ARMv8.2 extension which >> allows translation table entries to be shared between different PEs in >> the same inner shareable domain, so the hardware can use this fact to >> optimise the caching of such entries in the TLB. >> >> CNP occupies one bit in TTBRx_ELy and VTTBR_EL2, which advertises to >> the hardware that the translation table entries pointed to by this >> TTBR are the same as every PE in the same inner shareable domain for >> which the equivalent TTBR also has CNP bit set. In case CNP bit is set >> but TTBR does not point at the same translation table entries for a >> given ASID and VMID, then the system is mis-configured, so the results >> of translations are UNPREDICTABLE. >> >> For kernel we postpone setting CNP till all cpus are up and rely on >> cpufeature framework to 1) patch the code which is sensitive to CNP >> and 2) update TTBR1_EL1 with CNP bit set. TTBR1_EL1 can be >> reprogrammed as result of hibernation or cpuidle (via __enable_mmu). >> For these two cases we restore CNP bit via __cpu_suspend_exit(). >> >> There are a few cases we need to care of changes in TTBR0_EL1: >> - a switch to idmap >> - software emulated PAN >> >> we rule out latter via Kconfig options and for the former we make >> sure that CNP is set for non-zero ASIDs only. >> >> Reviewed-by: James Morse <james.morse@arm.com> >> Signed-off-by: Vladimir Murzin <vladimir.murzin@arm.com> >> --- >> arch/arm64/Kconfig | 13 +++++++++++++ >> arch/arm64/include/asm/cpucaps.h | 3 ++- >> arch/arm64/include/asm/cpufeature.h | 6 ++++++ >> arch/arm64/include/asm/mmu_context.h | 12 ++++++++++++ >> arch/arm64/include/asm/pgtable-hwdef.h | 2 ++ >> arch/arm64/kernel/cpufeature.c | 35 ++++++++++++++++++++++++++++++++++ >> arch/arm64/kernel/suspend.c | 4 ++++ >> arch/arm64/mm/context.c | 3 +++ >> arch/arm64/mm/proc.S | 6 ++++++ >> 9 files changed, 83 insertions(+), 1 deletion(-) >> >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 42c090c..70a4ad3 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -1124,6 +1124,19 @@ config ARM64_RAS_EXTN >> and access the new registers if the system supports the extension. >> Platform RAS features may additionally depend on firmware support. >> +config ARM64_CNP >> + bool "Enable support for Common Not Private (CNP) translations" >> + depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN >> + help >> + Common Not Private (CNP) allows translation table entries to >> + be shared between different PEs in the same inner shareable >> + domain, so the hardware can use this fact to optimise the >> + caching of such entries in the TLB. >> + >> + Selecting this option allows the CNP feature to be detected >> + at runtime, and does not affect PEs that do not implement >> + this feature. >> + >> endmenu >> config ARM64_SVE >> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h >> index 8a699c7..7219654 100644 >> --- a/arch/arm64/include/asm/cpucaps.h >> +++ b/arch/arm64/include/asm/cpucaps.h >> @@ -49,7 +49,8 @@ >> #define ARM64_HAS_CACHE_DIC 28 >> #define ARM64_HW_DBM 29 >> #define ARM64_SSBD 30 >> +#define ARM64_HAS_CNP 31 >> -#define ARM64_NCAPS 31 >> +#define ARM64_NCAPS 32 >> #endif /* __ASM_CPUCAPS_H */ >> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h >> index 1717ba1..b5e2440 100644 >> --- a/arch/arm64/include/asm/cpufeature.h >> +++ b/arch/arm64/include/asm/cpufeature.h >> @@ -508,6 +508,12 @@ static inline bool system_supports_sve(void) >> cpus_have_const_cap(ARM64_SVE); >> } >> +static inline bool system_supports_cnp(void) >> +{ >> + return IS_ENABLED(CONFIG_ARM64_CNP) && >> + cpus_have_const_cap(ARM64_HAS_CNP); >> +} >> + >> #define ARM64_SSBD_UNKNOWN -1 >> #define ARM64_SSBD_FORCE_DISABLE 0 >> #define ARM64_SSBD_KERNEL 1 >> diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h >> index 39ec0b8..c506fb7 100644 >> --- a/arch/arm64/include/asm/mmu_context.h >> +++ b/arch/arm64/include/asm/mmu_context.h >> @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) >> phys_addr_t pgd_phys = virt_to_phys(pgdp); >> + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { >> + /* >> + * cpu_replace_ttbr1() is used when there's a boot CPU >> + * up (i.e. cpufeature framework is not up yet) and >> + * latter only when we enable CNP via cpufeature's >> + * enable() callback. >> + * Also we rely on the cpu_hwcap bit being set before >> + * calling the enable() function. >> + */ >> + pgd_phys |= TTBR_CNP_BIT; >> + } >> + >> replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); >> cpu_install_idmap(); >> diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h >> index fd208ea..1d7d8da 100644 >> --- a/arch/arm64/include/asm/pgtable-hwdef.h >> +++ b/arch/arm64/include/asm/pgtable-hwdef.h >> @@ -211,6 +211,8 @@ >> #define PHYS_MASK_SHIFT (CONFIG_ARM64_PA_BITS) >> #define PHYS_MASK ((UL(1) << PHYS_MASK_SHIFT) - 1) >> +#define TTBR_CNP_BIT (UL(1) << 0) >> + >> /* >> * TCR flags. >> */ >> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c >> index d2856b1..e0129f2 100644 >> --- a/arch/arm64/kernel/cpufeature.c >> +++ b/arch/arm64/kernel/cpufeature.c >> @@ -20,6 +20,7 @@ >> #include <linux/bsearch.h> >> #include <linux/cpumask.h> >> +#include <linux/crash_dump.h> >> #include <linux/sort.h> >> #include <linux/stop_machine.h> >> #include <linux/types.h> >> @@ -117,6 +118,7 @@ EXPORT_SYMBOL(cpu_hwcap_keys); >> static bool __maybe_unused >> cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused); >> +static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap); >> /* >> * NOTE: Any changes to the visibility of features should be kept in >> @@ -858,6 +860,21 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry, >> return read_sanitised_ftr_reg(SYS_CTR_EL0) & BIT(CTR_DIC_SHIFT); >> } >> +static bool __maybe_unused >> +has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope) >> +{ >> +#ifdef CONFIG_CRASH_DUMP >> + /* >> + * Kdump isn't guaranteed to power-off all secondary CPUs, CNP >> + * may share TLB entries with a CPU stuck in the crashed >> + * kernel. >> + */ >> + if (elfcorehdr_size) >> + return false; >> +#endif > > Vladimir, > > If you are respinning with the changes mentioned by Catalin, could we use is_kdump_kernel() instead of relying on the variable ? Sure, it looks much better! Thanks for suggestion! > > Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com> Cheers Vladimir > > Suzuki >
On Tue, Jul 31, 2018 at 11:17:30AM +0100, Vladimir Murzin wrote: > Does it represent your idea correctly? > > diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h > index c506fb7..b6ba2f0 100644 > --- a/arch/arm64/include/asm/mmu_context.h > +++ b/arch/arm64/include/asm/mmu_context.h > @@ -147,7 +147,8 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > extern ttbr_replace_func idmap_cpu_replace_ttbr1; > ttbr_replace_func *replace_phys; > > - phys_addr_t pgd_phys = virt_to_phys(pgdp); > + /* phys_to_ttbr() zeros lower 2 bits of ttbr with 52-bit PA */ > + phys_addr_t ttbr1 = phys_to_ttbr(virt_to_phys(pgdp)); > > if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { > /* > @@ -158,13 +159,13 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) > * Also we rely on the cpu_hwcap bit being set before > * calling the enable() function. > */ > - pgd_phys |= TTBR_CNP_BIT; > + ttbr1 |= TTBR_CNP_BIT; > } > > replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); > > cpu_install_idmap(); > - replace_phys(pgd_phys); > + replace_phys(ttbr1); > cpu_uninstall_idmap(); > } > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > index bde3a3b..2c75b0b 100644 > --- a/arch/arm64/mm/proc.S > +++ b/arch/arm64/mm/proc.S > @@ -190,7 +190,7 @@ ENDPROC(cpu_do_switch_mm) > .endm > > /* > - * void idmap_cpu_replace_ttbr1(phys_addr_t new_pgd) > + * void idmap_cpu_replace_ttbr1(phys_addr_t ttbr1) > * > * This is the low-level counterpart to cpu_replace_ttbr1, and should not be > * called by anything else. It can only be executed from a TTBR0 mapping. > @@ -200,8 +200,7 @@ ENTRY(idmap_cpu_replace_ttbr1) > > __idmap_cpu_set_reserved_ttbr1 x1, x3 > > - phys_to_ttbr x3, x0 > - msr ttbr1_el1, x3 > + msr ttbr1_el1, x0 > isb > > restore_daif x2 It looks fine. Thanks.
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 42c090c..70a4ad3 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -1124,6 +1124,19 @@ config ARM64_RAS_EXTN and access the new registers if the system supports the extension. Platform RAS features may additionally depend on firmware support. +config ARM64_CNP + bool "Enable support for Common Not Private (CNP) translations" + depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN + help + Common Not Private (CNP) allows translation table entries to + be shared between different PEs in the same inner shareable + domain, so the hardware can use this fact to optimise the + caching of such entries in the TLB. + + Selecting this option allows the CNP feature to be detected + at runtime, and does not affect PEs that do not implement + this feature. + endmenu config ARM64_SVE diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 8a699c7..7219654 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -49,7 +49,8 @@ #define ARM64_HAS_CACHE_DIC 28 #define ARM64_HW_DBM 29 #define ARM64_SSBD 30 +#define ARM64_HAS_CNP 31 -#define ARM64_NCAPS 31 +#define ARM64_NCAPS 32 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 1717ba1..b5e2440 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -508,6 +508,12 @@ static inline bool system_supports_sve(void) cpus_have_const_cap(ARM64_SVE); } +static inline bool system_supports_cnp(void) +{ + return IS_ENABLED(CONFIG_ARM64_CNP) && + cpus_have_const_cap(ARM64_HAS_CNP); +} + #define ARM64_SSBD_UNKNOWN -1 #define ARM64_SSBD_FORCE_DISABLE 0 #define ARM64_SSBD_KERNEL 1 diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h index 39ec0b8..c506fb7 100644 --- a/arch/arm64/include/asm/mmu_context.h +++ b/arch/arm64/include/asm/mmu_context.h @@ -149,6 +149,18 @@ static inline void cpu_replace_ttbr1(pgd_t *pgdp) phys_addr_t pgd_phys = virt_to_phys(pgdp); + if (system_supports_cnp() && !WARN_ON(pgdp != lm_alias(swapper_pg_dir))) { + /* + * cpu_replace_ttbr1() is used when there's a boot CPU + * up (i.e. cpufeature framework is not up yet) and + * latter only when we enable CNP via cpufeature's + * enable() callback. + * Also we rely on the cpu_hwcap bit being set before + * calling the enable() function. + */ + pgd_phys |= TTBR_CNP_BIT; + } + replace_phys = (void *)__pa_symbol(idmap_cpu_replace_ttbr1); cpu_install_idmap(); diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h index fd208ea..1d7d8da 100644 --- a/arch/arm64/include/asm/pgtable-hwdef.h +++ b/arch/arm64/include/asm/pgtable-hwdef.h @@ -211,6 +211,8 @@ #define PHYS_MASK_SHIFT (CONFIG_ARM64_PA_BITS) #define PHYS_MASK ((UL(1) << PHYS_MASK_SHIFT) - 1) +#define TTBR_CNP_BIT (UL(1) << 0) + /* * TCR flags. */ diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index d2856b1..e0129f2 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -20,6 +20,7 @@ #include <linux/bsearch.h> #include <linux/cpumask.h> +#include <linux/crash_dump.h> #include <linux/sort.h> #include <linux/stop_machine.h> #include <linux/types.h> @@ -117,6 +118,7 @@ EXPORT_SYMBOL(cpu_hwcap_keys); static bool __maybe_unused cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused); +static void cpu_enable_cnp(struct arm64_cpu_capabilities const *cap); /* * NOTE: Any changes to the visibility of features should be kept in @@ -858,6 +860,21 @@ static bool has_cache_dic(const struct arm64_cpu_capabilities *entry, return read_sanitised_ftr_reg(SYS_CTR_EL0) & BIT(CTR_DIC_SHIFT); } +static bool __maybe_unused +has_useable_cnp(const struct arm64_cpu_capabilities *entry, int scope) +{ +#ifdef CONFIG_CRASH_DUMP + /* + * Kdump isn't guaranteed to power-off all secondary CPUs, CNP + * may share TLB entries with a CPU stuck in the crashed + * kernel. + */ + if (elfcorehdr_size) + return false; +#endif + return has_cpuid_feature(entry, scope); +} + #ifdef CONFIG_UNMAP_KERNEL_AT_EL0 static int __kpti_forced; /* 0: not forced, >0: forced on, <0: forced off */ @@ -1202,6 +1219,19 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .cpu_enable = cpu_enable_hw_dbm, }, #endif +#ifdef CONFIG_ARM64_CNP + { + .desc = "Common not Private translations", + .capability = ARM64_HAS_CNP, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_useable_cnp, + .sys_reg = SYS_ID_AA64MMFR2_EL1, + .sign = FTR_UNSIGNED, + .field_pos = ID_AA64MMFR2_CNP_SHIFT, + .min_field_value = 1, + .cpu_enable = cpu_enable_cnp, + }, +#endif {}, }; @@ -1638,6 +1668,11 @@ cpufeature_pan_not_uao(const struct arm64_cpu_capabilities *entry, int __unused) return (cpus_have_const_cap(ARM64_HAS_PAN) && !cpus_have_const_cap(ARM64_HAS_UAO)); } +static void __maybe_unused cpu_enable_cnp(struct arm64_cpu_capabilities const *cap) +{ + cpu_replace_ttbr1(lm_alias(swapper_pg_dir)); +} + /* * We emulate only the following system register space. * Op0 = 0x3, CRn = 0x0, Op1 = 0x0, CRm = [0, 4 - 7] diff --git a/arch/arm64/kernel/suspend.c b/arch/arm64/kernel/suspend.c index 70c2833..9405d1b 100644 --- a/arch/arm64/kernel/suspend.c +++ b/arch/arm64/kernel/suspend.c @@ -48,6 +48,10 @@ void notrace __cpu_suspend_exit(void) */ cpu_uninstall_idmap(); + /* Restore CnP bit in TTBR1_EL1 */ + if (system_supports_cnp()) + cpu_replace_ttbr1(lm_alias(swapper_pg_dir)); + /* * PSTATE was not saved over suspend/resume, re-enable any detected * features that might not have been set correctly. diff --git a/arch/arm64/mm/context.c b/arch/arm64/mm/context.c index c127f94..a65af49 100644 --- a/arch/arm64/mm/context.c +++ b/arch/arm64/mm/context.c @@ -196,6 +196,9 @@ void check_and_switch_context(struct mm_struct *mm, unsigned int cpu) unsigned long flags; u64 asid, old_active_asid; + if (system_supports_cnp()) + cpu_set_reserved_ttbr0(); + asid = atomic64_read(&mm->context.id); /* diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 5f9a73a..d46d0ca 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -160,6 +160,12 @@ ENTRY(cpu_do_switch_mm) mrs x2, ttbr1_el1 mmid x1, x1 // get mm->context.id phys_to_ttbr x3, x0 + +alternative_if ARM64_HAS_CNP + cbz x1, 1f // skip CNP for reserved ASID + orr x3, x3, #TTBR_CNP_BIT +1: +alternative_else_nop_endif #ifdef CONFIG_ARM64_SW_TTBR0_PAN bfi x3, x1, #48, #16 // set the ASID field in TTBR0 #endif