Re: [PATCH 03/21] hw: aspeed_scu: Add AST2600 support

[Andrew Jeffery]

On Thu, 19 Sep 2019, at 15:19, Cédric Le Goater wrote:
> From: Joel Stanley <address@hidden>
> 
> The SCU controller on the AST2600 SoC has extra registers. Increase
> the number of regs of the model and introduce a new field in the class
> to customize the MemoryRegion operations depending on the SoC model.
> 
> Signed-off-by: Joel Stanley <address@hidden>
> [clg: - improved commit log
>       - changed vmstate version
>       - reworked model integration into new objet class ]
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---
>  include/hw/misc/aspeed_scu.h |   7 +-
>  hw/misc/aspeed_scu.c         | 190 +++++++++++++++++++++++++++++++++--
>  2 files changed, 189 insertions(+), 8 deletions(-)

...

> +static void aspeed_ast2600_scu_write(void *opaque, hwaddr offset, 
> uint64_t data,
> +                                     unsigned size)
> +{
> +    AspeedSCUState *s = ASPEED_SCU(opaque);
> +    int reg = TO_REG(offset);
> +
> +    if (reg >= ASPEED_AST2600_SCU_NR_REGS) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Out-of-bounds write at offset 0x%" 
> HWADDR_PRIx "\n",
> +                      __func__, offset);
> +        return;
> +    }
> +
> +    if (reg > PROT_KEY && !s->regs[PROT_KEY]) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: SCU is locked!\n", 
> __func__);
> +    }
> +
> +    trace_aspeed_scu_write(offset, size, data);
> +
> +    switch (reg) {
> +    case AST2600_PROT_KEY:
> +        s->regs[reg] = (data == ASPEED_SCU_PROT_KEY) ? 1 : 0;
> +        return;
> +    case AST2600_HW_STRAP1:
> +    case AST2600_HW_STRAP2:
> +        if (s->regs[reg + 2]) {
> +            return;
> +        }
> +        /* fall through */
> +    case AST2600_SYS_RST_CTRL:
> +    case AST2600_SYS_RST_CTRL2:
> +        /* W1S (Write 1 to set) registers */
> +        s->regs[reg] |= data;
> +        return;
> +    case AST2600_SYS_RST_CTRL_CLR:
> +    case AST2600_SYS_RST_CTRL2_CLR:
> +    case AST2600_HW_STRAP1_CLR:
> +    case AST2600_HW_STRAP2_CLR:
> +        /* W1C (Write 1 to clear) registers */
> +        s->regs[reg] &= ~data;

This clear should respect the protection register for each strap case.

Andrew