Hi Ard, On Mon, Nov 09, 2015 at 02:18:58PM +0100, Ard Biesheuvel wrote: > Mark all cached memory mappings as shareable (or inner shareable on > AArch64) so that our view of memory is kept coherent by the hardware. > > This is primarily relevant under virtualization (where a guest may migrate > to another core) but in general, since UEFI on ARM is mostly used in a > context where the secure firmware and possibly a secure OS are already up > and running, it is best to refrain from using any non-shareable mappings.
Thanks, this is both an important correctness fix and nice code cleanup. I ran into an issue while testing this, which is why I haven't responded to this, but I've bisected it down to r18503/3a05b13, and am looking into what causes an issue with Linux booting. Regardless - this patch wasn't the cause of my issue, and it looks fine. Reviewed-by: Leif Lindholm <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > ArmPkg/Include/Chipset/AArch64Mmu.h | 5 +++++ > ArmPkg/Include/Chipset/ArmV7Mmu.h | 8 ++++---- > ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c | 21 ++++++++++---------- > 3 files changed, 19 insertions(+), 15 deletions(-) > > diff --git a/ArmPkg/Include/Chipset/AArch64Mmu.h > b/ArmPkg/Include/Chipset/AArch64Mmu.h > index 22e492d61dc0..3c3df6d9835c 100644 > --- a/ArmPkg/Include/Chipset/AArch64Mmu.h > +++ b/ArmPkg/Include/Chipset/AArch64Mmu.h > @@ -74,6 +74,11 @@ > #define TT_NS BIT5 > #define TT_AF BIT10 > > +#define TT_SH_NON_SHAREABLE (0x0 << 8) > +#define TT_SH_OUTER_SHAREABLE (0x2 << 8) > +#define TT_SH_INNER_SHAREABLE (0x3 << 8) > +#define TT_SH_MASK (0x3 << 8) > + > #define TT_PXN_MASK BIT53 > #define TT_UXN_MASK BIT54 // EL1&0 > #define TT_XN_MASK BIT54 // EL2 / EL3 > diff --git a/ArmPkg/Include/Chipset/ArmV7Mmu.h > b/ArmPkg/Include/Chipset/ArmV7Mmu.h > index 24ab1755faa7..aaa0977205fa 100644 > --- a/ArmPkg/Include/Chipset/ArmV7Mmu.h > +++ b/ArmPkg/Include/Chipset/ArmV7Mmu.h > @@ -175,14 +175,14 @@ > #define TT_DESCRIPTOR_SECTION_WRITE_BACK(NonSecure) > (TT_DESCRIPTOR_SECTION_TYPE_SECTION > | \ > ((NonSecure) ? > TT_DESCRIPTOR_SECTION_NS : 0) | \ > > TT_DESCRIPTOR_SECTION_NG_GLOBAL | \ > - > TT_DESCRIPTOR_SECTION_S_NOT_SHARED | \ > + > TT_DESCRIPTOR_SECTION_S_SHARED | \ > > TT_DESCRIPTOR_SECTION_DOMAIN(0) | \ > > TT_DESCRIPTOR_SECTION_AP_RW_RW | \ > > TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_BACK_ALLOC) > #define TT_DESCRIPTOR_SECTION_WRITE_THROUGH(NonSecure) > (TT_DESCRIPTOR_SECTION_TYPE_SECTION > | \ > ((NonSecure) ? > TT_DESCRIPTOR_SECTION_NS : 0) | \ > > TT_DESCRIPTOR_SECTION_NG_GLOBAL | \ > - > TT_DESCRIPTOR_SECTION_S_NOT_SHARED | \ > + > TT_DESCRIPTOR_SECTION_S_SHARED | \ > > TT_DESCRIPTOR_SECTION_DOMAIN(0) | \ > > TT_DESCRIPTOR_SECTION_AP_RW_RW | \ > > TT_DESCRIPTOR_SECTION_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC) > @@ -203,12 +203,12 @@ > > #define TT_DESCRIPTOR_PAGE_WRITE_BACK > (TT_DESCRIPTOR_PAGE_TYPE_PAGE > | \ > > TT_DESCRIPTOR_PAGE_NG_GLOBAL > | \ > - > TT_DESCRIPTOR_PAGE_S_NOT_SHARED > | \ > + > TT_DESCRIPTOR_PAGE_S_SHARED > | \ > > TT_DESCRIPTOR_PAGE_AP_RW_RW > | \ > > TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_BACK_ALLOC) > #define TT_DESCRIPTOR_PAGE_WRITE_THROUGH > (TT_DESCRIPTOR_PAGE_TYPE_PAGE > | \ > > TT_DESCRIPTOR_PAGE_NG_GLOBAL > | \ > - > TT_DESCRIPTOR_PAGE_S_NOT_SHARED > | \ > + > TT_DESCRIPTOR_PAGE_S_SHARED > | \ > > TT_DESCRIPTOR_PAGE_AP_RW_RW > | \ > > TT_DESCRIPTOR_PAGE_CACHE_POLICY_WRITE_THROUGH_NO_ALLOC) > #define TT_DESCRIPTOR_PAGE_DEVICE > (TT_DESCRIPTOR_PAGE_TYPE_PAGE > | \ > diff --git a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > index e40c09ae9685..8829c6286b36 100644 > --- a/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > +++ b/ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c > @@ -34,23 +34,22 @@ ArmMemoryAttributeToPageAttribute ( > { > switch (Attributes) { > case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK: > - return TT_ATTR_INDX_MEMORY_WRITE_BACK; > - case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH: > - return TT_ATTR_INDX_MEMORY_WRITE_THROUGH; > - case ARM_MEMORY_REGION_ATTRIBUTE_DEVICE: > - return TT_ATTR_INDX_DEVICE_MEMORY; > - case ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED: > - return TT_ATTR_INDX_MEMORY_NON_CACHEABLE; > case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK: > - return TT_ATTR_INDX_MEMORY_WRITE_BACK; > + return TT_ATTR_INDX_MEMORY_WRITE_BACK | TT_SH_INNER_SHAREABLE; > + > + case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH: > case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH: > - return TT_ATTR_INDX_MEMORY_WRITE_THROUGH; > - case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_DEVICE: > - return TT_ATTR_INDX_DEVICE_MEMORY; > + return TT_ATTR_INDX_MEMORY_WRITE_THROUGH | TT_SH_INNER_SHAREABLE; > + > + // Uncached and device mappings are treated as outer shareable by default, > + case ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED: > case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED: > return TT_ATTR_INDX_MEMORY_NON_CACHEABLE; > + > default: > ASSERT(0); > + case ARM_MEMORY_REGION_ATTRIBUTE_DEVICE: > + case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_DEVICE: > return TT_ATTR_INDX_DEVICE_MEMORY; > } > } > -- > 1.9.1 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
