diff mbox series

[PULL,04/32] Fix ast2500 protection register emulation

Message ID 20180222152307.7499-5-peter.maydell@linaro.org
State Accepted
Commit 5c1d3a2b6e5c084696b57edc33aa4bb554e7c37b
Headers show
Series target-arm queue | expand

Commit Message

Peter Maydell Feb. 22, 2018, 3:22 p.m. UTC 
From: Hugo Landau <hlandau@devever.net>


Some register blocks of the ast2500 are protected by protection key
registers which require the right magic value to be written to those
registers to allow those registers to be mutated.

Register manuals indicate that writing the correct magic value to these
registers should cause subsequent reads from those values to return 1,
and writing any other value should cause subsequent reads to return 0.

Previously, qemu implemented these registers incorrectly: the registers
were handled as simple memory, meaning that writing some value x to a
protection key register would result in subsequent reads from that
register returning the same value x. The protection was implemented by
ensuring that the current value of that register equaled the magic
value.

This modifies qemu to have the correct behaviour: attempts to write to a
ast2500 protection register results in a transition to 1 or 0 depending
on whether the written value is the correct magic. The protection logic
is updated to ensure that the value of the register is nonzero.

This bug caused deadlocks with u-boot HEAD: when u-boot is done with a
protectable register block, it attempts to lock it by writing the
bitwise inverse of the correct magic value, and then spinning forever
until the register reads as zero. Since qemu implemented writes to these
registers as ordinary memory writes, writing the inverse of the magic
value resulted in subsequent reads returning that value, leading to
u-boot spinning forever.

Signed-off-by: Hugo Landau <hlandau@devever.net>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Acked-by: Andrew Jeffery <andrew@aj.id.au>

Message-id: 20180220132627.4163-1-hlandau@devever.net
[PMM: fixed incorrect code indentation]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 hw/misc/aspeed_scu.c  | 6 +++++-
 hw/misc/aspeed_sdmc.c | 8 +++++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.16.1
diff mbox series

Patch

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 74537ce975..5e6d5744ee 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -191,7 +191,7 @@  static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
     }
 
     if (reg > PROT_KEY && reg < CPU2_BASE_SEG1 &&
-            s->regs[PROT_KEY] != ASPEED_SCU_PROT_KEY) {
+            !s->regs[PROT_KEY]) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
         return;
     }
@@ -199,6 +199,10 @@  static void aspeed_scu_write(void *opaque, hwaddr offset, uint64_t data,
     trace_aspeed_scu_write(offset, size, data);
 
     switch (reg) {
+    case PROT_KEY:
+        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
+        return;
+
     case FREQ_CNTR_EVAL:
     case VGA_SCRATCH1 ... VGA_SCRATCH8:
     case RNG_DATA:
diff --git a/hw/misc/aspeed_sdmc.c b/hw/misc/aspeed_sdmc.c
index f0b3053fae..0df008e52a 100644
--- a/hw/misc/aspeed_sdmc.c
+++ b/hw/misc/aspeed_sdmc.c
@@ -110,7 +110,12 @@  static void aspeed_sdmc_write(void *opaque, hwaddr addr, uint64_t data,
         return;
     }
 
-    if (addr != R_PROT && s->regs[R_PROT] != PROT_KEY_UNLOCK) {
+    if (addr == R_PROT) {
+        s->regs[addr] = (data == PROT_KEY_UNLOCK) ? 1 : 0;
+        return;
+    }
+
+    if (!s->regs[R_PROT]) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: SDMC is locked!\n", __func__);
         return;
     }
@@ -123,6 +128,7 @@  static void aspeed_sdmc_write(void *opaque, hwaddr addr, uint64_t data,
             data &= ~ASPEED_SDMC_READONLY_MASK;
             break;
         case AST2500_A0_SILICON_REV:
+        case AST2500_A1_SILICON_REV:
             data &= ~ASPEED_SDMC_AST2500_READONLY_MASK;
             break;
         default: