Skip to contentSkip to conversation

arm64: mte: Use PAGE_KERNEL_TAGGED in arch_add_memory

PATCH In review Patrick Daly 3 participants 8 messages Mar 3, 2021 – Mar 8, 2021

Subsystems (4)
Memory management MM core arm64 Memory hot(un)plug
lore
Share & tools
lore

Patrick Daly

author
#0 lore raw

PATCH 1/1 arm64: mte: Use PAGE_KERNEL_TAGGED in arch_add_memory

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
 arch/arm64/mm/mmu.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +11 -1
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
 		    struct mhp_params *params)
 {
 	int ret, flags = 0;
+	pgprot_t pgprot;
 
 	if (!inside_linear_region(start, size)) {
 		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
 	    IS_ENABLED(CONFIG_KFENCE))
 		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
 	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
 			     flags);
 
 	memblock_clear_nomap(start, size);
Signature · 8 lines
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer
#1 lore raw

Hi Patrick,

Thanks for this. Adding a few more people and linux-mm.

Quoted 28 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
 arch/arm64/mm/mmu.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +4 -0
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
 		    struct mhp_params *params)
 {
 	int ret, flags = 0;
+	pgprot_t pgprot;
 
 	if (!inside_linear_region(start, size)) {
 		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
 	    IS_ENABLED(CONFIG_KFENCE))
 		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as

arch/arm64/mm/mmu.c int arch_add_memory(int nid, u64 start, u64 size, · line 1492

Quoted 10 lines
arch/arm64/mm/mmu.c +7 -1
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
 	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
 			     flags);

We'll need a similar pattern for vmalloc() once we have the khwasan support in place. So we could add a pgprot_tagged() function (similar to pgprot_writecombine() etc.) which does the above check and returns PAGE_KERNEL_TAGGED, maybe only checking the PTE_ATTRINDX_MASK bits rather than the whole prot bits.

However, the bigger problem is that arch_add_memory() is also called for ZONE_DEVICE mappings and we can't always guarantee that such range supports tagging (most likely it doesn't, e.g. persistent memory), leading to potential external aborts.

One option is to expand mhp_params to pass additional information so that the arch code can make the right decision. Another option is to make PAGE_KERNEL_TAGGED global which is just PAGE_KERNEL for all the other architectures and use it in the core code.

Yet another option which we haven't fully explored with MTE is to have PAGE_KERNEL always tagged but add a new PAGE_KERNEL_DEVICE (or _UNTAGGED) for specific cases like ZONE_DEVICE. We need to make sure that PAGE_KERNEL doesn't end up in places where the backing memory does not support tags.

I'll give the last option a quick try and see if it falls apart (just changing PAGE_KERNEL to tagged). In terms of tag cache usage, it probably won't have much of an impact since the whole of the linear map is tagged already.

Signature · 6 lines
Catalin

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
maintainer
#2 lore raw
Quoted 62 lines · Catalin Marinas
Quoted 28 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
 arch/arm64/mm/mmu.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +4 -0
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
 		    struct mhp_params *params)
 {
 	int ret, flags = 0;
+	pgprot_t pgprot;
 
 	if (!inside_linear_region(start, size)) {
 		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
 	    IS_ENABLED(CONFIG_KFENCE))
 		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as

arch/arm64/mm/mmu.c int arch_add_memory(int nid, u64 start, u64 size, · line 1492

Quoted 10 lines
arch/arm64/mm/mmu.c +7 -1
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
 	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
 			     flags);

We'll need a similar pattern for vmalloc() once we have the khwasan support in place. So we could add a pgprot_tagged() function (similar to pgprot_writecombine() etc.) which does the above check and returns PAGE_KERNEL_TAGGED, maybe only checking the PTE_ATTRINDX_MASK bits rather than the whole prot bits.

However, the bigger problem is that arch_add_memory() is also called for ZONE_DEVICE mappings and we can't always guarantee that such range supports tagging (most likely it doesn't, e.g. persistent memory), leading to potential external aborts.

One option is to expand mhp_params to pass additional information so that the arch code can make the right decision. Another option is to make PAGE_KERNEL_TAGGED global which is just PAGE_KERNEL for all the other architectures and use it in the core code.

Yet another option which we haven't fully explored with MTE is to have PAGE_KERNEL always tagged but add a new PAGE_KERNEL_DEVICE (or _UNTAGGED) for specific cases like ZONE_DEVICE. We need to make sure that PAGE_KERNEL doesn't end up in places where the backing memory does not support tags.

Quoted 4 lines

I'll give the last option a quick try and see if it falls apart (just changing PAGE_KERNEL to tagged). In terms of tag cache usage, it probably won't have much of an impact since the whole of the linear map is tagged already.

I played with this a bit and the last option is not really feasible. There are several places in the kernel where PAGE_KERNEL is used just because the expectation is for write-back memory. For MTE, we need tagged memory either because the kernel allocators need it (with KASAN_HW_TAGS) or because it is shared with the user, potentially mapped as tagged in user space and such tags need to be preserved by the kernel. So for the latter, only the linear map needs to be tagged (and memory hotplug falls into this category).

This leaves us with one of the first two options. I think the easiest is:

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
 	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
 #define pgprot_device(prot) \
 	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
 /*
  * DMA allocations for non-coherent devices use what the Arm architecture calls
  * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
 #define pgprot_device pgprot_noncached
 #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
 #ifdef CONFIG_MMU
 #ifndef pgprot_modify
 #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +1 -1
index 5ba51a8bdaeb..4253d80a59ba 100644
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
  */
 int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
 {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };
 	u64 start, size;
 	bool new_node = false;
 	int ret;

---------------8<-------------------------------

And we can reuse pgprot_tagged() for vmalloc() with KASAN_HW_TAGS.

If we want to keep the memory hotplug change in arch_add_memory(), maybe something like below without extending mhp_params:

	struct mem_section *ms = __pfn_to_section(PHYS_PFN(start));

	if (!online_device_section(ms))
		pgprot = pgprot_tagged(params->pgprot);

My preference is for the generic code change as per the diff above as we'd need something similar for vmalloc().

Signature · 1 line
Catalin
#3 lore raw
Quoted 114 lines · Catalin Marinas
Quoted 62 lines · Catalin Marinas
Quoted 28 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
  arch/arm64/mm/mmu.c | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +4 -0
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
  		    struct mhp_params *params)
  {
  	int ret, flags = 0;
+	pgprot_t pgprot;
 
  	if (!inside_linear_region(start, size)) {
  		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
  	    IS_ENABLED(CONFIG_KFENCE))
  		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
 
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as

arch/arm64/mm/mmu.c int arch_add_memory(int nid, u64 start, u64 size, · line 1492

Quoted 10 lines
arch/arm64/mm/mmu.c +7 -1
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
  	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
  			     flags);

We'll need a similar pattern for vmalloc() once we have the khwasan support in place. So we could add a pgprot_tagged() function (similar to pgprot_writecombine() etc.) which does the above check and returns PAGE_KERNEL_TAGGED, maybe only checking the PTE_ATTRINDX_MASK bits rather than the whole prot bits.

However, the bigger problem is that arch_add_memory() is also called for ZONE_DEVICE mappings and we can't always guarantee that such range supports tagging (most likely it doesn't, e.g. persistent memory), leading to potential external aborts.

One option is to expand mhp_params to pass additional information so that the arch code can make the right decision. Another option is to make PAGE_KERNEL_TAGGED global which is just PAGE_KERNEL for all the other architectures and use it in the core code.

Yet another option which we haven't fully explored with MTE is to have PAGE_KERNEL always tagged but add a new PAGE_KERNEL_DEVICE (or _UNTAGGED) for specific cases like ZONE_DEVICE. We need to make sure that PAGE_KERNEL doesn't end up in places where the backing memory does not support tags.

Quoted 4 lines

I'll give the last option a quick try and see if it falls apart (just changing PAGE_KERNEL to tagged). In terms of tag cache usage, it probably won't have much of an impact since the whole of the linear map is tagged already.

I played with this a bit and the last option is not really feasible. There are several places in the kernel where PAGE_KERNEL is used just because the expectation is for write-back memory. For MTE, we need tagged memory either because the kernel allocators need it (with KASAN_HW_TAGS) or because it is shared with the user, potentially mapped as tagged in user space and such tags need to be preserved by the kernel. So for the latter, only the linear map needs to be tagged (and memory hotplug falls into this category).

This leaves us with one of the first two options. I think the easiest is:

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
  	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
  #define pgprot_device(prot) \
  	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
  /*
   * DMA allocations for non-coherent devices use what the Arm architecture calls
   * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
  #define pgprot_device pgprot_noncached
  #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
  #ifdef CONFIG_MMU
  #ifndef pgprot_modify
  #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +0 -0
index 5ba51a8bdaeb..4253d80a59ba 100644

mm/memory_hotplug.c static int online_memory_block(struct memory_block *mem, void *arg) · line 1075

Quoted 5 lines
mm/memory_hotplug.c +1 -1
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
   */
  int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
  {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };

This looks like we're pushing arch specific stuff ("tagged") in here. Can't we generalize this to something like

pgprot_mhp_default

(or a better name)

that defaults to PAGE_KERNEL on all architectures except arm64 which overwrites this somehow?

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#4 lore raw
Quoted 39 lines · David Hildenbrand
Quoted 38 lines · Catalin Marinas
Quoted 37 lines · Catalin Marinas
Quoted 36 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
  arch/arm64/mm/mmu.c | 12 +++++++++++-
  1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +11 -1
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
  		    struct mhp_params *params)
  {
  	int ret, flags = 0;
+	pgprot_t pgprot;
  	if (!inside_linear_region(start, size)) {
  		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
  	    IS_ENABLED(CONFIG_KFENCE))
  		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
  	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
  			     flags);

[...]

Quoted 43 lines
Quoted 34 lines

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
  	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
  #define pgprot_device(prot) \
  	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
  /*
   * DMA allocations for non-coherent devices use what the Arm architecture calls
   * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
  #define pgprot_device pgprot_noncached
  #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
  #ifdef CONFIG_MMU
  #ifndef pgprot_modify
  #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +0 -0
index 5ba51a8bdaeb..4253d80a59ba 100644

mm/memory_hotplug.c static int online_memory_block(struct memory_block *mem, void *arg) · line 1075

Quoted 5 lines
mm/memory_hotplug.c +1 -1
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
   */
  int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
  {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };

This looks like we're pushing arch specific stuff ("tagged") in here. Can't we generalize this to something like

Quoted 6 lines

pgprot_mhp_default

(or a better name)

that defaults to PAGE_KERNEL on all architectures except arm64 which overwrites this somehow?

It works for me but I prefer the prot modification style similar to pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and just changes bits of it).

Thanks.

Signature · 1 line
Catalin
#5 lore raw
Quoted 86 lines · Catalin Marinas
Quoted 39 lines · David Hildenbrand
Quoted 38 lines · Catalin Marinas
Quoted 37 lines · Catalin Marinas
Quoted 36 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
   arch/arm64/mm/mmu.c | 12 +++++++++++-
   1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +11 -1
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
   		    struct mhp_params *params)
   {
   	int ret, flags = 0;
+	pgprot_t pgprot;
   	if (!inside_linear_region(start, size)) {
   		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
   	    IS_ENABLED(CONFIG_KFENCE))
   		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
   			     flags);

[...]

Quoted 45 lines
Quoted 34 lines

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
   #define pgprot_device(prot) \
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
   /*
    * DMA allocations for non-coherent devices use what the Arm architecture calls
    * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
   #define pgprot_device pgprot_noncached
   #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
   #ifdef CONFIG_MMU
   #ifndef pgprot_modify
   #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +0 -0
index 5ba51a8bdaeb..4253d80a59ba 100644

mm/memory_hotplug.c static int online_memory_block(struct memory_block *mem, void *arg) · line 1075

Quoted 5 lines
mm/memory_hotplug.c +1 -1
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
    */
   int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
   {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };

This looks like we're pushing arch specific stuff ("tagged") in here. Can't we generalize this to something like

pgprot_mhp_default

Quoted 8 lines
Quoted 4 lines

(or a better name)

that defaults to PAGE_KERNEL on all architectures except arm64 which overwrites this somehow?

It works for me but I prefer the prot modification style similar to pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and just changes bits of it).

Sure, I'm wondering if we can abstract the "tagged" part here.

pgprot_mhp(PAGE_KERNEL) whatsoever.

But maybe we do consider the "tagged" part okay in common code already (it felt quite arm64 specific to me at first). I can spot untagged_addr() all over the place. So maybe pgprot_tagged() is just fine.

Signature · 3 lines
Thanks,

David / dhildenb
maintainer
#6 lore raw
Quoted 96 lines · David Hildenbrand
Quoted 86 lines · Catalin Marinas
Quoted 39 lines · David Hildenbrand
Quoted 38 lines · Catalin Marinas
Quoted 37 lines · Catalin Marinas
Quoted 36 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
   arch/arm64/mm/mmu.c | 12 +++++++++++-
   1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +11 -1
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
   		    struct mhp_params *params)
   {
   	int ret, flags = 0;
+	pgprot_t pgprot;
   	if (!inside_linear_region(start, size)) {
   		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
   	    IS_ENABLED(CONFIG_KFENCE))
   		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
   			     flags);

[...]

Quoted 45 lines
Quoted 34 lines

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
   #define pgprot_device(prot) \
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
   /*
    * DMA allocations for non-coherent devices use what the Arm architecture calls
    * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
   #define pgprot_device pgprot_noncached
   #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
   #ifdef CONFIG_MMU
   #ifndef pgprot_modify
   #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +0 -0
index 5ba51a8bdaeb..4253d80a59ba 100644

mm/memory_hotplug.c static int online_memory_block(struct memory_block *mem, void *arg) · line 1075

Quoted 5 lines
mm/memory_hotplug.c +1 -1
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
    */
   int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
   {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };

This looks like we're pushing arch specific stuff ("tagged") in here. Can't we generalize this to something like

pgprot_mhp_default

Quoted 8 lines
Quoted 4 lines

(or a better name)

that defaults to PAGE_KERNEL on all architectures except arm64 which overwrites this somehow?

It works for me but I prefer the prot modification style similar to pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and just changes bits of it).

Quoted 7 lines

Sure, I'm wondering if we can abstract the "tagged" part here.

pgprot_mhp(PAGE_KERNEL) whatsoever.

But maybe we do consider the "tagged" part okay in common code already (it felt quite arm64 specific to me at first). I can spot untagged_addr() all over the place. So maybe pgprot_tagged() is just fine.

That was my reasoning initially though 'tagged' in untagged_addr() refers to some bits in the top part of the address and that will become more common. 'Tagged' as a mapping attribute is specific to arm64 and sparc for now.

That said, pgprot_mhp() is probably better as the core code cannot tell whether the arch implementation wants the hotplugged memory tagged or not. Similarly, we'll add a pgprot_vmalloc() at some point.

Signature · 1 line
Catalin
author
#7 lore raw
Quoted 105 lines · Catalin Marinas
Quoted 96 lines · David Hildenbrand
Quoted 86 lines · Catalin Marinas
Quoted 39 lines · David Hildenbrand
Quoted 38 lines · Catalin Marinas
Quoted 37 lines · Catalin Marinas
Quoted 36 lines · Patrick Daly

In a system which supports MTE, the linear kernel region must allow reading/writing allocation tags. For memory present at boot this is already being done in map_mem(). Add the same in arch_add_memory().

Signed-off-by Patrick Daly <[email protected]>
---
   arch/arm64/mm/mmu.c | 12 +++++++++++-
   1 file changed, 11 insertions(+), 1 deletion(-)
arch/arm64/mm/mmu.c +11 -1
index 9b25d60b..0fcfe90 100644
@@ -1463,6 +1463,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
   		    struct mhp_params *params)
   {
   	int ret, flags = 0;
+	pgprot_t pgprot;
   	if (!inside_linear_region(start, size)) {
   		pr_err("[%llx %llx] is outside linear mapping region\n", start, start + size);

@@ -1477,8 +1478,17 @@ int arch_add_memory(int nid, u64 start, u64 size,
   	    IS_ENABLED(CONFIG_KFENCE))
   		flags = NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
+	/*
+	 * The linear map must allow allocation tags reading/writing
+	 * if MTE is present. Otherwise, it has the same attributes as
+	 * PAGE_KERNEL.
+	 */
+	pgprot = params->pgprot;
+	if (pgprot_val(pgprot) == pgprot_val(PAGE_KERNEL))
+		pgprot = PAGE_KERNEL_TAGGED;
+
   	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
-			     size, params->pgprot, __pgd_pgtable_alloc,
+			     size, pgprot, __pgd_pgtable_alloc,
   			     flags);

[...]

Quoted 45 lines
Quoted 34 lines

---------------8<-------------------------------

arch/arm64/include/asm/pgtable.h +2 -0
index e17b96d0e4b5..5c78b92d9ec5 100644
@@ -486,6 +486,8 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
   #define pgprot_device(prot) \
   	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE) | PTE_PXN | PTE_UXN)
+#define pgprot_tagged(prot) \
+	__pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_TAGGED))
   /*
    * DMA allocations for non-coherent devices use what the Arm architecture calls
    * "Normal non-cacheable" memory, which permits speculation, unaligned accesses
include/linux/pgtable.h +4 -0
index cdfc4e9f253e..f5f5044db2ce 100644
@@ -904,6 +904,10 @@ static inline void ptep_modify_prot_commit(struct vm_area_struct *vma,
   #define pgprot_device pgprot_noncached
   #endif
 
+#ifndef pgprot_tagged
+#define pgprot_tagged(prot)	(prot)
+#endif
+
   #ifdef CONFIG_MMU
   #ifndef pgprot_modify
   #define pgprot_modify pgprot_modify
mm/memory_hotplug.c +0 -0
index 5ba51a8bdaeb..4253d80a59ba 100644

mm/memory_hotplug.c static int online_memory_block(struct memory_block *mem, void *arg) · line 1075

Quoted 5 lines
mm/memory_hotplug.c +1 -1
@@ -1072,7 +1072,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
    */
   int __ref add_memory_resource(int nid, struct resource *res, mhp_t mhp_flags)
   {
-	struct mhp_params params = { .pgprot = PAGE_KERNEL };
+	struct mhp_params params = { .pgprot = pgprot_tagged(PAGE_KERNEL) };

This looks like we're pushing arch specific stuff ("tagged") in here. Can't we generalize this to something like

pgprot_mhp_default

Quoted 8 lines
Quoted 4 lines

(or a better name)

that defaults to PAGE_KERNEL on all architectures except arm64 which overwrites this somehow?

It works for me but I prefer the prot modification style similar to pgprot_writecombine() etc. (i.e. takes a parameter like PAGE_KERNEL and just changes bits of it).

Quoted 7 lines

Sure, I'm wondering if we can abstract the "tagged" part here.

pgprot_mhp(PAGE_KERNEL) whatsoever.

But maybe we do consider the "tagged" part okay in common code already (it felt quite arm64 specific to me at first). I can spot untagged_addr() all over the place. So maybe pgprot_tagged() is just fine.

Quoted 11 lines

That was my reasoning initially though 'tagged' in untagged_addr() refers to some bits in the top part of the address and that will become more common. 'Tagged' as a mapping attribute is specific to arm64 and sparc for now.

That said, pgprot_mhp() is probably better as the core code cannot tell whether the arch implementation wants the hotplugged memory tagged or not. Similarly, we'll add a pgprot_vmalloc() at some point.

Signature · 1 line
Catalin

Thanks for finding a cleaner solution than my original proposal.

Signature · 2 lines
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project