Re: [U-Boot] [PATCH v3] armv8: caches: Added routine to set non cacheable region
On Thu, Jun 11, 2015 at 08:17:15AM +0100, Siva Durga Prasad Paladugu wrote:
Hi Mark,
-Original Message-
From: Mark Rutland [mailto:mark.rutl...@arm.com]
Sent: Thursday, May 28, 2015 3:10 PM
To: Siva Durga Prasad Paladugu
Cc: u-boot@lists.denx.de; Michal Simek; Siva Durga Prasad Paladugu
Subject: Re: [PATCH v3] armv8: caches: Added routine to set non cacheable
region
Hi,
+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);
+ }
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.
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.
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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] armv8: caches: Added routine to set non cacheable region
Hi Mark,
-Original Message-
From: Mark Rutland [mailto:mark.rutl...@arm.com]
Sent: Thursday, May 28, 2015 3:10 PM
To: Siva Durga Prasad Paladugu
Cc: u-boot@lists.denx.de; Michal Simek; Siva Durga Prasad Paladugu
Subject: Re: [PATCH v3] armv8: caches: Added routine to set non cacheable
region
Hi,
+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);
+ }
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.
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?
Also minor nit, but s/upto/i/?
+
+ 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
diff --git a/arch/arm/include/asm/system.h
b/arch/arm/include/asm/system.h index 760e8ab..868ea54 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -15,9 +15,15 @@
#define CR_EE (1 25) /* Exception (Big) Endian
*/
#define PGTABLE_SIZE (0x1)
+/* 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
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.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH v3] armv8: caches: Added routine to set non cacheable region
Hi,
+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);
+ }
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/?
+
+ 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
diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 760e8ab..868ea54 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -15,9 +15,15 @@
#define CR_EE(1 25) /* Exception (Big) Endian
*/
#define PGTABLE_SIZE (0x1)
+/* 2MB granularity */
+#define MMU_SECTION_SHIFT21
Do we only expect 4K pages for now?
Thanks,
Mark.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
