Re: [PATCH v2 0/4] hw/arm/virt: Improve address assignment for high memory regions

[Eric Auger <eric.auger@redhat.com>]

Hi Gavin,

On 8/24/22 05:29, Gavin Shan wrote:
> Hi Marc,
>
> On 8/15/22 4:29 PM, Gavin Shan wrote:
>> There are three high memory regions, which are VIRT_HIGH_REDIST2,
>> VIRT_HIGH_PCIE_ECAM and VIRT_HIGH_PCIE_MMIO. Their base addresses
>> are floating on highest RAM address. However, they can be disabled
>> in several cases.
>>      (1) One specific high memory region is disabled by developer by
>>      toggling vms->highmem_{redists, ecam, mmio}.
>>      (2) VIRT_HIGH_PCIE_ECAM region is disabled on machine, which is
>>      'virt-2.12' or ealier than it.
>>      (3) VIRT_HIGH_PCIE_ECAM region is disabled when firmware is loaded
>>      on 32-bits system.
>>      (4) One specific high memory region is disabled when it breaks the
>>      PA space limit.
>>      The current implementation of virt_set_memmap() isn't comprehensive
>> because the space for one specific high memory region is always
>> reserved from the PA space for case (1), (2) and (3). In the code,
>> 'base' and 'vms->highest_gpa' are always increased for those three
>> cases. It's unnecessary since the assigned space of the disabled
>> high memory region won't be used afterwards.
>>
>> The series intends to improve the address assignment for these
>> high memory regions:
>>
>> PATCH[1] and PATCH[2] are cleanup and preparatory works.
>> PATCH[3] improves address assignment for these high memory regions
>> PATCH[4] moves the address assignment logic into standalone helper
>>
>> History
>> =======
>> v1: https://lists.nongnu.org/archive/html/qemu-arm/2022-08/msg00013.html
>>
>> Changelog
>> =========
>> v2:
>>    * Split the patches for easier review                        (Gavin)
>>    * Improved changelog                                         (Marc)
>>    * Use 'bool fits' in virt_set_high_memmap()                  (Eric)
>>
You did not really convince me about migration compat wrt the high MMIO
region. Aren't the PCI BARs saved/restored meaning the device driver is
expecting to find data at the same GPA. But what if your high MMIO
region was relocated in the dest QEMU with a possibly smaller VM IPA?
Don't you have MMIO regions now allocated outside of the dest MMIO
region? How does the PCI host bridge route accesses to those regions?
What do I miss?

Thanks

Eric
>
> Could you help to review when you have free cycles? It's just a kindly
> ping :)
>
> Thanks,
> Gavin
>
>>
>> Gavin Shan (4):
>>    hw/arm/virt: Rename variable size to region_size in virt_set_memmap()
>>    hw/arm/virt: Introduce variable region_base in virt_set_memmap()
>>    hw/arm/virt: Improve address assignment for high memory regions
>>    virt/hw/virt: Add virt_set_high_memmap() helper
>>
>>   hw/arm/virt.c | 84 ++++++++++++++++++++++++++++++---------------------
>>   1 file changed, 50 insertions(+), 34 deletions(-)
>>
>