Re: [PATCH v2 4/4] virt/hw/virt: Add virt_set_high_memmap() helper

[Zhenyu Zhang]

commit 49e00c1fe2ab24b73ac16908f3c05ebe88b9186d (HEAD -> master)
Author: Gavin Shan <gshan@redhat.com>
Date:   Mon Aug 15 14:29:58 2022 +0800

    virt/hw/virt: Add virt_set_high_memmap() helper

    The logic to assign high memory region's address in virt_set_memmap()
    is independent. Lets move the logic to virt_set_high_memmap() helper.
    "each device" is replaced by "each region" in the comments.

    No functional change intended.

    Signed-off-by: Gavin Shan <gshan@redhat.com>

The patchs works well on my Fujitsu host.

[root@hpe-apollo80-02-n00 qemu]# /home/qemu/build/qemu-system-aarch64 -version
QEMU emulator version 7.0.92 (v7.1.0-rc2-12-gd102b8162a)
[root@hpe-apollo80-02-n00 qemu]# /home/qemu/build/qemu-system-aarch64
-accel kvm -m 4096,maxmem=1023G -machine virt-2.12 -cpu host

[root@hpe-apollo80-02-n00 qemu]# /home/qemu/build/qemu-system-aarch64
-accel kvm -m 4096,maxmem=1024G -machine virt-2.12 -cpu host
qemu-system-aarch64: -accel kvm: Addressing limited to 40 bits, but
memory exceeds it by 1073741824 bytes

[root@hpe-apollo80-02-n00 qemu]# /home/qemu/build/qemu-system-aarch64
-accel kvm -m 4096,maxmem=1023G -machine virt -cpu host

[root@hpe-apollo80-02-n00 qemu]# /home/qemu/build/qemu-system-aarch64
-accel kvm -m 4096,maxmem=1024G -machine virt -cpu host
qemu-system-aarch64: -accel kvm: Addressing limited to 40 bits, but
memory exceeds it by 1073741824 bytes

Tested-by:zhenyzha@redhat.com


On Mon, Aug 15, 2022 at 2:30 PM Gavin Shan <gshan@redhat.com> wrote:
>
> The logic to assign high memory region's address in virt_set_memmap()
> is independent. Lets move the logic to virt_set_high_memmap() helper.
> "each device" is replaced by "each region" in the comments.
>
> No functional change intended.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  hw/arm/virt.c | 92 ++++++++++++++++++++++++++++-----------------------
>  1 file changed, 50 insertions(+), 42 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index e38b6919c9..4dde08a924 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1688,6 +1688,55 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState 
> *vms, int idx)
>      return arm_cpu_mp_affinity(idx, clustersz);
>  }
>
> +static void virt_set_high_memmap(VirtMachineState *vms,
> +                                 hwaddr base, int pa_bits)
> +{
> +    hwaddr region_base, region_size;
> +    bool *region_enabled, 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;
> +
> +        switch (i) {
> +        case VIRT_HIGH_GIC_REDIST2:
> +            region_enabled = &vms->highmem_redists;
> +            break;
> +        case VIRT_HIGH_PCIE_ECAM:
> +            region_enabled = &vms->highmem_ecam;
> +            break;
> +        case VIRT_HIGH_PCIE_MMIO:
> +            region_enabled = &vms->highmem_mmio;
> +            break;
> +        default:
> +            region_enabled = NULL;
> +        }
> +
> +        /* Skip unknwon or disabled regions */
> +        if (!region_enabled || !*region_enabled) {
> +            continue;
> +        }
> +
> +        /*
> +         * Check each region to see if they fit in the PA space,
> +         * moving highest_gpa as we go.
> +         *
> +         * For each device that doesn't fit, disable it.
> +         */
> +        fits = (region_base + region_size) <= BIT_ULL(pa_bits);
> +        if (fits) {
> +            vms->memmap[i].base = region_base;
> +            vms->memmap[i].size = region_size;
> +
> +            base = region_base + region_size;
> +            vms->highest_gpa = region_base + region_size - 1;
> +        } else {
> +            *region_enabled = false;
> +        }
> +    }
> +}
> +
>  static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
>  {
>      MachineState *ms = MACHINE(vms);
> @@ -1742,48 +1791,7 @@ static void virt_set_memmap(VirtMachineState *vms, int 
> pa_bits)
>
>      /* We know for sure that at least the memory fits in the PA space */
>      vms->highest_gpa = memtop - 1;
> -
> -    for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
> -        hwaddr region_base = ROUND_UP(base, extended_memmap[i].size);
> -        hwaddr region_size = extended_memmap[i].size;
> -        bool *region_enabled, fits;
> -
> -        switch (i) {
> -        case VIRT_HIGH_GIC_REDIST2:
> -            region_enabled = &vms->highmem_redists;
> -            break;
> -        case VIRT_HIGH_PCIE_ECAM:
> -            region_enabled = &vms->highmem_ecam;
> -            break;
> -        case VIRT_HIGH_PCIE_MMIO:
> -            region_enabled = &vms->highmem_mmio;
> -            break;
> -        default:
> -            region_enabled = NULL;
> -        }
> -
> -        /* Skip unknwon or disabled regions */
> -        if (!region_enabled || !*region_enabled) {
> -            continue;
> -        }
> -
> -        /*
> -         * Check each device to see if they fit in the PA space,
> -         * moving highest_gpa as we go.
> -         *
> -         * For each device that doesn't fit, disable it.
> -         */
> -        fits = (region_base + region_size) <= BIT_ULL(pa_bits);
> -        if (fits) {
> -            vms->memmap[i].base = region_base;
> -            vms->memmap[i].size = region_size;
> -
> -            base = region_base + region_size;
> -            vms->highest_gpa = region_base + region_size - 1;
> -        } else {
> -            *region_enabled = false;
> -        }
> -    }
> +    virt_set_high_memmap(vms, base, pa_bits);
>
>      if (device_memory_size > 0) {
>          ms->device_memory = g_malloc0(sizeof(*ms->device_memory));
> --
> 2.23.0
>


-- 
I respect your work/life balance, no need to reply to this email if it
is outside of your normal working hours.

Zhenyu Zhang

Senior Quality Engineer, Products And Technologies

Red Hat Software (Beijing) Co., R&D Branch