Although I agree with the logic, I would like to see the magic numbers replaced
with #defines.
> + switch ((Mmfr >> 12) & 0xf) {
For maintainability and isolation purposes I expect that we would have a header
file (or files) for architectural defines like MMFR0 definitions. In this case
I would expect this to be in an ARMv7 architecture definition header file.
For UefiCpuPackage the convention of Include/Register/<somefile.h> is used,
perhaps something similar for ARMv7 and ARMv8 architecture definitions can be
used in ArmPkg so the definiton can come in with #include <Register/Armv7.h> or
something like that.
I know for many processor ARM provides XML definitions for registers. Are the
architectural register definitions available as XML such that header files can
be generated instead of manually entered?
Thanks for the update - this is much better.
Eugene
> -----Original Message-----
> From: Ard Biesheuvel [mailto:[email protected]]
> Sent: Tuesday, December 08, 2015 7:03 AM
> To: [email protected]; [email protected]; Cohen, Eugene
> <[email protected]>
> Cc: Ard Biesheuvel <[email protected]>
> Subject: [PATCH] ArmPkg/ArmV7Mmu: prefer non shareable memory
> on non-coherent hardware
>
> Commit SVN r18778 made all mappings of normal memory (inner)
> shareable,
> even on hardware that implements shareability as uncached accesses.
> The original concerns that prompted the change, regarding coherent
> DMA
> and virt guests migrating between CPUs, do not apply to such
> hardware,
> so revert to the original behavior in that case.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <[email protected]>
> ---
> ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c | 38
> ++++++++++++++++++--
> 1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c
> b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c
> index 23d2e43beba0..8aecbe8cc354 100644
> --- a/ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c
> +++ b/ArmPkg/Library/ArmLib/ArmV7/ArmV7Mmu.c
> @@ -42,6 +42,38 @@ ConvertSectionAttributesToPageAttributes (
> }
>
> STATIC
> +BOOLEAN
> +PreferNonshareableMemory (
> + VOID
> + )
> +{
> + UINTN Mmfr;
> + BOOLEAN Result;
> +
> + if (FeaturePcdGet (PcdNormalMemoryNonshareableOverride)) {
> + return TRUE;
> + }
> +
> + //
> + // Check whether the innermost level of shareability (the level we
> will use
> + // by default to map normal memory) is implemented with
> hardware coherency
> + // support. Otherwise, revert to mapping as non-shareable.
> + //
> + Mmfr = ArmReadIdMmfr0 ();
> + switch ((Mmfr >> 12) & 0xf) {
> + case 0x1:
> + // two levels of shareability
> + Result = ((Mmfr >> 28) & 0xf) != 1;
> + break;
> + default:
> + // one level of shareability
> + Result = ((Mmfr >> 8) & 0xf) != 1;
> + break;
> + }
> + return Result;
> +}
> +
> +STATIC
> VOID
> PopulateLevel2PageTable (
> IN UINT32 *SectionEntry,
> @@ -80,7 +112,7 @@ PopulateLevel2PageTable (
> break;
> }
>
> - if (FeaturePcdGet(PcdNormalMemoryNonshareableOverride)) {
> + if (PreferNonshareableMemory ()) {
> PageAttributes &= ~TT_DESCRIPTOR_PAGE_S_SHARED;
> }
>
> @@ -189,7 +221,7 @@ FillTranslationTable (
> break;
> }
>
> - if (FeaturePcdGet(PcdNormalMemoryNonshareableOverride)) {
> + if (PreferNonshareableMemory ()) {
> Attributes &= ~TT_DESCRIPTOR_SECTION_S_SHARED;
> }
>
> @@ -281,7 +313,7 @@ ArmConfigureMmu (
> }
>
> if (TTBRAttributes & TTBR_SHAREABLE) {
> - if (FeaturePcdGet(PcdNormalMemoryNonshareableOverride)) {
> + if (PreferNonshareableMemory ()) {
> TTBRAttributes ^= TTBR_SHAREABLE;
> } else {
> //
> --
> 1.9.1
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
