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

[Henry Wang <Henry.Wang@xxxxxxx>]

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

Indeed,

> My preference is your initial
> approach because I find strange to have virtual CPU interface
> information the physical one.

then I will keep it as it is if there is no strong objection from Michal.

Kind regards,
Henry

> 
> Cheers,
> 
> --
> Julien Grall