Re: [PATCH 2/3] xen/arm: Defer GICv2 CPU interface mapping until the first access

[Julien Grall <julien@xxxxxxx>]

On 27/01/2023 11:30, Henry Wang wrote:
> Hi Julien,
>
> > -----Original Message-----
> > From: Julien Grall <julien@xxxxxxx>
> > Subject: Re: [PATCH 2/3] xen/arm: Defer GICv2 CPU interface mapping until
> > the first access
> >
> > Hi,
> >
> > > > > @@ -153,6 +153,8 @@ struct vgic_dist {
> > > > >        /* Base address for guest GIC */
> > > > >        paddr_t dbase; /* Distributor base address */
> > > > >        paddr_t cbase; /* CPU interface base address */
> > > > > +    paddr_t csize; /* CPU interface size */
> > > > > +    paddr_t vbase; /* virtual CPU interface base address */
> > > > Could you swap them so that base address variables are grouped?
> > > Sure, my original thought was grouping the CPU interface related fields but
> > > since you prefer grouping the base address, I will follow your suggestion.
> > I would actually prefer your approach because it is easier to associate
> > the size with the base.
> >
> > An alternative would be to use a structure to combine the base/size. So
> > it is even clearer the association.
> >
> > I don't have a strong opinion on either of the two approach I suggested.
> Maybe we can do something like this:
> ```
> paddr_t dbase; /* Distributor base address */
> paddr_t vbase; /* virtual CPU interface base address */
> paddr_t cbase; /* CPU interface base address */
> paddr_t csize; /* CPU interface size */
> ```
>
> So we can ensure both "base address variables are grouped" and
> "CPU interface variables are grouped".
>
> If you don't like this, I would prefer the way I am currently doing, as
> personally I think an extra structure would slightly be an overkill :)
This is really a matter of taste here. My preference is your initial 
approach because I find strange to have virtual CPU interface 
information the physical one.
Cheers,

--
Julien Grall