Re: [Qemu-devel] [PATCH v2 12/22] hw/intc/arm_gicv3: Implement GICv3 redistributor registers

[Peter Maydell]

On 14 June 2016 at 13:25, Peter Maydell <address@hidden> wrote:
> On 14 June 2016 at 04:09, Shannon Zhao <address@hidden> wrote:
>>> +    case GICR_NSACR:
>>> +        if (!attrs.secure && !(cs->gic->gicd_ctlr & GICD_CTLR_DS)) {
>>> +            *data = 0;
>>> +            return MEMTX_OK;
>>> +        }
>>> +        *data = cs->gicr_nsacr;
>> Look like this is not consistent with the SPEC.
>> The SPEC says
>> "When GICD_CTLR.DS == 1, this register is RAZ/WI.
>> When GICD_CTLR.DS == 0, this register is Secure, and is RAZ/WI to
>> Non-secure accesses."
>>
>> So when GICD_CTLR.DS == 1, it should make *data = 0.
>
> Agreed. This should have the same kind of condition as GICR_IGRPMODR0:
>
>         if ((cs->gic->gicd_ctlr & GICD_CTLR_DS) || !attrs.secure) {
>             /* RAZ/WI if security disabled, or if
>              * security enabled and this is an NS access
>              */
>             *data = 0;
>             return MEMTX_OK;
>
> (similarly for writes)

This also applies to GICD_NSACR.

thanks
-- PMM