Re: [PATCH v2 03/15] arm/xen: move gic save and restore registers to gic driver

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

On Thu, 2014-04-10 at 10:20 +0530, Vijay Kilari wrote:
> On Wed, Apr 9, 2014 at 10:21 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2014-04-04 at 17:26 +0530, vijay.kilari@gmail.com wrote:
> >
> >> gic saved registers are moved to gic driver.
> >> This required structure is allocated at runtime
> >> and is saved & restored.
> >
> > I don't follow why this change required switch to allocating them
> > dynamically. If it is a v2 vs v3 thing then I think a union in struct
> > arch_vcpu would be fine, unless the v3 stuff is relatively enormous.
> >
> v2:
> struct gic_state_data {
>     uint32_t gic_hcr, gic_vmcr;
>     uint32_t gic_apr;
>     uint32_t gic_lr[64];
> };
> 
> v3:
>  struct gic_state_data {
>      uint32_t gic_hcr, gic_vmcr;
>      uint32_t gic_apr0[4];
>       uint32_t gic_apr1[4];
>       uint64_t gic_lr[16];
>   };
> 
> Except hcr & vmcr registers, v2 & v3 are different.

looks like it is 268 bytes for v2 and 168 for v3 (surprised it gets
smaller! Fewer LRs I suppose)

> Does static allocation helps in caching the domain struct and
> is this the reason for not choosing dynamic allocation?

Right plus it avoids a pointer indirection. Maybe that's not so
important, but it also reduces code complexity a bit and we can defer
that complexity until the size of the vcpu struct gets too big.

> If so, I would suggest to move this structure to gic.h without
> dynamic allocation?

Shouldn't they be in gic_v2_defs.h and gic_v3_defs.h respectively, adn
with a distinct name? Then in struct arch_vcpu:
	union {
		struct gic_v2_state v2;
		struct gic_v3_state v3;
	} gic;