Re: [PATCH v8 4/7] xen/arm: Add virtual GICv3 support

[Julien Grall <julien.grall@linaro.org>]

On 07/28/2014 05:02 PM, Ian Campbell wrote:
> On Mon, 2014-07-28 at 16:35 +0100, Julien Grall wrote:
>> On 07/28/2014 03:55 PM, Ian Campbell wrote:
>>> On Mon, 2014-07-28 at 15:35 +0100, Ian Campbell wrote:
>>>> FYI I'm seeing a trap in gicv3_init when running on a model. I haven't
>>>> tracked it down to an actual location yet though. I'll let you know what
>>>> I find.
>>>
>>> The access to ICC_BPR1_EL1 in gicv3_cpu_init is faulting for some
>>> reason. do_hyp_trap is then access current-> but it is too soon to do
>>> that, hence a data abort occurs which is what I was seeing.
>>>
>>> This is on a fast model with the boot-wrapper, perhaps the "firmware"
>>> failed to setup something?
>>
>> IIRC, the "firmware" has to configure some register in secure. I suspect
>> it's ICC_SRE_EL1.
> 
> _EL3, but otherwise I think you are right.

It might be worse to add a check in Xen that ICC_SRE_EL3 as the SRE bit
enabled.

>> I guess your bootwrapper already contains this stuff.
> 
> It turns out it does not because I'm using an old version of the
> upstream bootwrapper modified for Xen.
> 
> Unfortunately upstream has moved on in ways which make rebasing the Xen
> support a case of a complete rewrite so I'd been putting it off. Looks
> like it just got bumped up the TODO a bit...
> 
>>> BTW, ICC_BPR1_EL1 is the secure alias of the NS version of the register.
>>> Why not just use the NS version directly?
>>
>>
>> AFAIU the spec, the name is not different between secure and non-secure.
>> The processor will know it following the state.
> 
> The spec says:
>         This register is an alias of the Non-secure view of
>         ICC_BPR0_EL1, and a Secure access to this register is identical
>         to a Non-secure access to ICC_BPR0_EL1.
> 
> So for Xen (necessarily running NS) ICC_BPR0_EL1 and ICC_BPR1_EL1 are
> the same, but accessing BPR0 would seem more natural.

You are right. I read quickly the specification.

Regards,

-- 
Julien Grall