diff mbox

[2/2] target/arm: lie more convincingly about memory attributes in PAR_EL1

Message ID 20170228215801.10472-3-Andrew.Baumann@microsoft.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrew Baumann Feb. 28, 2017, 9:58 p.m. UTC
On a successful long-descriptor translation, PAR_EL1 bits 56:63 are
expected to report the memory attributes of the page. However, the
page table walker (get_phys_addr()) does not currently retrieve these
attributes. Rather than leaving these bits clear (which implies
uncacheable device memory), this change sets them to 0xff, which
corresponds to write-back cached normal memory.

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
In my (biased!) opinion, this is a better lie to tell for an emulated
environment. It also happens to un-break the Windows boot process, and
enable an NT kernel optimisation (DC ZVA for page zeroing).

 target/arm/helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Peter Maydell Feb. 28, 2017, 10:52 p.m. UTC | #1
On 28 February 2017 at 21:58, Andrew Baumann
<Andrew.Baumann@microsoft.com> wrote:
> On a successful long-descriptor translation, PAR_EL1 bits 56:63 are
> expected to report the memory attributes of the page. However, the
> page table walker (get_phys_addr()) does not currently retrieve these
> attributes. Rather than leaving these bits clear (which implies
> uncacheable device memory), this change sets them to 0xff, which
> corresponds to write-back cached normal memory.
>
> Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
> ---
> In my (biased!) opinion, this is a better lie to tell for an emulated
> environment. It also happens to un-break the Windows boot process, and
> enable an NT kernel optimisation (DC ZVA for page zeroing).

It's not clear to me that misreporting Device memory as
Normal is any better than misreporting Normal memory as
Device -- at least, it seems like it might break currently
working guests.

I think it would be safer to implement the PAR as "honour the
attribute index bits in page table descriptors and return
the corresponding bits from the MAIR registers".

thanks
-- PMM
diff mbox

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 760092a..b858d6d 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2159,7 +2159,13 @@  static uint64_t do_ats_write(CPUARMState *env, uint64_t value,
             if (!attrs.secure) {
                 par64 |= (1 << 9); /* NS */
             }
-            /* We don't set the ATTR or SH fields in the PAR. */
+            /* ATTR bits are all set, which implies:
+             *   normal memory
+             *   outer write-back non-transient, read/write-allocate
+             *   inner write-back non-transient, read/write-allocate
+             */
+            par64 |= ((uint64_t)0xff << 56); /* ATTR */
+            /* We don't set the SH field in the PAR. */
         } else {
             par64 |= 1; /* F */
             par64 |= (fsr & 0x3f) << 1; /* FS */