Re: [PATCH v3 3/5] hw/arm/virt: Introduce variable region_base in virt_set_high_memmap()

[Eric Auger]

Hi Gavin,

On 9/22/22 01:13, Gavin Shan wrote:
> This introduces variable 'region_base' for the base address of the
> specific high memory region. It's the preparatory work to optimize
> high memory region address assignment.
Why is it a preparatory work (same comment for previous patch, ie [2/5]
). Are those changes really needed? why?

Eric
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  hw/arm/virt.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 187b3ee0e2..b0b679d1f4 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1692,15 +1692,15 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState 
> *vms, int idx)
>  static void virt_set_high_memmap(VirtMachineState *vms,
>                                   hwaddr base, int pa_bits)
>  {
> -    hwaddr region_size;
> +    hwaddr region_base, region_size;
>      bool fits;
>      int i;
>  
>      for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
> +        region_base = ROUND_UP(base, extended_memmap[i].size);
>          region_size = extended_memmap[i].size;
>  
> -        base = ROUND_UP(base, region_size);
> -        vms->memmap[i].base = base;
> +        vms->memmap[i].base = region_base;
>          vms->memmap[i].size = region_size;
>  
>          /*
> @@ -1709,9 +1709,9 @@ static void virt_set_high_memmap(VirtMachineState *vms,
>           *
>           * For each device that doesn't fit, disable it.
>           */
> -        fits = (base + region_size) <= BIT_ULL(pa_bits);
> +        fits = (region_base + region_size) <= BIT_ULL(pa_bits);
>          if (fits) {
> -            vms->highest_gpa = base + region_size - 1;
> +            vms->highest_gpa = region_base + region_size - 1;
>          }
>  
>          switch (i) {
> @@ -1726,7 +1726,7 @@ static void virt_set_high_memmap(VirtMachineState *vms,
>              break;
>          }
>  
> -        base += region_size;
> +        base = region_base + region_size;
>      }
>  }
>