armv8: caches: Added routine to set non cacheable region

PATCH Superseded Siva Durga Prasad Paladugu 2 participants 4 messages May 26, 2015 – Jun 11, 2015 Arm (32-bit)

lore

Share & tools

lore

Siva Durga Prasad Paladugu

author

2015-05-26 03:34 UTC #0 lore raw

PATCH v3 1/1 armv8: caches: Added routine to set non cacheable region

Added routine mmu_set_region_dcache_behaviour() to set a particular region as non cacheable.

Define dummy routine for mmu_set_region_dcache_behaviour() to handle incase of dcache off.

Signed-off-by Siva Durga Prasad Paladugu <[email protected]>

Acked-by Michal Simek <[email protected]>

---
Changes in v3:
- Removed board depenedency to calculate the page table address to start with looking into the entry. Now, get the page table address from the board specific code.
- Updated flush dcache with actual Page address instead of MMU entry.
Changes in v2:
- Fix patch subject (remove addional zzz from v1)
- Remove armv8: caches: Disable dcache after flush patch from this series based on the talk with Mark Rutland (patch is not needed anymore)

---
 arch/arm/cpu/armv8/cache_v8.c |   32 ++++++++++++++++++++++++++++++++
 arch/arm/include/asm/system.h |   29 +++++++++++++++++++----------
 2 files changed, 51 insertions(+), 10 deletions(-)

arch/arm/cpu/armv8/cache_v8.c +32 -0

index c5ec529..1c6b4b4 100644
@@ -139,6 +139,33 @@ int dcache_status(void)
 	return (get_sctlr() & CR_C) != 0;
 }
 
+u64 *__weak arch_get_page_table(void) {
+	puts("No page table offset defined\n");
+
+	return NULL;
+}
+
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+	u64 *page_table = arch_get_page_table();
+	u64 upto, end;
+
+	if (page_table == NULL)
+		return;
+
+	end = ALIGN(start + size, (1 << MMU_SECTION_SHIFT)) >>
+	      MMU_SECTION_SHIFT;
+	start = start >> MMU_SECTION_SHIFT;
+	for (upto = start; upto < end; upto++) {
+		page_table[upto] &= ~PMD_ATTRINDX_MASK;
+		page_table[upto] |= PMD_ATTRINDX(option);
+	}
+
+	start = start << MMU_SECTION_SHIFT;
+	end = end << MMU_SECTION_SHIFT;
+	flush_dcache_range(start, end);
+}
 #else	/* CONFIG_SYS_DCACHE_OFF */
 
 void invalidate_dcache_all(void)

@@ -170,6 +197,11 @@ int dcache_status(void)
 	return 0;
 }
 
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+}
+
 #endif	/* CONFIG_SYS_DCACHE_OFF */
 
 #ifndef CONFIG_SYS_ICACHE_OFF

arch/arm/include/asm/system.h +19 ~~-10~~

index 760e8ab..868ea54 100644
@@ -15,9 +15,15 @@
 #define CR_EE		(1 << 25)	/* Exception (Big) Endian	*/
 
 #define PGTABLE_SIZE	(0x10000)
+/* 2MB granularity */
+#define MMU_SECTION_SHIFT	21
 
 #ifndef __ASSEMBLY__
 
+enum dcache_option {
+	DCACHE_OFF = 0x3,
+};
+
 #define isb()				\
 	({asm volatile(			\
 	"isb" : : : "memory");		\

@@ -265,16 +271,6 @@ enum {
 #endif
 
 /**
- * Change the cache settings for a region.
- *
- * \param start		start address of memory region to change
- * \param size		size of memory region to change
- * \param option	dcache option to select
- */
-void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
-				     enum dcache_option option);
-
-/**
  * Register an update to the page tables, and flush the TLB
  *
  * \param start		start address of update in page table

@@ -295,4 +291,17 @@ phys_addr_t noncached_alloc(size_t size, size_t align);
 
 #endif /* CONFIG_ARM64 */
 
+#ifndef __ASSEMBLY__
+/**
+ * Change the cache settings for a region.
+ *
+ * \param start		start address of memory region to change
+ * \param size		size of memory region to change
+ * \param option	dcache option to select
+ */
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option);
+
+#endif /* __ASSEMBLY__ */
+
 #endif

Signature · 1 line

1.7.1

Mark Rutland

2015-05-28 09:39 UTC #1 lore raw

PATCH v3 1/1 armv8: caches: Added routine to set non cacheable region

Hi,

Quoted 6 lines

arch/arm/cpu/armv8/cache_v8.c +6 -0

+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+	u64 *page_table = arch_get_page_table();
+	u64 upto, end;
+

arch/arm/cpu/armv8/cache_v8.c int dcache_status(void) · line 163

Quoted 10 lines

arch/arm/cpu/armv8/cache_v8.c +10 -0

+	if (page_table == NULL)
+		return;
+
+	end = ALIGN(start + size, (1 << MMU_SECTION_SHIFT)) >>
+	      MMU_SECTION_SHIFT;
+	start = start >> MMU_SECTION_SHIFT;
+	for (upto = start; upto < end; upto++) {
+		page_table[upto] &= ~PMD_ATTRINDX_MASK;
+		page_table[upto] |= PMD_ATTRINDX(option);
+	}

These writes might not be visible to the page table walkers immediately, and the TLBs might still contain stale values for a while afterwards. That could render the cache maintenance useless (as speculative fetches could still occur due to cacheable attributes still being in place).

You need a DSB to ensure writes are visible to the page table walkers (with a compiler barrier to ensure that the writes actually occur before the DSB), and some TLB maintenance (complete with another DSB) to ensure that the TLBs don't contain stale values by the time to get to the cache afterwards.

Also minor nit, but s/upto/i/?

Quoted 25 lines

arch/arm/cpu/armv8/cache_v8.c +10 -0

+
+	start = start << MMU_SECTION_SHIFT;
+	end = end << MMU_SECTION_SHIFT;
+	flush_dcache_range(start, end);
+}
 #else	/* CONFIG_SYS_DCACHE_OFF */
 
 void invalidate_dcache_all(void)

@@ -170,6 +197,11 @@ int dcache_status(void)
 	return 0;
 }
 
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+				     enum dcache_option option)
+{
+}
+
 #endif	/* CONFIG_SYS_DCACHE_OFF */
 
 #ifndef CONFIG_SYS_ICACHE_OFF

arch/arm/include/asm/system.h +0 -0

index 760e8ab..868ea54 100644

arch/arm/include/asm/system.h line 19

Quoted 5 lines

arch/arm/include/asm/system.h +2 -0

@@ -15,9 +15,15 @@
 #define CR_EE		(1 << 25)	/* Exception (Big) Endian	*/
 
 #define PGTABLE_SIZE	(0x10000)
+/* 2MB granularity */
+#define MMU_SECTION_SHIFT	21

Do we only expect 4K pages for now?

Thanks,
Mark.

Siva Durga Prasad Paladugu

author

2015-06-11 07:17 UTC #2 lore raw

PATCH v3 1/1 armv8: caches: Added routine to set non cacheable region

Hi Mark,

Quoted 32 lines

-----Original Message-----
From: Mark Rutland [mailto:mark.rutland at arm.com]
Sent: Thursday, May 28, 2015 3:10 PM
To: Siva Durga Prasad Paladugu
Cc: u-boot at lists.denx.de; Michal Simek; Siva Durga Prasad Paladugu
Subject: Re: [PATCH v3] armv8: caches: Added routine to set non cacheable region
Hi,
Quoted 6 lines
arch/arm/cpu/armv8/cache_v8.c +6 -0
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+                                enum dcache_option option)
+{
+   u64 *page_table = arch_get_page_table();
+   u64 upto, end;
+
arch/arm/cpu/armv8/cache_v8.c int dcache_status(void) · line 163
Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+   if (page_table == NULL)
+           return;
+
+   end = ALIGN(start + size, (1 << MMU_SECTION_SHIFT)) >>
+         MMU_SECTION_SHIFT;
+   start = start >> MMU_SECTION_SHIFT;
+   for (upto = start; upto < end; upto++) {
+           page_table[upto] &= ~PMD_ATTRINDX_MASK;
+           page_table[upto] |= PMD_ATTRINDX(option);
+   }
These writes might not be visible to the page table walkers immediately, and the TLBs might still contain stale values for a while afterwards. That could render the cache maintenance useless (as speculative fetches could still occur due to cacheable attributes still being in place).

Quoted 4 lines

You need a DSB to ensure writes are visible to the page table walkers (with a compiler barrier to ensure that the writes actually occur before the DSB), and some TLB maintenance (complete with another DSB) to ensure that the TLBs don't contain stale values by the time to get to the cache afterwards.

The flush_dcache _range() below contains a dsb. Isn't it fine enough? Or we need a separte dsb in the for loopafter we changed the cache attribute. Regarding the TLB maintenance if we have _asm_invalidate_tlb_all() after the flush dcache range below it should be fine right?

Quoted 31 lines

Also minor nit, but s/upto/i/?

Quoted 25 lines

arch/arm/cpu/armv8/cache_v8.c +10 -0

+
+   start = start << MMU_SECTION_SHIFT;
+   end = end << MMU_SECTION_SHIFT;
+   flush_dcache_range(start, end);
+}
 #else      /* CONFIG_SYS_DCACHE_OFF */
 
 void invalidate_dcache_all(void)

@@ -170,6 +197,11 @@ int dcache_status(void)
    return 0;
 }
 
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+                                enum dcache_option option)
+{
+}
+
 #endif     /* CONFIG_SYS_DCACHE_OFF */
 
 #ifndef CONFIG_SYS_ICACHE_OFF

(quoted diff) +0 -0

b/arch/arm/include/asm/system.h index 760e8ab..868ea54 100644

arch/arm/include/asm/system.h line 15

Quoted 1 line

arch/arm/include/asm/system.h +0 -0

@@ -15,9 +15,15 @@
 #define CR_EE              (1 << 25)       /* Exception (Big) Endian

*/

Quoted 1 line

Quoted 5 lines

arch/arm/include/asm/system.h line 19
Quoted 3 lines
#define PGTABLE_SIZE (0x10000)
arch/arm/include/asm/system.h +2 -0
+/* 2MB granularity */
+#define MMU_SECTION_SHIFT  21
Do we only expect 4K pages for now?

Ahh yes for now and can modify it

Regards,
Siva

Quoted 3 lines

Thanks,
Mark.

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Mark Rutland

2015-06-11 11:01 UTC #3 lore raw

PATCH v3 1/1 armv8: caches: Added routine to set non cacheable region

Quoted 35 lines · Siva Durga Prasad Paladugu

Hi Mark,
Quoted 32 lines
-----Original Message-----
From: Mark Rutland [mailto:mark.rutland at arm.com]
Sent: Thursday, May 28, 2015 3:10 PM
To: Siva Durga Prasad Paladugu
Cc: u-boot at lists.denx.de; Michal Simek; Siva Durga Prasad Paladugu
Subject: Re: [PATCH v3] armv8: caches: Added routine to set non cacheable region
Hi,
Quoted 6 lines
arch/arm/cpu/armv8/cache_v8.c +6 -0
+void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
+                                enum dcache_option option)
+{
+   u64 *page_table = arch_get_page_table();
+   u64 upto, end;
+
arch/arm/cpu/armv8/cache_v8.c int dcache_status(void) · line 163
Quoted 10 lines
arch/arm/cpu/armv8/cache_v8.c +10 -0
+   if (page_table == NULL)
+           return;
+
+   end = ALIGN(start + size, (1 << MMU_SECTION_SHIFT)) >>
+         MMU_SECTION_SHIFT;
+   start = start >> MMU_SECTION_SHIFT;
+   for (upto = start; upto < end; upto++) {
+           page_table[upto] &= ~PMD_ATTRINDX_MASK;
+           page_table[upto] |= PMD_ATTRINDX(option);
+   }
These writes might not be visible to the page table walkers immediately, and the TLBs might still contain stale values for a while afterwards. That could render the cache maintenance useless (as speculative fetches could still occur due to cacheable attributes still being in place).

Quoted 7 lines

Quoted 4 lines
You need a DSB to ensure writes are visible to the page table walkers (with a compiler barrier to ensure that the writes actually occur before the DSB), and some TLB maintenance (complete with another DSB) to ensure that the TLBs don't contain stale values by the time to get to the cache afterwards.
The flush_dcache _range() below contains a dsb. Isn't it fine enough? Or we need a separte dsb in the for loopafter we changed the cache attribute.

The DSB in flush_dcache_range() is not sufficient. You need a DSB between the page table modifications and the TLB invalidation, and the TLB invalidation must be completed before the cache maintenance begins.

Quoted 2 lines

Regarding the TLB maintenance if we have _asm_invalidate_tlb_all() after the flush dcache range below it should be fine right?

No. The TLB maintenance must be complete _before_ the cache maintenance, or the cache can be refilled while the maintenance is ongoing (e.g. the CPU could make speculative prefetches).

You need a strictly-ordered sequence:

1) Modify the page tables

2) DSB

This ensures the updates are visible to the page table walker(s).

3) TLB invalidation

4) DSB

This ensures that the TLB invalidation is complete (i.e. from this point on the TLBs cannot hold entries for the region with cacheable attributes).

5) ISB

This ensures that the effects of TLB invalidation are visible to later instructions. Otherwise instructions later could be using stale attributes fetched earlier by the CPU from the TLB, before the TLB invalidation completed (and hence could allocate in the caches).

6) Cache maintenance

7) DSB to complete cache maintenance

Thanks,
Mark.