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

2022-08-16 Thread Zhenyu Zhang
commit 49e00c1fe2ab24b73ac16908f3c05ebe88b9186d (HEAD -> master)
Author: Gavin Shan 
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 

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:zheny...@redhat.com


On Mon, Aug 15, 2022 at 2:30 PM Gavin Shan  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 
> ---
>  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 = >highmem_redists;
> +break;
> +case VIRT_HIGH_PCIE_ECAM:
> +region_enabled = >highmem_ecam;
> +break;
> +case VIRT_HIGH_PCIE_MMIO:
> +region_enabled = >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 = >highmem_redists;
> -break;
> -case VIRT_HIGH_PCIE_ECAM:
> -region_enabled = >highmem_ecam;
> -break;
> -case VIRT_HIGH_PCIE_MMIO:
> -region_enabled = >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 

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

2022-08-15 Thread Gavin Shan
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 
---
 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 = >highmem_redists;
+break;
+case VIRT_HIGH_PCIE_ECAM:
+region_enabled = >highmem_ecam;
+break;
+case VIRT_HIGH_PCIE_MMIO:
+region_enabled = >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 = >highmem_redists;
-break;
-case VIRT_HIGH_PCIE_ECAM:
-region_enabled = >highmem_ecam;
-break;
-case VIRT_HIGH_PCIE_MMIO:
-region_enabled = >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