RFC ARM core support for hardware I/O coherency in non-SMP platforms
Get series with b4
b4 am [email protected]Versions newest first
- v1 RFC Latest0 of 7 patches 0 repliesNew
Patches in v1 No applies recorded yet
None of the 7 patches the cover letter announces are in this archive.
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [RFC PATCHv1 0/7] ARM core support for hardware I/O coherency in non-SMP platforms
Quoted 30 lines · Thomas Petazzoni
Quoted 4 lines · Catalin Marinas
Quoted 3 lines · Thomas Petazzoni
This hardware I/O coherency mechanism needs a set of ARM core requirements to operate properly:
Quoted 8 lines
Quoted 3 lines
* On Armada 370 (a single core processor)
- The cache policy of pages must be set to "write allocate".
Arguably, I would make this the default for ARMv6+ CPUs even if UP. It's a hint that the CPU may or may not ignore but it shouldn't break anything (well, maybe some artificial benchmarks designed to show that write-allocate caches are bad).
So, something like:
arch/arm/mm/mmu.c +1
-1@@ -57,7 +57,7 @@ pmd_t *top_pmd;#define CPOLICY_WRITEBACK 3#define CPOLICY_WRITEALLOC 4 -static unsigned int cachepolicy__initdata =CPOLICY_WRITEBACK ; +static unsigned int cachepolicy__initdata =CPOLICY_WRITEALLOC ;static unsigned int ecc_mask__initdata =0 ;pgprot_t pgprot_user;pgprot_t pgprot_kernel;
arch/arm/mm/mmu.c static void __init build_mem_type_table(void) · line 413
Quoted 6 lines
arch/arm/mm/mmu.c +2
-0@@ -408,14 +408,14 @@ static void __init build_mem_type_table(void)if (cachepolicy >CPOLICY_WRITETHROUGH ) cachepolicy =CPOLICY_WRITETHROUGH ;#endif +if (cachepolicy >CPOLICY_WRITEBACK ) + cachepolicy =CPOLICY_WRITEBACK ; }
There is some more clean-up to do in this file as there are other assumptions about cachepolicy. For example, early_cachepolicy() sets it back to writeback (and possibly the initial kernel mapping to avoid aliases, though having the same type, just the allocation hint doesn't really cause problems).
Quoted 11 lines
arch/arm/mm/mmu.c +0
-0> > * On Armada 375/38x (which have single core and dual core variants)Quoted 7 lines
Quoted 4 lines
- The cache policy of pages must be set to "write allocate".
- The SMP and TLB broadcast bits must be set in the Auxiliary Control Register (the core is a Cortex-A9)What about setting this bit in the
firmware/bootloader? It's a sane initialisation firmware should do.I'm sorry, but I don't buy the "fix your bootloader" argument. For various reasons:
Well, for arm64 people should get used to this because I'm not taking code for setting up things like SCU and ACTLR during boot.
Quoted 4 lines
- The Linux kernel policy has always been to do *more* things in the kernel, to avoid relying on crappy vendor-specific bootloaders, and avoid having bugs that cannot be fixed at the kernel level. Going in the opposite direction looks completely backward.
There is no written policy here for how close to the hardware reset the kernel should run. With the same arguments people may want to initialise the DRAM controller in Linux while booting in SRAM temporarily (or ROM/flash + XIP).
But in this particular case, let's look at ACTLR. It's a secure-only register, so what if someone decides to deploy Linux as a non-secure OS on this platform? Do you add a config option to have a different kernel build?
Quoted 2 lines
- Doing so means crazy complex dependencies of kernel versions against bootloader versions, which are a nightmare for users.
Because we didn't have a clear policy here, the secure extensions came in later when we were already doing things like this. It's more difficult to enforce it now but we face such issues regularly. For example, on several occasions we had TI asking for per-SoC CPU initialisation (possibly as addition to __v7_setup) for specific SMC calls to the firmware which breaks single Image.
It make sense a lot of sense to place sane initialisation in a SoC-specific firmware/bootloader but whether we want to enforce such policy is Russell's call.
Quoted 5 lines
- The kernel is setting the SMP and TLB broadcast bit already in proc-v7.s when the system is SMP. What would these actions have to be done in the bootloader when the system is non-SMP, but I/O coherent, and done in the kernel when the system is SMP? This doesn't make sense.
Because of historical reasons, we can't remove the ACTLR setting in proc-v7.S, as much as I would like. But when Linux runs in non-secure mode, such initialisation is already done by firmware, so it's not unheard of in the 32-bit ARM world.
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
- The pages must be set as shareable.
Here you may have some conflict between the initial page tables set in
__create_page_tablesas non-shareable (that's unless MPIDR shows it as SMP but I guess not since smp-on-up kicks in).
[...]
Quoted 10 lines
Quoted 4 lines
I have to think a bit
more about the implications (the ARM ARM has a chapter on mismatched memory attributes and I think it talks about shareable vs non-shareable).Unfortunately,
__create_page_tablesis called so early that I don't know if it's possible to access 'structmachine_desc' at this point to know whether the platform needs shareable pages or not.Also, don't we have the same problem with the TTB_FLAGS_{SMP,UP) ?
I don't think we do because we use the ALT_{SMP,UP} for TTB settings and we have the smp-on-up fixup running before __v7_setup.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
- The SCU must be enabled
Again, could the firmware do this?
See above. If the kernel does it for SMP cases, why wouldn't it do it also for !SMP I/O coherent cases?
The I/O coherency is an SoC property rather than an ARM architecture property. I want to separate the two so that the kernel can boot a significant part assuming sane architecture settings. Once you have DT available and start loading device drivers, you are in the SoC realm and you can do whatever initialisation for buses, interconnects, but not going back to change architected settings.
Quoted 5 lines
It's either it's *always* done by the
bootloader and we completely remove the SCU enabling code in the kernel, and add to the ARM boot protocol that enabling the SCU is the responsibility of the bootloader, or the kernel does the SCU enabling in all situations.
I'm always for the former (and that's the case for arm64), though for some older boards it's too late to change.
Signature · 1 line
Catalin