> 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 <t...@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_...@aspeedtech.com>
Thanks-Jamin
