[PATCH v2] hw/arm/virt: Remove the lower bound of HighMem IO Regions

Akihiko Odaki posted 1 patch 3 weeks, 6 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250901-virt-v2-1-ac2379402c80@rsg.ci.i.u-tokyo.ac.jp
Maintainers: Peter Maydell <peter.maydell@linaro.org>
include/hw/arm/virt.h |  1 +
hw/arm/virt.c         | 16 ++++++++++++----
2 files changed, 13 insertions(+), 4 deletions(-)
[PATCH v2] hw/arm/virt: Remove the lower bound of HighMem IO Regions
Posted by Akihiko Odaki 3 weeks, 6 days ago
Remove the lower bound of the Highmem IO Regions' addresses for the
latest machine version to increase the chance to fit the regions in the
PA space.

The lower bound was especially problematic when using virt-install on
Apple M2. virt-install 5.0.0 adds multiple pcie-root-port devices that
require sufficient space in the ECAM region. However, the Highmem ECAM
region did not fit in the limited PA space on the hardware, and the ECAM
region size was limited to 16 MiB. If virt-install had added more than
16 devices to the root bridge, the region overflowed, which prevented
edk2-stable202505 from scanning PCI devices, including the boot disk,
causing boot failures.

Ideally, a virtual machine with more than 16 devices added to the root
bridge should just work so that users and management layers do not have
to care whether they use constrained hardware.

The base address of the Highmem IO Regions was fixed when commit
f90747c4e8fb ("hw/arm/virt: GICv3 DT node with one or two redistributor
regions") added the first Highmem IO Region. Later, commit 957e32cffa57
("hw/arm/virt: Dynamic memory map depending on RAM requirements")
allowed moving the Highmem IO Regions to higher addresses to accommodate
RAM more than 255 GiB, but the lower bound remained to keep the legacy
memory map.

Remove the lower bound for the latest machine version to accommodate more
devices with the root bridge. Keeping the lower bound for the old
machine versions ensures the compatibility is still maintained.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Changes in v2:
- Rebased.
- Link to v1: https://lore.kernel.org/qemu-devel/20250728-virt-v1-1-0ab9682262c8@rsg.ci.i.u-tokyo.ac.jp
---
 include/hw/arm/virt.h |  1 +
 hw/arm/virt.c         | 16 ++++++++++++----
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 365a28b082cae36321ed906d9a13242997fa543a..341eb171d084b911e05b9861b9f4ba516fa2888e 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -120,6 +120,7 @@ typedef enum VirtGICType {
 
 struct VirtMachineClass {
     MachineClass parent;
+    hwaddr min_highmem_base;
     bool no_tcg_its;
     bool no_highmem_compact;
     bool no_ged;   /* Machines < 4.2 have no support for ACPI GED device */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 1e63f40fbece3997dedc8aa953957471f930d44c..3cb978dae2ec0cdd9e7b902b3b427dac8a27bebf 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1887,6 +1887,7 @@ static void virt_set_high_memmap(VirtMachineState *vms,
 static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
 {
     MachineState *ms = MACHINE(vms);
+    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
     hwaddr base, device_memory_base, device_memory_size, memtop;
     int i;
 
@@ -1913,8 +1914,7 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
     /*
      * We compute the base of the high IO region depending on the
      * amount of initial and device memory. The device memory start/size
-     * is aligned on 1GiB. We never put the high IO region below 256GiB
-     * so that if maxram_size is < 255GiB we keep the legacy memory map.
+     * is aligned on 1GiB.
      * The device region size assumes 1GiB page max alignment per slot.
      */
     device_memory_base =
@@ -1932,8 +1932,8 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
         error_report("maxmem/slots too huge");
         exit(EXIT_FAILURE);
     }
-    if (base < vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES) {
-        base = vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES;
+    if (base < vmc->min_highmem_base) {
+        base = vmc->min_highmem_base;
     }
 
     /* We know for sure that at least the memory fits in the PA space */
@@ -3464,8 +3464,16 @@ DEFINE_VIRT_MACHINE_AS_LATEST(10, 2)
 
 static void virt_machine_10_1_options(MachineClass *mc)
 {
+    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
     virt_machine_10_2_options(mc);
     compat_props_add(mc->compat_props, hw_compat_10_1, hw_compat_10_1_len);
+
+    /*
+     * Do not put the high IO region below 256GiB so that if maxram_size is
+     * < 255GiB we keep the legacy memory map.
+     */
+    vmc->min_highmem_base = base_memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES;
 }
 DEFINE_VIRT_MACHINE(10, 1)
 

---
base-commit: e101d33792530093fa0b0a6e5f43e4d8cfe4581e
change-id: 20250728-virt-833dafa6c11b

Best regards,
-- 
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH v2] hw/arm/virt: Remove the lower bound of HighMem IO Regions
Posted by Peter Maydell 1 week, 5 days ago
On Mon, 1 Sept 2025 at 13:00, Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> Remove the lower bound of the Highmem IO Regions' addresses for the
> latest machine version to increase the chance to fit the regions in the
> PA space.
>
> The lower bound was especially problematic when using virt-install on
> Apple M2. virt-install 5.0.0 adds multiple pcie-root-port devices that
> require sufficient space in the ECAM region. However, the Highmem ECAM
> region did not fit in the limited PA space on the hardware, and the ECAM
> region size was limited to 16 MiB. If virt-install had added more than
> 16 devices to the root bridge, the region overflowed, which prevented
> edk2-stable202505 from scanning PCI devices, including the boot disk,
> causing boot failures.
>
> Ideally, a virtual machine with more than 16 devices added to the root
> bridge should just work so that users and management layers do not have
> to care whether they use constrained hardware.
>
> The base address of the Highmem IO Regions was fixed when commit
> f90747c4e8fb ("hw/arm/virt: GICv3 DT node with one or two redistributor
> regions") added the first Highmem IO Region. Later, commit 957e32cffa57
> ("hw/arm/virt: Dynamic memory map depending on RAM requirements")
> allowed moving the Highmem IO Regions to higher addresses to accommodate
> RAM more than 255 GiB, but the lower bound remained to keep the legacy
> memory map.
>
> Remove the lower bound for the latest machine version to accommodate more
> devices with the root bridge. Keeping the lower bound for the old
> machine versions ensures the compatibility is still maintained.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---

Hi; this fails "make check":

(1) fails the bios-tables-check (likely the golden-reference
files need updating if the memory layout has changed)

acpi-test: Warning! MCFG binary file mismatch. Actual
[aml:/tmp/aml-PD53C3], Expected
[aml:tests/data/acpi/aarch64/virt/MCFG].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
acpi-test: Warning! MCFG mismatch. Actual [asl:/tmp/asl-6N53C3.dsl,
aml:/tmp/aml-PD53C3], Expected [asl:/tmp/asl-FF33C3.dsl,
aml:tests/data/acpi/aarch64/virt/MCFG].
acpi-test: Warning! DSDT binary file mismatch. Actual
[aml:/tmp/aml-YK53C3], Expected
[aml:tests/data/acpi/aarch64/virt/DSDT].
See source file tests/qtest/bios-tables-test.c for instructions on how
to update expected files.
acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-JEN4C3.dsl,
aml:/tmp/aml-YK53C3], Expected [asl:/tmp/asl-3ZF4C3.dsl,
aml:tests/data/acpi/aarch64/virt/DSDT].
**
ERROR:../../tests/qtest/bios-tables-test.c:554:test_acpi_asl:
assertion failed: (all_tables_match)


(2) qos-test now fails with a NULL pointer related problem
when running with a build with the sanitizers enabled:


# Start of e1000e-tests tests
# starting QEMU: exec ./qemu-system-aarch64 -qtest
unix:/tmp/qtest-1298136.sock -qtest-log /dev/null -chardev
socket,path=/tmp/qtest-1298136.qmp,id=char0 -mon
chardev=char0,mode=control -display none -audio none -M virt, -cpu max
-device e1000e,netdev=hs0 -netdev socket,fd=6,id=hs0  -accel qtest
----------------------------------- stderr -----------------------------------
../../tests/qtest/libqos/pci.c:394:15: runtime error: member access
within null pointer of type 'QPCIBus' (aka 'struct QPCIBus')
    #0 0x60d9ef616f23 in qpci_config_writel
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:394:15
    #1 0x60d9ef615cc6 in qpci_iomap
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/libqos/pci.c:534:5
    #2 0x60d9ef632504 in e1000e_pci_create
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/libqos/e1000e.c:189:19
    #3 0x60d9ef61334c in qos_driver_new
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/libqos/qgraph.c:756:11
    #4 0x60d9ef6145a1 in allocate_objects
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/libqos/qos_external.c:136:19
    #5 0x60d9ef6c0c3a in qos_allocate_objects
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/qos-test.c:131:12
    #6 0x60d9ef6c0c3a in run_one_test
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/qos-test.c:179:11
    #7 0x7753c4e80387
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x88387) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #8 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #9 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #10 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #11 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #12 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #13 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #14 0x7753c4e802a2
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x882a2) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #15 0x7753c4e80899 in g_test_run_suite
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x88899) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #16 0x7753c4e8092f in g_test_run
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x8892f) (BuildId:
1eb6131419edb83b2178b682829a6913cf682d75)
    #17 0x60d9ef6be7ae in main
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/../../tests/qtest/qos-test.c:347:5
    #18 0x7753c4a2a1c9 in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #19 0x7753c4a2a28a in __libc_start_main csu/../csu/libc-start.c:360:3
    #20 0x60d9ef5cb224 in _start
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/arm-clang/tests/qtest/qos-test+0x16c224)
(BuildId: 538375f704342381ea206855e1476b3ff0beef38)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior
../../tests/qtest/libqos/pci.c:394:15

thanks
-- PMM
Re: [PATCH v2] hw/arm/virt: Remove the lower bound of HighMem IO Regions
Posted by Eric Auger 3 weeks, 3 days ago
Hi,
On 9/1/25 2:00 PM, Akihiko Odaki wrote:
> Remove the lower bound of the Highmem IO Regions' addresses for the
> latest machine version to increase the chance to fit the regions in the
> PA space.
>
> The lower bound was especially problematic when using virt-install on
> Apple M2. virt-install 5.0.0 adds multiple pcie-root-port devices that
> require sufficient space in the ECAM region. However, the Highmem ECAM
> region did not fit in the limited PA space on the hardware, and the ECAM
> region size was limited to 16 MiB. If virt-install had added more than
> 16 devices to the root bridge, the region overflowed, which prevented
> edk2-stable202505 from scanning PCI devices, including the boot disk,
> causing boot failures.
>
> Ideally, a virtual machine with more than 16 devices added to the root
> bridge should just work so that users and management layers do not have
> to care whether they use constrained hardware.
>
> The base address of the Highmem IO Regions was fixed when commit
> f90747c4e8fb ("hw/arm/virt: GICv3 DT node with one or two redistributor
> regions") added the first Highmem IO Region. Later, commit 957e32cffa57
> ("hw/arm/virt: Dynamic memory map depending on RAM requirements")
> allowed moving the Highmem IO Regions to higher addresses to accommodate
> RAM more than 255 GiB, but the lower bound remained to keep the legacy
> memory map.
>
> Remove the lower bound for the latest machine version to accommodate more
> devices with the root bridge. Keeping the lower bound for the old
> machine versions ensures the compatibility is still maintained.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
This looks good to me.
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

> ---
> Changes in v2:
> - Rebased.
> - Link to v1: https://lore.kernel.org/qemu-devel/20250728-virt-v1-1-0ab9682262c8@rsg.ci.i.u-tokyo.ac.jp
> ---
>  include/hw/arm/virt.h |  1 +
>  hw/arm/virt.c         | 16 ++++++++++++----
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 365a28b082cae36321ed906d9a13242997fa543a..341eb171d084b911e05b9861b9f4ba516fa2888e 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -120,6 +120,7 @@ typedef enum VirtGICType {
>  
>  struct VirtMachineClass {
>      MachineClass parent;
> +    hwaddr min_highmem_base;
>      bool no_tcg_its;
>      bool no_highmem_compact;
>      bool no_ged;   /* Machines < 4.2 have no support for ACPI GED device */
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 1e63f40fbece3997dedc8aa953957471f930d44c..3cb978dae2ec0cdd9e7b902b3b427dac8a27bebf 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1887,6 +1887,7 @@ static void virt_set_high_memmap(VirtMachineState *vms,
>  static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
>  {
>      MachineState *ms = MACHINE(vms);
> +    VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
>      hwaddr base, device_memory_base, device_memory_size, memtop;
>      int i;
>  
> @@ -1913,8 +1914,7 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
>      /*
>       * We compute the base of the high IO region depending on the
>       * amount of initial and device memory. The device memory start/size
> -     * is aligned on 1GiB. We never put the high IO region below 256GiB
> -     * so that if maxram_size is < 255GiB we keep the legacy memory map.
> +     * is aligned on 1GiB.
>       * The device region size assumes 1GiB page max alignment per slot.
>       */
>      device_memory_base =
> @@ -1932,8 +1932,8 @@ static void virt_set_memmap(VirtMachineState *vms, int pa_bits)
>          error_report("maxmem/slots too huge");
>          exit(EXIT_FAILURE);
>      }
> -    if (base < vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES) {
> -        base = vms->memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES;
> +    if (base < vmc->min_highmem_base) {
> +        base = vmc->min_highmem_base;
>      }
>  
>      /* We know for sure that at least the memory fits in the PA space */
> @@ -3464,8 +3464,16 @@ DEFINE_VIRT_MACHINE_AS_LATEST(10, 2)
>  
>  static void virt_machine_10_1_options(MachineClass *mc)
>  {
> +    VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
>      virt_machine_10_2_options(mc);
>      compat_props_add(mc->compat_props, hw_compat_10_1, hw_compat_10_1_len);
> +
> +    /*
> +     * Do not put the high IO region below 256GiB so that if maxram_size is
> +     * < 255GiB we keep the legacy memory map.
> +     */
> +    vmc->min_highmem_base = base_memmap[VIRT_MEM].base + LEGACY_RAMLIMIT_BYTES;
>  }
>  DEFINE_VIRT_MACHINE(10, 1)
>  
>
> ---
> base-commit: e101d33792530093fa0b0a6e5f43e4d8cfe4581e
> change-id: 20250728-virt-833dafa6c11b
>
> Best regards,