Re: [PATCH v2 15/15] xen/arm: gic-v3: Add support of vGICv2 when available

[Julien Grall <julien.grall@citrix.com>]

Hi Ian,

On 01/07/15 09:12, Ian Campbell wrote:
> On Tue, 2015-06-30 at 18:29 +0100, Julien Grall wrote:
>>>
>>>> +     */
>>>> +    if ( v->domain->arch.vgic.version == GIC_V2 )
>>>> +        val &= ~GICC_SRE_EL2_ENEL1;
>>>> +    else
>>>> +        val |= GICC_SRE_EL2_ENEL1;
>>>> +    WRITE_SYSREG32(val, ICC_SRE_EL2);
>>>> +    isb();
>>>
>>> Is the isb strictly needed? I suppose we are already using rather too
>>> many, perhaps a more complete audit is in order.
>>
>> AFAICT no, the ENEL1 doesn't gate any access to EL1 systems register in EL2.
>>
>> There is an isb in the caller (gic_restore_state) but I find it
>> confusing because we rely on the caller doing the right thing for us.
>> I'm thinking to push the isb within the callee for more clarify.
> 
> isb's aren't free though.
> 
>> It can be part of a bigger audit.
> 
> Indeed, this isn't one for now/.
> 
> Really there should probably be a single isb at the end of ctxt
> save/restore and very little otherwise except where there is an absolute
> need for some sort of ordering between steps.

I will drop this one in the next version.

Regards,


-- 
Julien Grall