On 11/09/2015 07:34 PM, Ard Biesheuvel wrote:
On 9 November 2015 at 09:00, Heyi Guo <[email protected]> wrote:
Hi All,

In AARCH64 MMU configuration code, sharable attribute will be set to inner
sharable when memory attribute is write back. However, I have 2 questions
about this code:
1. Why is it set to inner sharable only when memory attribute is write back?
Either write through or uncached will use non-shareable for TCR.
2. All memory regions will be set to none shareable in AARCH64 MMU
configuration code, whatever cachable attribute they have (WB, WT or UC). In
my opinion, TCR attribute should be kept consistent with the memory where
translation tables are located, i.e. TCR_SH_NON_SHAREABLE will always be the
proper attribute for TCR.

So could you help to confirm whether there is anything wrong with the code?

You are right: this is incorrect. Leo Duran of AMD contributed a patch
at some point that addressed this, I think, but I don't know what
happened to it.

We could probably get away with using inner shareable for all mappings
  (since uncached mappings are treated as outer shareable anyway), but
for correctness, let's set everything explicitly.
Then, the TCR bits should just reflect the shareability of the mappings.

I will cook up a patch in a minute.

Did you observe any actual issues? Or did you find this by code inspection only?

Yes, we sometimes got issue when we change memory attribute for one region in DXE phase; the resulted translation table is not the vale as we expect.

Heyi


Thanks,
Ard.




ArmPkg/Library/ArmLib/AArch64/AArch64Mmu.c


   if ((TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_UNCACHED_UNBUFFERED) ||
       (TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_UNCACHED_UNBUFFERED)) {
     TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_NON_CACHEABLE |
TCR_RGN_INNER_NON_CACHEABLE;
   } else if ((TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK) ||
       (TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK)) {

++    TCR |= **TCR_SH_INNER_SHAREABLE** | TCR_RGN_OUTER_WRITE_BACK_ALLOC |
TCR_RGN_INNER_WRITE_BACK_ALLOC;


   } else if ((TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH) ||
       (TranslationTableAttribute ==
ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH)) {
     TCR |= TCR_SH_NON_SHAREABLE | TCR_RGN_OUTER_WRITE_THROUGH |
TCR_RGN_INNER_WRITE_THROUGH;


_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to