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

[Ian Campbell <Ian.Campbell@citrix.com>]

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.

> 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.

Ian.