Re: [PATCH v2 14/15] xen/arm: Add vgic support for GIC v3

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

On Thu, 2014-04-10 at 16:13 +0530, Vijay Kilari wrote:
> On Thu, Apr 10, 2014 at 3:53 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-04-04 at 17:26 +0530, vijay.kilari@gmail.com wrote:
> >> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
> >>
> >> Add vgic driver support for GIC v3 version.
> >
> > There is an awful lot of code duplication with the v2 stuff here,
> > especially with the distributor, but even the redistrubotr stuff looks
> > like it could share common implementation with the v2 distro for at
> > least some functionality.
> >
> > I think we should investigate a sinlge driver here, or at least
> > refactoring anything which is common into a library of helper functions
> > which both can use.
> 
> Yes, only distributor code is common which is just 2 functions i.e
> read & write functions.
> But the whole complexity is v2 gic & vgic driver is it runs with byte
> offset that is the reason
> why in gic_v2_defs all the register addresses definitions are divided by 4.
> So this make it quite difficult to make common code.

It sounds trivial to make this work with appropriate accessor macros to
me.

In fact I'm not sure why readl(&GICD[GICD_FOO]) won't work for you just
as it does for v2 (e.g. type of GICD is uint32_t and GICD_FOO is /4).

> With GICv2 HW constraint, I could not take it up. May be we can take
> this as separate task.

What do you mean?

> >> +    case GICR_SETLPIR:
> > I think you should either register two mmio handlers or maybe just
> > handle this all at once in a single callback.
> >
>    There is only one address rdistributore base and size extracted from dt.
> So we register one handle for this entire region and this single callback will
> take access either GICR or SGI's GICR region.

Having a single region in DT doesn't stop you from doing the same
calculation you do now in the callback at init time and registering two
regions.

> > I didn't see any system register handling, I suppose that is all handled
> > by the hardware GICV stuff?
>    For all system register accesses in Dom's, HW treats are virtual access.
> The memory mapped GICD & GICR registers are virtualized by VGIC

I was looking for some register configuration which causes EL1 accesses
to the system registers to go to the "GICV" equivalent, but I didn't see
it. Can you point me to it please?

Ian.