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

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Henry,

On 27/01/2023 12:39, 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
>>>>>>> @@ -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.
there are none. It was just a suggestion.

~Michal