ARM: mm: cache shareability tweak
cover ARM: mm: cache shareability tweak
Hi,
This RFC patch attempts to implement support for specifying cache shareability setting via kernel cmdline. This is required at least for TI keystone2 generation SoCs, where DMA masters are snooping on the cache maintenance messages to maintain coherency. Currently we are carrying an internal hack that modifies the macros via #ifdefs, this is obviously bad as the same kernel image can only work with keystone2 (or at least might be causing problems with other SoCs.) It would be very much preferred to replace this hardcoded implementation with a runtime solution.
Some obvious holes in this implementation:
1) during execution of arch/arm/kernel/head.S, the tweaked MMU shareability settings are not in place. However, I am not too sure how much that matters, as I am not sure what is mapped at this point. Kernel image mapping should not matter at least, as we typically should not be doing any DMA transfers from the kernel image. I would like some comments on this, if handling during head.S should be fixed also, how can this be done? Some hack under compressed/keystone-head.S?
2) the cmdline parameter could be something more descriptive
3) The single RFC patch should probably be split up a bit
- Tero
RFC 1/1 ARM: mm: add support for specifying shareability level via cmdline
pmd_sect_s=x can be used to specify cache coherency level for the CPU. The value is used to setup cache coherency level for specific MMU mappings. Currently kernel only supports inner shareable attribute.
This feature is required at least for keystone DMA cache coherency support, where pages meant to be used by DMA must be marked as outer shareable also, so the DMA masters can snoop the maintenance messages.
--- arch/arm/include/asm/fixmap.h | 2 +- arch/arm/include/asm/pgtable.h | 3 +++ arch/arm/kernel/setup.c | 39 +++++++++++++++++++++++++++++++--- arch/arm/mm/dump.c | 3 +++ arch/arm/mm/mmu.c | 46 +++++++++++++++++++++++++--------------- 5 files changed, 72 insertions(+), 21 deletions(-)
arch/arm/include/asm/fixmap.h +1 -1
@@ -45,7 +45,7 @@ static const enum fixed_addresses __end_of_fixed_addresses =#define FIXMAP_PAGE_RO (FIXMAP_PAGE_NORMAL | L_PTE_RDONLY)/* Used by set_fixmap_(io|nocache), both meant for mapping a device */ -#define FIXMAP_PAGE_IO (FIXMAP_PAGE_COMMON | L_PTE_MT_DEV_SHARED | L_PTE_SHARED) +#define FIXMAP_PAGE_IO (FIXMAP_PAGE_COMMON | L_PTE_MT_DEV_SHARED | l_pte_shared)#define FIXMAP_PAGE_NOCACHE FIXMAP_PAGE_IO#define __early_set_fixmap __set_fixmap
arch/arm/include/asm/pgtable.h +3 -0
@@ -86,6 +86,9 @@ extern pgprot_t pgprot_hyp_device;extern pgprot_t pgprot_s2;extern pgprot_t pgprot_s2_device; +extern pteval_t pmd_sect_s; +extern pteval_t l_pte_shared; +#define _MOD_PROT (p, b) __pgprot(pgprot_val(p) | (b))#define PAGE_NONE _MOD_PROT(pgprot_user, L_PTE_XN | L_PTE_RDONLY | L_PTE_NONE)
arch/arm/kernel/setup.c +36 -3
@@ -33,6 +33,7 @@#include <linux/compiler.h> #include <linux/sort.h> #include <linux/psci.h> +#include <linux/moduleparam.h> #include <asm/unified.h> #include <asm/cp15.h>
@@ -668,6 +669,8 @@ static void __init smp_build_mpidr_hash(void) }#endif +static unsigned long cpu_mm_mmu_flags; +static void __init setup_processor (void ) {struct proc_info_list *list;
@@ -704,6 +707,8 @@ static void __init setup_processor(void) cpu_name,read_cpuid_id (),read_cpuid_id () &15 , proc_arch[cpu_architecture()], get_cr()); + cpu_mm_mmu_flags = list->__cpu_mm_mmu_flags; +snprintf (init_utsname ()->machine, __NEW_UTS_LEN +1 ,"%s%c" , list->arch_name,ENDIANNESS ); snprintf(elf_platform, ELF_PLATFORM_SIZE, "%s%c",
@@ -716,9 +721,6 @@ static void __init setup_processor(void)#ifndef CONFIG_ARM_THUMB elf_hwcap &= ~(HWCAP_THUMB |HWCAP_IDIVT );#endif -#ifdef CONFIG_MMU -init_default_cache_policy (list->__cpu_mm_mmu_flags); -#endif erratum_a15_798181_init ();elf_hwcap_fixup ();
@@ -1002,6 +1004,36 @@ void __init hyp_mode_check(void)#endif } +pteval_t pmd_sect_s =PMD_SECT_S ; +pteval_t l_pte_shared =L_PTE_SHARED ; + +static int __init parse_mmu_setup (char *str) +{ +u32 new_val; + +if (!get_option (&str, &new_val)) +return -EINVAL ; + +if (new_val <2 || new_val >3 ) { +pr_err ("%s: bad value: %d\n" , __func__, new_val); +return -EINVAL ; + } + + new_val =_AT (pteval_t, new_val) <<8 ; + +pr_info ("%s: setting PMD_SECT_S to %08x, old-val%08x\n" , __func__, + new_val, (u32 )pmd_sect_s); + + pmd_sect_s = new_val; + l_pte_shared = new_val; + + cpu_mm_mmu_flags = ~(u32 )PMD_SECT_S ; + cpu_mm_mmu_flags |= pmd_sect_s; + +return 0 ; +} +early_param ("pmd_sect_s" , parse_mmu_setup); +void __init setup_arch (char **cmdline_p) {const struct machine_desc *mdesc;
@@ -1032,6 +1064,7 @@ void __init setup_arch(char **cmdline_p)parse_early_param ();#ifdef CONFIG_MMU +init_default_cache_policy (cpu_mm_mmu_flags);early_paging_init (mdesc);#endif setup_dma_zone (mdesc);
arch/arm/mm/dump.c +3 -0
@@ -346,6 +346,9 @@ static int ptdump_init(void)struct dentry *pe;unsigned i, j; +if (l_pte_shared !=L_PTE_SHARED ) + pte_bits[3 ].val = l_pte_shared; +for (i =0 ; i <ARRAY_SIZE (pg_level); i++)if (pg_level[i].bits)for (j =0 ; j < pg_level[i].num; j++)
arch/arm/mm/mmu.c +29 -17
65 lines not on this page · raw message
Signature · 1 line
1.7.9.5
cover ARM: mm: cache shareability tweak
In reply to Tero KristoQuoted 9 lines · Tero Kristo
Hi,
This RFC patch attempts to implement support for specifying cache shareability setting via kernel cmdline. This is required at least for TI keystone2 generation SoCs, where DMA masters are snooping on the cache maintenance messages to maintain coherency. Currently we are carrying an internal hack that modifies the macros via #ifdefs, this is obviously bad as the same kernel image can only work with keystone2 (or at least might be causing problems with other SoCs.)
The de-facto semantics (which we should codify) for dma-coherent with ARMv7 is that a device makes accesses which are coherent with Normal, Inner Shareable, Inner Write-Back, Outer Write-Back.
In arch/arm/boot/dts/keystone.dtsi I see that /soc/usb at 2680000 has a dma-coherent flag. Is that device coherent today with upstream? Or is that misleading currently?
If the device isn't coherent with that, then dma-coherent isn't strictly true (and should go), and we need additional properties to correctly describe this case.
Quoted 10 lines
It would be very much preferred to replace this hardcoded implementation with a runtime solution.
Some obvious holes in this implementation:
1) during execution of
arch/arm/kernel/head.S, the tweaked MMU shareability settings are not in place. However, I am not too sure how much that matters, as I am not sure what is mapped at this point. Kernel image mapping should not matter at least, as we typically should not be doing any DMA transfers from the kernel image.
Strictly speaking, changing the shareability can result in a loss of coherency, even if all accesses are made by the same CPU. See "Mismatched memory attributes" in section A3.5.7 of the ARMv7-AR Reference Manual (ARM DDI 0406C.c).
It's not just DMA that matters. I believe we may have page tables as part of the kernel image, for instance, and those need to be accessed with consistent attributes by the MMU when doing page table walks.
You can avoid issues so long as you have appropriate cache maintenance, but that's both expensive (all memory previously mapped must be Clean+Invalidated by VA) and painful (as you can't reliably use any of said memory until after the maintenance).
Quoted 3 lines
I would like some comments on this, if handling during
head.Sshould be fixed also, how can this be done? Some hack undercompressed/keystone-head.S?
If you need to do this, you need consistent attributes from the outset, or you need to disable the MMU, perform cache maintenance, and re-enter the kernel.
Quoted 3 lines
2) the cmdline parameter could be something more descriptive
3) The single RFC patch should probably be split up a bit
4) It isn't possible to use dma-coherent to describe this without weakening the semantics so as to be meaningless in general. So if we go for this approach we need a mechanism to accurately describe the coherency guarantees of masters in the system beyond a boolean.
Thanks,
Mark.
cover ARM: mm: cache shareability tweak
Quoted 11 lines · Mark Rutland
Quoted 9 lines · Tero Kristo
Hi,
This RFC patch attempts to implement support for specifying cache shareability setting via kernel cmdline. This is required at least for TI keystone2 generation SoCs, where DMA masters are snooping on the cache maintenance messages to maintain coherency. Currently we are carrying an internal hack that modifies the macros via #ifdefs, this is obviously bad as the same kernel image can only work with keystone2 (or at least might be causing problems with other SoCs.)
Quoted 7 lines
The de-facto semantics (which we should codify) for dma-coherent with ARMv7 is that a device makes accesses which are coherent with Normal, Inner Shareable, Inner Write-Back, Outer Write-Back.
In
arch/arm/boot/dts/keystone.dtsiI see that /soc/usb at 2680000 has a dma-coherent flag. Is that device coherent today with upstream? Or is that misleading currently?
Good question, Murali, can you comment on this? What peripherals are actually requiring the DMA coherency on K2?
Quoted 16 lines
If the device isn't coherent with that, then dma-coherent isn't strictly true (and should go), and we need additional properties to correctly describe this case.
Quoted 10 lines
It would be very much preferred to replace this hardcoded implementation with a runtime solution.
Some obvious holes in this implementation:
1) during execution of
arch/arm/kernel/head.S, the tweaked MMU shareability settings are not in place. However, I am not too sure how much that matters, as I am not sure what is mapped at this point. Kernel image mapping should not matter at least, as we typically should not be doing any DMA transfers from the kernel image.
Quoted 4 lines
Strictly speaking, changing the shareability can result in a loss of coherency, even if all accesses are made by the same CPU. See "Mismatched memory attributes" in section A3.5.7 of the ARMv7-AR Reference Manual (ARM DDI
0406C.c).
Basically we are not attempting to change shareability in-the-fly, but instead configure a different shareability value that is going to be used always.
Quoted 9 lines
It's not just DMA that matters. I believe we may have page tables as part of the kernel image, for instance, and those need to be accessed with consistent attributes by the MMU when doing page table walks.
You can avoid issues so long as you have appropriate cache maintenance, but that's both expensive (all memory previously mapped must be Clean+Invalidated by VA) and painful (as you can't reliably use any of said memory until after the maintenance).
The hack we have internally just maps all the DMA pages as outer shareable. I think maybe adding the original hack might help understanding the issue, so added inline in the end as reference. We just attempt to change the shareability value from 3 (the current) to 2.
Quoted 13 lines
Quoted 4 lines
I would like some comments on this, if handling during
head.Sshould be fixed also, how can this be done? Some hack undercompressed/keystone-head.S?If you need to do this, you need consistent attributes from the outset, or you need to disable the MMU, perform cache maintenance, and re-enter the kernel.
Quoted 3 lines
2) the cmdline parameter could be something more descriptive
3) The single RFC patch should probably be split up a bit
Quoted 8 lines
4) It isn't possible to use dma-coherent to describe this without weakening the semantics so as to be meaningless in general. So if we go for this approach we need a mechanism to accurately describe the coherency guarantees of masters in the system beyond a boolean.
Thanks,
Mark.
(quoted diff) +0 -0
b/arch/arm/include/asm/pgtable-3level-hwdef.h
index f8f1cff..62adf21 100644
arch/arm/include/asm/pgtable-3level-hwdef.h +8 -0
@@ -44,7 +44,11 @@#define PMD_SECT_CACHEABLE (_AT(pmdval_t, 1) << 3)#define PMD_SECT_USER (_AT(pmdval_t, 1) << 6)/* AP[1] */ #define PMD_SECT_AP2 (_AT(pmdval_t, 1) << 7)/* read only */ +#ifdef CONFIG_KEYSTONE2_DMA_COHERENT +#define PMD_SECT_S (_AT(pmdval_t, 2) << 8) +#else #define PMD_SECT_S (_AT(pmdval_t, 3) << 8) +#endif #define PMD_SECT_AF (_AT(pmdval_t, 1) << 10)#define PMD_SECT_nG (_AT(pmdval_t, 1) << 11)#define PMD_SECT_PXN (_AT(pmdval_t, 1) << 53)
@@ -73,7 +77,12 @@#define PTE_BUFFERABLE (_AT(pteval_t, 1) << 2)/* AttrIndx[0] */ #define PTE_CACHEABLE (_AT(pteval_t, 1) << 3)/* AttrIndx[1] */ #define PTE_AP2 (_AT(pteval_t, 1) << 7)/* AP[2] */ +#ifdef CONFIG_KEYSTONE2_DMA_COHERENT +/* SH[1:0], outer shareable */ +#define PTE_EXT_SHARED (_AT(pteval_t, 2) << 8) +#else #define PTE_EXT_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner
shareable */
+#endif
#define PTE_EXT_AF (_AT(pteval_t, 1) << 10) /* Access Flag */
#define PTE_EXT_NG (_AT(pteval_t, 1) << 11) /* nG */
#define PTE_EXT_PXN (_AT(pteval_t, 1) << 53) /* PXN */
(quoted diff) +0 -0
b/arch/arm/include/asm/pgtable-3level.h
index a745a2a..b4090b1 100644
arch/arm/include/asm/pgtable-3level.h +4 -0
@@ -78,7 +78,12 @@#define L_PTE_VALID (_AT(pteval_t, 1) << 0)/* Valid */ #define L_PTE_PRESENT (_AT(pteval_t, 3) << 0)/* Present */ #define L_PTE_USER (_AT(pteval_t, 1) << 6)/* AP[1] */ +#ifdef CONFIG_KEYSTONE2_DMA_COHERENT +/* SH[1:0], outer shareable */ +#define L_PTE_SHARED (_AT(pteval_t, 2) << 8) +#else #define L_PTE_SHARED (_AT(pteval_t, 3) << 8)/* SH[1:0], inner
shareable */
+#endif
#define L_PTE_YOUNG (_AT(pteval_t, 1) << 10) /* AF */
#define L_PTE_XN (_AT(pteval_t, 1) << 54) /* XN */
#define L_PTE_DIRTY (_AT(pteval_t, 1) << 55)
arch/arm/mach-keystone/Kconfig +4 -0
@@ -11,6 +11,10 @@ config ARCH_KEYSTONE select ZONE_DMA if ARM_LPAE select MIGHT_HAVE_PCI select PCI_DOMAINS if PCI + select KEYSTONE2_DMA_COHERENT help Support for boards based on the Texas Instruments Keystone family of SoCs. + +config KEYSTONE2_DMA_COHERENT + bool
cover ARM: mm: cache shareability tweak
Quoted 10 lines · Tero Kristo
Quoted 9 lines · Mark Rutland
Quoted 7 lines · Tero Kristo
Some obvious holes in this implementation:
1) during execution of
arch/arm/kernel/head.S, the tweaked MMU shareability settings are not in place. However, I am not too sure how much that matters, as I am not sure what is mapped at this point. Kernel image mapping should not matter at least, as we typically should not be doing any DMA transfers from the kernel image.
Quoted 8 lines
Quoted 4 lines
Strictly speaking, changing the shareability can result in a loss of coherency, even if all accesses are made by the same CPU. See "Mismatched memory attributes" in section A3.5.7 of the ARMv7-AR Reference Manual (ARM DDI
0406C.c).Basically we are not attempting to change shareability in-the-fly, but instead configure a different shareability value that is going to be used always.
I understood that this was a one-time transition; the problem still applies, per the architecture. Your implementation _may_ provide stronger guarantees than architecturally required.
Caches lines allocated as a result of accesses with one set of attributes are not necessarily coherent with accesses with differing attributes (even if only differing in terms of shareability). Until the cache maintenance mandated by the ARM ARM is performed, you may encounter a number of issues.
Thus changing attributes once during the boot process is potentially problematic.
Quoted 11 lines
Quoted 8 lines
It's not just DMA that matters. I believe we may have page tables as part of the kernel image, for instance, and those need to be accessed with consistent attributes by the MMU when doing page table walks.
You can avoid issues so long as you have appropriate cache maintenance, but that's both expensive (all memory previously mapped must be Clean+Invalidated by VA) and painful (as you can't reliably use any of said memory until after the maintenance).
The hack we have internally just maps all the DMA pages as outer shareable.
I'm not sure precisely what you mean by "DMA pages" here. Surely this applies to any mappings created using the usual page table accessors?
Quoted 3 lines
I think maybe adding the original hack might help understanding the issue, so added inline in the end as reference. We just attempt to change the shareability value from 3 (the current) to 2.
I understand that you only wish to change the shareability.
What I am describing are the caveats that come with doing do, which are not necessarily intuitive, but are laid out in the ARM ARM.
Thanks,
Mark.