Sam Lewis has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
armv7: mmu: Use 'tlbimva' to invalidate TLB entries
The tlbimvaa operation (invalidate unified TLB by MVA, all address space identifiers) is only available on armv7 processors that support Multiprocessing Extensions. When used on processors that do not support the extensions it causes an "undefined instruction" exception.
This patch changes the MMU table entry filling code to use the tlbimva (invalidate unified TLB entry by MVA and address space identifier) operation for invalidating TLB entries, which is supported on all armv7 processors.
As address space identifiers are not used in TLB entries in coreboot (all entries are set as global), these two operations can safely be used interchangebly. The ASID value supplied to the operation is not checked for global TLB entries.
More information as well as the data formats for the tlbimvaa and tlbimba operations are detailed in the "ARM Architecture Reference Manual ARMv7-A" edition, issue "C.c" page B4-1747.
TEST: Booted Beaglebone Black (my current in progress port)
Change-Id: Ie7dfb4adab20dc7eecb1b20aa2ee6355215a1521 Signed-off-by: Sam Lewis sam.vr.lewis@gmail.com --- M src/arch/arm/armv7/mmu.c M src/arch/arm/include/armv7/arch/cache.h 2 files changed, 8 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/43137/1
diff --git a/src/arch/arm/armv7/mmu.c b/src/arch/arm/armv7/mmu.c index d823c61..51b4860 100644 --- a/src/arch/arm/armv7/mmu.c +++ b/src/arch/arm/armv7/mmu.c @@ -118,7 +118,7 @@
/* Invalidate the TLB entries. */ for (i = start_idx; i < end_idx; i++) - tlbimvaa(offset + (i << shift)); + tlbimva(offset + (i << shift)); dsb(); isb(); } @@ -152,7 +152,7 @@ *pgd_entry = (pte_t)(uintptr_t)table | ATTR_NEXTLEVEL; dccmvac((uintptr_t)pgd_entry); dsb(); - tlbimvaa(start_addr); + tlbimva(start_addr); dsb(); isb();
diff --git a/src/arch/arm/include/armv7/arch/cache.h b/src/arch/arm/include/armv7/arch/cache.h index 600ec46..efd1a19 100644 --- a/src/arch/arm/include/armv7/arch/cache.h +++ b/src/arch/arm/include/armv7/arch/cache.h @@ -79,6 +79,12 @@ asm volatile ("mcr p15, 0, %0, c8, c7, 3" : : "r" (mva) : "memory"); }
+/* invalidate unified TLB by MVA and ASID */ +static inline void tlbimva(unsigned long mva) +{ + asm volatile ("mcr p15, 0, %0, c8, c7, 1" : : "r" (mva) : "memory"); +} + /* write data access control register (DACR) */ static inline void write_dacr(uint32_t val) {
Sam Lewis has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
Patch Set 1:
I'm new to coreboot and using Gerrit so please don't hesitate to let me know if there's anything I should change about this patch!
I don't have any armv7 hardware that coreboot already works on (and the QEMU port doesn't seem like it currently works?), so would appreciate help in testing this. That said - if I understand the ARM architecture reference manual, it should be a safe change to make because (as stated in the commit) the tlbimva/tlbimvaa operations are equivalent when used on global TLB pages.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
Patch Set 1:
(2 comments)
I don't have any armv7 hardware that coreboot already works on (and the QEMU port doesn't seem like it currently works?), so would appreciate help in testing this. That said - if I understand the ARM architecture reference manual, it should be a safe change to make because (as stated in the commit) the tlbimva/tlbimvaa operations are equivalent when used on global TLB pages.
Thanks for the patch and welcome to coreboot! Based on your explanation and that you have tested it on your hardware, I'm happy to just take this patch without further testing. If it works for you it should work on other platforms too.
Please check out the line comments and upload another version with those addressed (at least the one about removing the function), then I'll approve it!
https://review.coreboot.org/c/coreboot/+/43137/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/43137/1//COMMIT_MSG@25 PS1, Line 25: b nit: 'v'
https://review.coreboot.org/c/coreboot/+/43137/1/src/arch/arm/include/armv7/... File src/arch/arm/include/armv7/arch/cache.h:
https://review.coreboot.org/c/coreboot/+/43137/1/src/arch/arm/include/armv7/... PS1, Line 77: static inline void tlbimvaa(unsigned long mva) Let's just remove this function completely so we can be sure it's not still accidentally used anywhere.
Hello build bot (Jenkins), Julius Werner,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/43137
to look at the new patch set (#2).
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
armv7: mmu: Use 'tlbimva' to invalidate TLB entries
The tlbimvaa operation (invalidate unified TLB by MVA, all address space identifiers) is only available on armv7 processors that support Multiprocessing Extensions. When used on processors that do not support the extensions it causes an "undefined instruction" exception.
This patch changes the MMU table entry filling code to use the tlbimva (invalidate unified TLB entry by MVA and address space identifier) operation for invalidating TLB entries, which is supported on all armv7 processors.
As address space identifiers are not used in TLB entries in coreboot (all entries are set as global), these two operations can safely be used interchangeably. The ASID value supplied to the operation is not checked for global TLB entries.
More information as well as the data formats for the tlbimvaa and tlbimva operations are detailed in the "ARM Architecture Reference Manual ARMv7-A" edition, issue "C.c" page B4-1747.
TEST: Booted Beaglebone Black (my current in progress port)
Change-Id: Ie7dfb4adab20dc7eecb1b20aa2ee6355215a1521 Signed-off-by: Sam Lewis sam.vr.lewis@gmail.com --- M src/arch/arm/armv7/mmu.c M src/arch/arm/include/armv7/arch/cache.h 2 files changed, 5 insertions(+), 5 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/43137/2
Sam Lewis has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
Patch Set 2:
Thanks for the patch and welcome to coreboot! Based on your explanation and that you have tested it on your hardware, I'm happy to just take this patch without further testing. If it works for you it should work on other platforms too.
Please check out the line comments and upload another version with those addressed (at least the one about removing the function), then I'll approve it!
Thanks for the speedy reply and warm welcome, Julius! Have submitted a second version that addresses your comments.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
Patch Set 2: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/43137/1/src/arch/arm/include/armv7/... File src/arch/arm/include/armv7/arch/cache.h:
https://review.coreboot.org/c/coreboot/+/43137/1/src/arch/arm/include/armv7/... PS1, Line 77: static inline void tlbimvaa(unsigned long mva)
Let's just remove this function completely so we can be sure it's not still accidentally used anywhe […]
Done
(You always have to mark comments you addressed as "Resolved" in Gerrit. It won't let us merge patches if there are still comments open.)
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/43137 )
Change subject: armv7: mmu: Use 'tlbimva' to invalidate TLB entries ......................................................................
armv7: mmu: Use 'tlbimva' to invalidate TLB entries
The tlbimvaa operation (invalidate unified TLB by MVA, all address space identifiers) is only available on armv7 processors that support Multiprocessing Extensions. When used on processors that do not support the extensions it causes an "undefined instruction" exception.
This patch changes the MMU table entry filling code to use the tlbimva (invalidate unified TLB entry by MVA and address space identifier) operation for invalidating TLB entries, which is supported on all armv7 processors.
As address space identifiers are not used in TLB entries in coreboot (all entries are set as global), these two operations can safely be used interchangeably. The ASID value supplied to the operation is not checked for global TLB entries.
More information as well as the data formats for the tlbimvaa and tlbimva operations are detailed in the "ARM Architecture Reference Manual ARMv7-A" edition, issue "C.c" page B4-1747.
TEST: Booted Beaglebone Black (my current in progress port)
Change-Id: Ie7dfb4adab20dc7eecb1b20aa2ee6355215a1521 Signed-off-by: Sam Lewis sam.vr.lewis@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/43137 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Julius Werner jwerner@chromium.org --- M src/arch/arm/armv7/mmu.c M src/arch/arm/include/armv7/arch/cache.h 2 files changed, 5 insertions(+), 5 deletions(-)
Approvals: build bot (Jenkins): Verified Julius Werner: Looks good to me, approved
diff --git a/src/arch/arm/armv7/mmu.c b/src/arch/arm/armv7/mmu.c index d823c61..51b4860 100644 --- a/src/arch/arm/armv7/mmu.c +++ b/src/arch/arm/armv7/mmu.c @@ -118,7 +118,7 @@
/* Invalidate the TLB entries. */ for (i = start_idx; i < end_idx; i++) - tlbimvaa(offset + (i << shift)); + tlbimva(offset + (i << shift)); dsb(); isb(); } @@ -152,7 +152,7 @@ *pgd_entry = (pte_t)(uintptr_t)table | ATTR_NEXTLEVEL; dccmvac((uintptr_t)pgd_entry); dsb(); - tlbimvaa(start_addr); + tlbimva(start_addr); dsb(); isb();
diff --git a/src/arch/arm/include/armv7/arch/cache.h b/src/arch/arm/include/armv7/arch/cache.h index 600ec46..e332c31 100644 --- a/src/arch/arm/include/armv7/arch/cache.h +++ b/src/arch/arm/include/armv7/arch/cache.h @@ -73,10 +73,10 @@ asm volatile ("mcr p15, 0, %0, c8, c7, 0" : : "r" (0) : "memory"); }
-/* invalidate unified TLB by MVA, all ASID */ -static inline void tlbimvaa(unsigned long mva) +/* invalidate unified TLB by MVA and ASID */ +static inline void tlbimva(unsigned long mva) { - asm volatile ("mcr p15, 0, %0, c8, c7, 3" : : "r" (mva) : "memory"); + asm volatile ("mcr p15, 0, %0, c8, c7, 1" : : "r" (mva) : "memory"); }
/* write data access control register (DACR) */
-
Julius Werner (Code Review) -
Sam Lewis (Code Review)