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

[Cédric Le Goater]

On 21/09/2019 06:37, Joel Stanley wrote:
> On Fri, 20 Sep 2019 at 15:15, Cédric Le Goater <address@hidden> wrote:
>>
>> On 20/09/2019 06:10, Andrew Jeffery wrote:
>>>
>>>
>>> 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.
>>>>
>>>> +    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.
>>
>> Joel,
>>
>> You are the expert ! :)
> 
> Someone could implement this if they wanted to. In the future it might
> be useful to create a detailed model for the OTP and secure boot
> behavior, and that can affect the strapping.
> 
> However it is not critical for running guests under qemu. I think we
> should defer it until there is some guest code that needs the detailed
> behavior.

ok. It think we could trap the invalid writes with a simple mask
array at the beginning of the write op .

Thanks,

C.