On 12 November 2015 at 12:35, Leif Lindholm <leif.lindh...@linaro.org> wrote:
> 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 <leif.lindh...@linaro.org>
>
Thanks. Committed as SVN r18778
--
Ard.
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> ---
>> 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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
