1. Patchew
  2. QEMU
  3. View series

[PATCH v3] hw/misc/aspeed_scu: Handle AST2600 protection key registers correctly

Tan Siewert posted 1 patch 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250613181906.50078-1-tan@siewert.io
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>
There is a newer version of this series
hw/misc/aspeed_scu.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
[PATCH v3] hw/misc/aspeed_scu: Handle AST2600 protection key registers correctly
Posted by Tan Siewert 8 months ago 
The AST2600 SCU has two protection key registers (0x00 and 0x10) that
both need to be unlocked. (Un-)locking 0x00 modifies both protection key
registers, while modifying 0x10 only modifies itself.

This commit updates the SCU write logic to reject writes unless both
protection key registers are unlocked, matching the behaviour of
real hardware.

Signed-off-by: Tan Siewert <tan@siewert.io>
---
V3:
  - Change logic to unlock both registers on 0x00 write, while writing
    to 0x10 only modifies itself. [Jamin]

 hw/misc/aspeed_scu.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
index 4930e00fed..993cb800a8 100644
--- a/hw/misc/aspeed_scu.c
+++ b/hw/misc/aspeed_scu.c
@@ -91,6 +91,7 @@
 #define BMC_DEV_ID           TO_REG(0x1A4)
 
 #define AST2600_PROT_KEY          TO_REG(0x00)
+#define AST2600_PROT_KEY2         TO_REG(0x10)
 #define AST2600_SILICON_REV       TO_REG(0x04)
 #define AST2600_SILICON_REV2      TO_REG(0x14)
 #define AST2600_SYS_RST_CTRL      TO_REG(0x40)
@@ -722,6 +723,7 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
     int reg = TO_REG(offset);
     /* Truncate here so bitwise operations below behave as expected */
     uint32_t data = data64;
+    bool unlocked = s->regs[AST2600_PROT_KEY] && s->regs[AST2600_PROT_KEY2];
 
     if (reg >= ASPEED_AST2600_SCU_NR_REGS) {
         qemu_log_mask(LOG_GUEST_ERROR,
@@ -730,15 +732,25 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
         return;
     }
 
-    if (reg > PROT_KEY && !s->regs[PROT_KEY]) {
+    if ((reg != AST2600_PROT_KEY || reg != AST2600_PROT_KEY2) && !unlocked) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
+        return;
     }
 
     trace_aspeed_scu_write(offset, size, data);
 
     switch (reg) {
     case AST2600_PROT_KEY:
-        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
+        /*
+         * Writing a value to SCU000 will modify both protection
+         * registers to each protection register individually.
+         */
+        u32 value = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
+        s->regs[AST2600_PROT_KEY] = value;
+        s->regs[AST2600_PROT_KEY2] = value;
+        return;
+    case AST2600_PROT_KEY2:
+        s->regs[AST2600_PROT_KEY2] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
         return;
     case AST2600_HW_STRAP1:
     case AST2600_HW_STRAP2:
-- 
2.49.0
Re: [PATCH v3] hw/misc/aspeed_scu: Handle AST2600 protection key registers correctly
Posted by Cédric Le Goater 7 months, 3 weeks ago 
On 6/13/25 20:19, Tan Siewert wrote:
> The AST2600 SCU has two protection key registers (0x00 and 0x10) that
> both need to be unlocked. (Un-)locking 0x00 modifies both protection key
> registers, while modifying 0x10 only modifies itself.
> 
> This commit updates the SCU write logic to reject writes unless both
> protection key registers are unlocked, matching the behaviour of
> real hardware.
> 
> Signed-off-by: Tan Siewert <tan@siewert.io>
> ---
> V3:
>    - Change logic to unlock both registers on 0x00 write, while writing
>      to 0x10 only modifies itself. [Jamin]
> 
>   hw/misc/aspeed_scu.c | 16 ++++++++++++++--
>   1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
> index 4930e00fed..993cb800a8 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -91,6 +91,7 @@
>   #define BMC_DEV_ID           TO_REG(0x1A4)
>   
>   #define AST2600_PROT_KEY          TO_REG(0x00)
> +#define AST2600_PROT_KEY2         TO_REG(0x10)
>   #define AST2600_SILICON_REV       TO_REG(0x04)
>   #define AST2600_SILICON_REV2      TO_REG(0x14)
>   #define AST2600_SYS_RST_CTRL      TO_REG(0x40)
> @@ -722,6 +723,7 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
>       int reg = TO_REG(offset);
>       /* Truncate here so bitwise operations below behave as expected */
>       uint32_t data = data64;
> +    bool unlocked = s->regs[AST2600_PROT_KEY] && s->regs[AST2600_PROT_KEY2];
>   
>       if (reg >= ASPEED_AST2600_SCU_NR_REGS) {
>           qemu_log_mask(LOG_GUEST_ERROR,
> @@ -730,15 +732,25 @@ static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset,
>           return;
>       }
>   
> -    if (reg > PROT_KEY && !s->regs[PROT_KEY]) {
> +    if ((reg != AST2600_PROT_KEY || reg != AST2600_PROT_KEY2) && !unlocked) {
>           qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", __func__);
> +        return;
>       }
>   
>       trace_aspeed_scu_write(offset, size, data);
>   
>       switch (reg) {
>       case AST2600_PROT_KEY:
> -        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        /*
> +         * Writing a value to SCU000 will modify both protection
> +         * registers to each protection register individually.
> +         */
> +        u32 value = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;

u32 -> uint32_t and it is best to declare variables at the top
of the function.

Thanks,

C.



> +        s->regs[AST2600_PROT_KEY] = value;
> +        s->regs[AST2600_PROT_KEY2] = value;
> +        return;
> +    case AST2600_PROT_KEY2:
> +        s->regs[AST2600_PROT_KEY2] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
>           return;
>       case AST2600_HW_STRAP1:
>       case AST2600_HW_STRAP2:
RE: [PATCH v3] hw/misc/aspeed_scu: Handle AST2600 protection key registers correctly
Posted by Jamin Lin 7 months, 3 weeks ago 

> Subject: [PATCH v3] hw/misc/aspeed_scu: Handle AST2600 protection key
> registers correctly
> 
> The AST2600 SCU has two protection key registers (0x00 and 0x10) that both
> need to be unlocked. (Un-)locking 0x00 modifies both protection key registers,
> while modifying 0x10 only modifies itself.
> 
> This commit updates the SCU write logic to reject writes unless both protection
> key registers are unlocked, matching the behaviour of real hardware.
> 
> Signed-off-by: Tan Siewert <tan@siewert.io>
> ---
> V3:
>   - Change logic to unlock both registers on 0x00 write, while writing
>     to 0x10 only modifies itself. [Jamin]
> 
>  hw/misc/aspeed_scu.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c index
> 4930e00fed..993cb800a8 100644
> --- a/hw/misc/aspeed_scu.c
> +++ b/hw/misc/aspeed_scu.c
> @@ -91,6 +91,7 @@
>  #define BMC_DEV_ID           TO_REG(0x1A4)
> 
>  #define AST2600_PROT_KEY          TO_REG(0x00)
> +#define AST2600_PROT_KEY2         TO_REG(0x10)
>  #define AST2600_SILICON_REV       TO_REG(0x04)
>  #define AST2600_SILICON_REV2      TO_REG(0x14)
>  #define AST2600_SYS_RST_CTRL      TO_REG(0x40)
> @@ -722,6 +723,7 @@ static void aspeed_ast2600_scu_write(void *opaque,
> hwaddr offset,
>      int reg = TO_REG(offset);
>      /* Truncate here so bitwise operations below behave as expected */
>      uint32_t data = data64;
> +    bool unlocked = s->regs[AST2600_PROT_KEY] &&
> + s->regs[AST2600_PROT_KEY2];
> 
>      if (reg >= ASPEED_AST2600_SCU_NR_REGS) {
>          qemu_log_mask(LOG_GUEST_ERROR,
> @@ -730,15 +732,25 @@ static void aspeed_ast2600_scu_write(void *opaque,
> hwaddr offset,
>          return;
>      }
> 
> -    if (reg > PROT_KEY && !s->regs[PROT_KEY]) {
> +    if ((reg != AST2600_PROT_KEY || reg != AST2600_PROT_KEY2) &&
> + !unlocked) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n",
> __func__);
> +        return;
>      }
> 
>      trace_aspeed_scu_write(offset, size, data);
> 
>      switch (reg) {
>      case AST2600_PROT_KEY:
> -        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        /*
> +         * Writing a value to SCU000 will modify both protection
> +         * registers to each protection register individually.
> +         */
> +        u32 value = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        s->regs[AST2600_PROT_KEY] = value;
> +        s->regs[AST2600_PROT_KEY2] = value;
> +        return;
> +    case AST2600_PROT_KEY2:
> +        s->regs[AST2600_PROT_KEY2] = (data == ASPEED_SCU_PROT_KEY) ?
> 1
> + : 0;
>          return;
>      case AST2600_HW_STRAP1:
>      case AST2600_HW_STRAP2:
> --
> 2.49.0


Reviewed-by: Jamin Lin <jamin_lin@aspeedtech.com>

Thanks-Jamin

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.