Re: [PATCH v4 4/6] hw/arm/virt: Introduce virt_get_high_memmap_enabled() helper

[Eric Auger]

On 10/5/22 00:47, Gavin Shan wrote:
> Hi Connie,
>
> On 10/4/22 6:41 PM, Cornelia Huck wrote:
>> On Tue, Oct 04 2022, Gavin Shan <gshan@redhat.com> wrote:
>>
>>> This introduces virt_get_high_memmap_enabled() helper, which returns
>>> the pointer to vms->highmem_{redists, ecam, mmio}. The pointer will
>>> be used in the subsequent patches.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>>> ---
>>>   hw/arm/virt.c | 30 +++++++++++++++++-------------
>>>   1 file changed, 17 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index b0b679d1f4..59de7b78b5 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -1689,14 +1689,29 @@ static uint64_t
>>> virt_cpu_mp_affinity(VirtMachineState *vms, int idx)
>>>       return arm_cpu_mp_affinity(idx, clustersz);
>>>   }
>>>   +static inline bool *virt_get_high_memmap_enabled(VirtMachineState
>>> *vms,
>>> +                                                 int index)
>>> +{
>>> +    bool *enabled_array[] = {
>>> +        &vms->highmem_redists,
>>> +        &vms->highmem_ecam,
>>> +        &vms->highmem_mmio,
>>> +    };
>>> +
>>> +    assert(index - VIRT_LOWMEMMAP_LAST < ARRAY_SIZE(enabled_array));
>>
>> I wonder whether we want an assert(ARRAY_SIZE(extended_memmap) ==
>> ARRAY_SIZE(enabled_array))? IIUC, we never want those two to get out of
>> sync?
>>
>
> Yeah, It makes sense to ensure both arrays synchronized. I will add
> the extra check in next respin.

With Connie's suggestion this looks good to me.

Thanks

Eric
>
>>> +
>>> +    return enabled_array[index - VIRT_LOWMEMMAP_LAST];
>>> +}
>>> +
>
> Thanks,
> Gavin
>