* [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions
@ 2026-07-06 13:08 Dominik Csapak
2026-07-07 9:11 ` Fiona Ebner
0 siblings, 1 reply; 5+ messages in thread
From: Dominik Csapak @ 2026-07-06 13:08 UTC (permalink / raw)
To: pve-devel
When passing through devices with a large memory region, for example
video memory, there needs to be enough address space for OVMF to
correctly map that region.
By default, the address space is 32G, which should work for cards up to
16G of video memory. To get a bigger address space in OVMF, one needs to
either:
* set the CPU type to host (the address space from the host will be used)
* set 'phys-bits' on the cpu (this sets the address space to that value)
with possibly the cpu flag 'pdpe1gb', since without that, OVMF limits
the mmio address space to 128G.
Try to detect the largest memory region from sysfs, and warn when the VM
config includes a situation where the conditions are not fulfilled.
This won't detect all circumstances fully, but should detect a large
chunk of them.
Includes some tests for 0G, 16G, 32G and 512G regions.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v2:
* use PVE::File instead of PVE::Tools
* improve variable naming
* improve log message
* adhere to 100column line length
* use snprintf instead of floating point operation for determining the
highest set bit
* improve comment
src/PVE/QemuServer.pm | 29 +++-
src/PVE/QemuServer/PCI.pm | 136 +++++++++++++++++
src/test/Makefile | 5 +-
src/test/run_pci_memory_detection_tests.pl | 162 +++++++++++++++++++++
4 files changed, 329 insertions(+), 3 deletions(-)
create mode 100755 src/test/run_pci_memory_detection_tests.pl
diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 1fe47c16..55499019 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -5688,14 +5688,24 @@ sub vm_start_nolock {
PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout);
my $uuid;
+ my $need_min_phys_bits = 0;
for my $id (sort keys %$pci_devices) {
my $d = $pci_devices->{$id};
my ($index) = ($id =~ m/^hostpci(\d+)$/);
my $chosen_mdev;
for my $dev ($d->{ids}->@*) {
- my $info =
- eval { PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d) };
+ my $phys_bits_for_dev =
+ eval { PVE::QemuServer::PCI::min_phys_bits_needed($conf, $dev->{id}) };
+ warn "could not determine needed MMIO size for '$dev->{id}': $@\n" if $@;
+
+ if (defined($phys_bits_for_dev) && $phys_bits_for_dev > $need_min_phys_bits) {
+ $need_min_phys_bits = $phys_bits_for_dev;
+ }
+
+ my $info = eval {
+ PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d);
+ };
if ($d->{mdev} || $d->{nvidia}) {
warn $@ if $@;
$chosen_mdev = $info;
@@ -5719,6 +5729,21 @@ sub vm_start_nolock {
}
}
push @$cmd, '-uuid', $uuid if defined($uuid);
+
+ if ($need_min_phys_bits > 0) {
+ my $size = render_bytes(2**$need_min_phys_bits);
+ my $warn_text =
+ "A PCI device with a large memory region (e.g. VRAM) was detected, but VM"
+ . " is not configured for a big enough address space for OVMF (needs $size).
+ . " Consider enabling CPU type 'host' or setting 'phys-bits'"
+ . " (at least $need_min_phys_bits).";
+
+ if ($need_min_phys_bits > 40) {
+ $warn_text .= " You also need to set the 'pdpe1gb' flag.";
+ }
+
+ log_warn($warn_text);
+ }
};
if (my $err = $@) {
eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 0b67943c..7c6a0c6b 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -5,11 +5,13 @@ use strict;
use IO::File;
+use PVE::File;
use PVE::JSONSchema;
use PVE::Mapping::PCI;
use PVE::SysFSTools;
use PVE::Tools;
+use PVE::QemuServer::CPUConfig;
use PVE::QemuServer::Helpers;
use PVE::QemuServer::Machine;
use PVE::QemuServer::PCI::Mdev;
@@ -871,4 +873,138 @@ sub reserve_pci_usage {
die $@ if $@;
}
+=pod
+
+=head3 get_biggest_memory_region
+
+ my $size = get_biggest_memory_region($pci_id);
+
+Returns the size of biggest memory region for a PCI device in bytes
+This can be used to check if the config is correct for having an MMIO size that is large enough.
+
+Parameters:
+
+=over
+
+=item C<$pci_id>: The PCI Id to get the biggest memory region from.
+
+=back
+
+=cut
+
+sub get_biggest_memory_region {
+ my ($pci_id) = @_;
+
+ $pci_id = PVE::SysFSTools::normalize_pci_id($pci_id);
+
+ # read resource regions from sysfs
+ my $resource_file = "/sys/bus/pci/devices/$pci_id/resource";
+ my $regions = PVE::File::file_get_contents($resource_file);
+
+ # for each line parse start/end/flags.
+ my $biggest_size = 0;
+ for my $line (split('\n', $regions)) {
+ if ($line =~ m/^0x([a-f0-9]{16})\s0x([a-f0-9]{16})\s0x([a-f0-9]{16})$/i) {
+ # avoid warning when parsing long hex values with hex()
+ no warnings 'portable'; # Support for 64-bit ints required
+
+ my $start = hex($1);
+ my $end = hex($2);
+ my $flags = hex($3);
+
+ # find largest memory region with 'IORESOURCE_MEM' flag (see include/linux/ioport.h in kernel source)
+ if (($flags & 0x200) != 0) {
+ # $end is inclusive, so + 1 for the overall size
+ my $size = $end - $start + 1;
+ if ($size > $biggest_size) {
+ $biggest_size = $size;
+ }
+ }
+ }
+ }
+
+ return $biggest_size;
+}
+
+# return the max bit set by the given number
+my sub get_max_set_bit {
+ my ($num) = @_;
+ return length(sprintf("%b", $num));
+}
+
+=pod
+
+=head3 min_phys_bits_needed
+
+ my $bits = min_phys_bits_needed($conf, $pci_id);
+
+Returns the minimum phys-bits value that needs to be configured so that the MMIO size is enough.
+For PCI devices with memory regions > 16G, the vm either has to:
+* boot with seabios
+* use 'host' type cpu
+* use high enough 'phys-bits' value (or 'host') and (possibly) 'pdpe1gb'
+
+return 0 if vm config does have enough MMIO space configured already
+
+Parameters:
+
+=over
+
+=item C<$conf>: The config of the given VM.
+
+=item C<$pci_id>: The PCI ID to check.
+
+=back
+
+=cut
+
+sub min_phys_bits_needed {
+ my ($conf, $pci_id) = @_;
+
+ return 0 if ($conf->{bios} // 'seabios') eq 'seabios';
+
+ my $size = get_biggest_memory_region($pci_id);
+
+ return 0 if $size <= 16 * 1024 * 1024 * 1024;
+
+ # calculate the needed phys bits. The MMIO size needs to be larger than $size,
+ # so the bits needed must be bigger than log2($size) + 3. So simply get the max set bit (via snprintf).
+ # edk2 limits the mmio space to an eigth of the overall space. (PhysMemAddressWidth - 3)
+ #
+ # see edk2 source code: OvmfPkg/Library/PlatformInitLib/MemDetect.c
+
+ my $needed_phys_bits = get_max_set_bit($size) + 3;
+
+ return $needed_phys_bits if !defined($conf->{cpu});
+
+ my $cpu = PVE::JSONSchema::parse_property_string('pve-vm-cpu-conf', $conf->{cpu})
+ or die "Cannot parse cpu description: $conf->{cpu}\n";
+
+ my $cpu_type = $cpu->{cputype} // '';
+
+ return 0 if $cpu_type eq 'host';
+
+ if (my $phys_bits = $cpu->{'phys-bits'}) {
+ return 0 if $phys_bits eq 'host';
+ # if it's not 'host' it must be a number between 8 and 64
+
+ # Same as for 'check_phys_bits_above_40_compat', we'd need CPU model expansion, but this is
+ # not cheap to get. Check the pdpe1gb flag only for qemu64/kvm64 cpu types, since most
+ # other types have it already.
+ if (
+ ($cpu_type eq 'qemu64' || $cpu_type eq 'kvm64')
+ && ($cpu->{flags} // '') !~ m/\+pdpe1gb/
+ ) {
+ # edk2 limits the phys bits to 40 in case of no 1gb pages
+ #
+ # see edk2 source code: OvmfPkg/Library/PlatformInitLib/MemDetect.c
+ $phys_bits = 40 if $phys_bits > 40;
+ }
+
+ return 0 if $phys_bits >= $needed_phys_bits;
+ }
+
+ return $needed_phys_bits;
+}
+
1;
diff --git a/src/test/Makefile b/src/test/Makefile
index cf589f41..25e0f2f6 100644
--- a/src/test/Makefile
+++ b/src/test/Makefile
@@ -1,6 +1,6 @@
all: test
-test: test_snapshot test_cfg_to_cmd test_cfg_to_cmd_aarch64 test_pci_addr_conflicts test_pci_reservation test_qemu_img_convert test_migration test_restore_config test_parse_config
+test: test_snapshot test_cfg_to_cmd test_cfg_to_cmd_aarch64 test_pci_addr_conflicts test_pci_reservation test_qemu_img_convert test_migration test_restore_config test_parse_config test_pci_memory_detection
test_snapshot: run_snapshot_tests.pl
./run_snapshot_tests.pl
@@ -18,6 +18,9 @@ test_qemu_img_convert: run_qemu_img_convert_tests.pl
test_pci_addr_conflicts: run_pci_addr_checks.pl
./run_pci_addr_checks.pl
+test_pci_memory_detection: run_pci_memory_detection_tests.pl
+ ./run_pci_memory_detection_tests.pl
+
test_pci_reservation: run_pci_reservation_tests.pl
./run_pci_reservation_tests.pl
diff --git a/src/test/run_pci_memory_detection_tests.pl b/src/test/run_pci_memory_detection_tests.pl
new file mode 100755
index 00000000..f9c246da
--- /dev/null
+++ b/src/test/run_pci_memory_detection_tests.pl
@@ -0,0 +1,162 @@
+#!/usr/bin/perl
+
+use v5.36;
+
+use lib qw(..);
+
+use JSON;
+use Test::More;
+use Test::MockModule;
+
+use PVE::JSONSchema;
+use PVE::QemuServer::CPUConfig;
+use PVE::QemuServer::PCI;
+
+my $tools_module;
+$tools_module = Test::MockModule->new('PVE::File');
+$tools_module->mock(
+ 'file_get_contents' => sub($path) {
+ if ($path =~ m/01:00.0/) {
+ # 0 B region
+ return <<EOF;
+0x0000000000000000 0x0000000000000000 0x0000000000000000
+EOF
+ } elsif ($path =~ m/01:01.0/) {
+ # 16 G region
+ return <<EOF;
+0x0000017000000000 0x00000173ffffffff 0x000000000014220c
+EOF
+ } elsif ($path =~ m/02:00.0/) {
+ # 32 G region
+ return <<EOF;
+0x0000017000000000 0x00000177ffffffff 0x000000000014220c
+EOF
+ } elsif ($path =~ m/03:00.0/) {
+ # 512 G region
+ return <<EOF;
+0x0000000000000000 0x0000007FFFFFFFFF 0x0000000000000200
+EOF
+ }
+ },
+);
+
+my $region_pci_map = {
+ "0G" => { address => "01:00.0", size => 0 },
+ "16G" => { address => "01:01.0", size => 16 * 1024 * 1024 * 1024 },
+ "32G" => { address => "02:00.0", size => 32 * 1024 * 1024 * 1024 },
+ "512G" => { address => "03:00.0", size => 512 * 1024 * 1024 * 1024 },
+};
+
+# test parser
+for my $test (sort keys $region_pci_map->%*) {
+ my $pci_id = $region_pci_map->{$test}->{address};
+ my $size = PVE::QemuServer::PCI::get_biggest_memory_region($pci_id);
+ my $expected = $region_pci_map->{$test}->{size};
+
+ is($size, $expected, "Size parsing - $test");
+}
+
+my $tests = [
+ {
+ name => "Empty Config",
+ conf => {},
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 0,
+ "512G" => 0,
+ },
+ },
+ {
+ name => "OVMF - no CPU configured",
+ conf => {
+ bios => 'ovmf',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 39,
+ "512G" => 43,
+ },
+ },
+ {
+ name => "OVMF - HOST CPU configured",
+ conf => {
+ bios => 'ovmf',
+ cpu => 'host',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 0,
+ "512G" => 0,
+ },
+ },
+ {
+ name => "OVMF - 38 phys-bits CPU configured",
+ conf => {
+ bios => 'ovmf',
+ cpu => 'qemu64,phys-bits=38',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 39,
+ "512G" => 43,
+ },
+ },
+ {
+ name => "OVMF - 40 phys-bits CPU configured",
+ conf => {
+ bios => 'ovmf',
+ cpu => 'qemu64,phys-bits=40',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 0,
+ "512G" => 43,
+ },
+ },
+ {
+ name => "OVMF - 43 phys-bits CPU configured",
+ conf => {
+ bios => 'ovmf',
+ cpu => 'qemu64,phys-bits=43',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 0,
+ "512G" => 43,
+ },
+ },
+ {
+ name => "OVMF - 43 phys-bits + pdpe1gb CPU configured",
+ conf => {
+ bios => 'ovmf',
+ cpu => 'qemu64,phys-bits=43,flags=+pdpe1gb',
+ },
+ expected => {
+ "0G" => 0,
+ "16G" => 0,
+ "32G" => 0,
+ "512G" => 0,
+ },
+ },
+];
+
+foreach my $test (@{$tests}) {
+ my $name = $test->{name};
+ my $expected = $test->{expected};
+ my $conf = $test->{conf};
+ for my $size (sort keys $region_pci_map->%*) {
+ my $pciid = $region_pci_map->{$size}->{address};
+ my $actual = PVE::QemuServer::PCI::min_phys_bits_needed($conf, $pciid);
+
+ is($actual, $expected->{$size}, "$name - $size");
+ }
+
+}
+
+done_testing();
--
2.47.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions
2026-07-06 13:08 [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions Dominik Csapak
@ 2026-07-07 9:11 ` Fiona Ebner
2026-07-07 10:21 ` Dominik Csapak
0 siblings, 1 reply; 5+ messages in thread
From: Fiona Ebner @ 2026-07-07 9:11 UTC (permalink / raw)
To: Dominik Csapak, pve-devel
Am 06.07.26 um 3:10 PM schrieb Dominik Csapak:
> @@ -5719,6 +5729,21 @@ sub vm_start_nolock {
> }
> }
> push @$cmd, '-uuid', $uuid if defined($uuid);
> +
> + if ($need_min_phys_bits > 0) {
> + my $size = render_bytes(2**$need_min_phys_bits);
> + my $warn_text =
> + "A PCI device with a large memory region (e.g. VRAM) was detected, but VM"
> + . " is not configured for a big enough address space for OVMF (needs $size).
Fails to build, there is a quote missing at the end.
> + . " Consider enabling CPU type 'host' or setting 'phys-bits'"
> + . " (at least $need_min_phys_bits).";
> +
> + if ($need_min_phys_bits > 40) {
> + $warn_text .= " You also need to set the 'pdpe1gb' flag.";
> + }
> +
> + log_warn($warn_text);
> + }
> };
> if (my $err = $@) {
> eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
> diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
> index 0b67943c..7c6a0c6b 100644
> --- a/src/PVE/QemuServer/PCI.pm
> +++ b/src/PVE/QemuServer/PCI.pm
> @@ -5,11 +5,13 @@ use strict;
>
> use IO::File;
>
> +use PVE::File;
> use PVE::JSONSchema;
> use PVE::Mapping::PCI;
> use PVE::SysFSTools;
> use PVE::Tools;
>
> +use PVE::QemuServer::CPUConfig;
Sorry, that I only notice now, but I don't think we should pull in the
CPUConfig here and have logic for CPU settings in the latter half of
min_phys_bits_needed(). Let's pass along the CPU phys bits from outside
instead. Maybe have an effective_phys_bits_heuristic() helper in
CPUConfig to calculate it depending on bios/pdpe1gb flag. And we can
skip the call to min_phys_bits_needed() up-front if we know that the
check does not apply for the current CPU model/phys-bits setting.
> use PVE::QemuServer::Helpers;
> use PVE::QemuServer::Machine;
> use PVE::QemuServer::PCI::Mdev;
> @@ -871,4 +873,138 @@ sub reserve_pci_usage {
> die $@ if $@;
> }
>
> +=pod
> +
> +=head3 get_biggest_memory_region
> +
> + my $size = get_biggest_memory_region($pci_id);
> +
> +Returns the size of biggest memory region for a PCI device in bytes
Nit: missing dot after sentence
> +This can be used to check if the config is correct for having an MMIO size that is large enough.
> +
> +Parameters:
> +
> +=over
> +
> +=item C<$pci_id>: The PCI Id to get the biggest memory region from.
> +
> +=back
> +
> +=cut
> +
> +sub get_biggest_memory_region {
> + my ($pci_id) = @_;
> +
> + $pci_id = PVE::SysFSTools::normalize_pci_id($pci_id);
> +
> + # read resource regions from sysfs
> + my $resource_file = "/sys/bus/pci/devices/$pci_id/resource";
> + my $regions = PVE::File::file_get_contents($resource_file);
> +
> + # for each line parse start/end/flags.
> + my $biggest_size = 0;
> + for my $line (split('\n', $regions)) {
> + if ($line =~ m/^0x([a-f0-9]{16})\s0x([a-f0-9]{16})\s0x([a-f0-9]{16})$/i) {
> + # avoid warning when parsing long hex values with hex()
> + no warnings 'portable'; # Support for 64-bit ints required
> +
> + my $start = hex($1);
> + my $end = hex($2);
> + my $flags = hex($3);
> +
> + # find largest memory region with 'IORESOURCE_MEM' flag (see include/linux/ioport.h in kernel source)
Nit: comment too long
> + if (($flags & 0x200) != 0) {
> + # $end is inclusive, so + 1 for the overall size
> + my $size = $end - $start + 1;
> + if ($size > $biggest_size) {
> + $biggest_size = $size;
> + }
> + }
> + }
Should we add an else branch warning about unexpected lines just to be sure?
> + }
> +
> + return $biggest_size;
> +}
> +
> +# return the max bit set by the given number
s/by the given number/for the given number/
I'd mention that it's the position starting from 1. Otherwise, it could
as well be the offset. Or maybe say "number of bits required to
represent the number in binary"?
The potential confusion with offset makes me wonder if we even need this
helper. Or if having it inline and a good code comment might be better?
But no big deal, both variants are fine.
> +my sub get_max_set_bit {
> + my ($num) = @_;
> + return length(sprintf("%b", $num));
> +}
> +
> +=pod
> +
> +=head3 min_phys_bits_needed
> +
> + my $bits = min_phys_bits_needed($conf, $pci_id);
> +
> +Returns the minimum phys-bits value that needs to be configured so that the MMIO size is enough.
> +For PCI devices with memory regions > 16G, the vm either has to:
> +* boot with seabios
> +* use 'host' type cpu
> +* use high enough 'phys-bits' value (or 'host') and (possibly) 'pdpe1gb'
Style nits:
Note that this does not show up as a bullet point list when viewed with
perldoc.
> +
> +return 0 if vm config does have enough MMIO space configured already
s/return/Returns/
Missing dot at the end of the sentence.
> +
> +Parameters:
> +
> +=over
> +
> +=item C<$conf>: The config of the given VM.
> +
> +=item C<$pci_id>: The PCI ID to check.
> +
> +=back
> +
> +=cut
> +
> +sub min_phys_bits_needed {
> + my ($conf, $pci_id) = @_;
> +
> + return 0 if ($conf->{bios} // 'seabios') eq 'seabios';
> +
> + my $size = get_biggest_memory_region($pci_id);
> +
> + return 0 if $size <= 16 * 1024 * 1024 * 1024;
> +
> + # calculate the needed phys bits. The MMIO size needs to be larger than $size,
> + # so the bits needed must be bigger than log2($size) + 3. So simply get the max set bit (via snprintf).
Style nit: comment line too long
> + # edk2 limits the mmio space to an eigth of the overall space. (PhysMemAddressWidth - 3)
s/eigth/eighth/
> diff --git a/src/test/run_pci_memory_detection_tests.pl b/src/test/run_pci_memory_detection_tests.pl
> new file mode 100755
> index 00000000..f9c246da
> --- /dev/null
> +++ b/src/test/run_pci_memory_detection_tests.pl
> @@ -0,0 +1,162 @@
> +foreach my $test (@{$tests}) {
Style nit: use for instead of foreach
> + my $name = $test->{name};
> + my $expected = $test->{expected};
> + my $conf = $test->{conf};
> + for my $size (sort keys $region_pci_map->%*) {
> + my $pciid = $region_pci_map->{$size}->{address};
> + my $actual = PVE::QemuServer::PCI::min_phys_bits_needed($conf, $pciid);
> +
> + is($actual, $expected->{$size}, "$name - $size");
> + }
> +
> +}
> +
> +done_testing();
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions
2026-07-07 9:11 ` Fiona Ebner
@ 2026-07-07 10:21 ` Dominik Csapak
2026-07-07 11:01 ` Fiona Ebner
0 siblings, 1 reply; 5+ messages in thread
From: Dominik Csapak @ 2026-07-07 10:21 UTC (permalink / raw)
To: Fiona Ebner, pve-devel
On 7/7/26 11:11 AM, Fiona Ebner wrote:
> Am 06.07.26 um 3:10 PM schrieb Dominik Csapak:
>> @@ -5719,6 +5729,21 @@ sub vm_start_nolock {
>> }
>> }
>> push @$cmd, '-uuid', $uuid if defined($uuid);
>> +
>> + if ($need_min_phys_bits > 0) {
>> + my $size = render_bytes(2**$need_min_phys_bits);
>> + my $warn_text =
>> + "A PCI device with a large memory region (e.g. VRAM) was detected, but VM"
>> + . " is not configured for a big enough address space for OVMF (needs $size).
>
> Fails to build, there is a quote missing at the end.
interesting, it's here on my branch, not sure how that quote got missing...
>
>> + . " Consider enabling CPU type 'host' or setting 'phys-bits'"
>> + . " (at least $need_min_phys_bits).";
>> +
>> + if ($need_min_phys_bits > 40) {
>> + $warn_text .= " You also need to set the 'pdpe1gb' flag.";
>> + }
>> +
>> + log_warn($warn_text);
>> + }
>> };
>> if (my $err = $@) {
>> eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); };
>> diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
>> index 0b67943c..7c6a0c6b 100644
>> --- a/src/PVE/QemuServer/PCI.pm
>> +++ b/src/PVE/QemuServer/PCI.pm
>> @@ -5,11 +5,13 @@ use strict;
>>
>> use IO::File;
>>
>> +use PVE::File;
>> use PVE::JSONSchema;
>> use PVE::Mapping::PCI;
>> use PVE::SysFSTools;
>> use PVE::Tools;
>>
>> +use PVE::QemuServer::CPUConfig;
>
> Sorry, that I only notice now, but I don't think we should pull in the
> CPUConfig here and have logic for CPU settings in the latter half of
> min_phys_bits_needed(). Let's pass along the CPU phys bits from outside
> instead. Maybe have an effective_phys_bits_heuristic() helper in
> CPUConfig to calculate it depending on bios/pdpe1gb flag. And we can
> skip the call to min_phys_bits_needed() up-front if we know that the
> check does not apply for the current CPU model/phys-bits setting.
sure, then it's probably also easier to expand custom cpu models, so we
can include that in the heuristic
>
>> use PVE::QemuServer::Helpers;
>> use PVE::QemuServer::Machine;
>> use PVE::QemuServer::PCI::Mdev;
>> @@ -871,4 +873,138 @@ sub reserve_pci_usage {
>> die $@ if $@;
>> }
>>
>> +=pod
>> +
>> +=head3 get_biggest_memory_region
>> +
>> + my $size = get_biggest_memory_region($pci_id);
>> +
>> +Returns the size of biggest memory region for a PCI device in bytes
>
> Nit: missing dot after sentence
>
>> +This can be used to check if the config is correct for having an MMIO size that is large enough.
>> +
>> +Parameters:
>> +
>> +=over
>> +
>> +=item C<$pci_id>: The PCI Id to get the biggest memory region from.
>> +
>> +=back
>> +
>> +=cut
>> +
>> +sub get_biggest_memory_region {
>> + my ($pci_id) = @_;
>> +
>> + $pci_id = PVE::SysFSTools::normalize_pci_id($pci_id);
>> +
>> + # read resource regions from sysfs
>> + my $resource_file = "/sys/bus/pci/devices/$pci_id/resource";
>> + my $regions = PVE::File::file_get_contents($resource_file);
>> +
>> + # for each line parse start/end/flags.
>> + my $biggest_size = 0;
>> + for my $line (split('\n', $regions)) {
>> + if ($line =~ m/^0x([a-f0-9]{16})\s0x([a-f0-9]{16})\s0x([a-f0-9]{16})$/i) {
>> + # avoid warning when parsing long hex values with hex()
>> + no warnings 'portable'; # Support for 64-bit ints required
>> +
>> + my $start = hex($1);
>> + my $end = hex($2);
>> + my $flags = hex($3);
>> +
>> + # find largest memory region with 'IORESOURCE_MEM' flag (see include/linux/ioport.h in kernel source)
>
> Nit: comment too long
>
>> + if (($flags & 0x200) != 0) {
>> + # $end is inclusive, so + 1 for the overall size
>> + my $size = $end - $start + 1;
>> + if ($size > $biggest_size) {
>> + $biggest_size = $size;
>> + }
>> + }
>> + }
>
> Should we add an else branch warning about unexpected lines just to be sure?
i'm not convinced we'd gain anything by that. the format of these
kernel provided sysfs files are rather static, not sure how likely
it is that this changes
>
>> + }
>> +
>> + return $biggest_size;
>> +}
>> +
>> +# return the max bit set by the given number
>
> s/by the given number/for the given number/
>
> I'd mention that it's the position starting from 1. Otherwise, it could
> as well be the offset. Or maybe say "number of bits required to
> represent the number in binary"?
>
> The potential confusion with offset makes me wonder if we even need this
> helper. Or if having it inline and a good code comment might be better?
> But no big deal, both variants are fine.
yeah i'll inline it, makes sense...
>
>> +my sub get_max_set_bit {
>> + my ($num) = @_;
>> + return length(sprintf("%b", $num));
>> +}
>> +
>> +=pod
>> +
>> +=head3 min_phys_bits_needed
>> +
>> + my $bits = min_phys_bits_needed($conf, $pci_id);
>> +
>> +Returns the minimum phys-bits value that needs to be configured so that the MMIO size is enough.
>> +For PCI devices with memory regions > 16G, the vm either has to:
>> +* boot with seabios
>> +* use 'host' type cpu
>> +* use high enough 'phys-bits' value (or 'host') and (possibly) 'pdpe1gb'
>
> Style nits:
>
> Note that this does not show up as a bullet point list when viewed with
> perldoc.
>
>> +
>> +return 0 if vm config does have enough MMIO space configured already
>
> s/return/Returns/
>
> Missing dot at the end of the sentence.
>
>> +
>> +Parameters:
>> +
>> +=over
>> +
>> +=item C<$conf>: The config of the given VM.
>> +
>> +=item C<$pci_id>: The PCI ID to check.
>> +
>> +=back
>> +
>> +=cut
>> +
>> +sub min_phys_bits_needed {
>> + my ($conf, $pci_id) = @_;
>> +
>> + return 0 if ($conf->{bios} // 'seabios') eq 'seabios';
>> +
>> + my $size = get_biggest_memory_region($pci_id);
>> +
>> + return 0 if $size <= 16 * 1024 * 1024 * 1024;
>> +
>> + # calculate the needed phys bits. The MMIO size needs to be larger than $size,
>> + # so the bits needed must be bigger than log2($size) + 3. So simply get the max set bit (via snprintf).
>
> Style nit: comment line too long
>
>> + # edk2 limits the mmio space to an eigth of the overall space. (PhysMemAddressWidth - 3)
>
> s/eigth/eighth/
>
>> diff --git a/src/test/run_pci_memory_detection_tests.pl b/src/test/run_pci_memory_detection_tests.pl
>> new file mode 100755
>> index 00000000..f9c246da
>> --- /dev/null
>> +++ b/src/test/run_pci_memory_detection_tests.pl
>> @@ -0,0 +1,162 @@
>> +foreach my $test (@{$tests}) {
>
> Style nit: use for instead of foreach
>
>> + my $name = $test->{name};
>> + my $expected = $test->{expected};
>> + my $conf = $test->{conf};
>> + for my $size (sort keys $region_pci_map->%*) {
>> + my $pciid = $region_pci_map->{$size}->{address};
>> + my $actual = PVE::QemuServer::PCI::min_phys_bits_needed($conf, $pciid);
>> +
>> + is($actual, $expected->{$size}, "$name - $size");
>> + }
>> +
>> +}
>> +
>> +done_testing();
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions
2026-07-07 10:21 ` Dominik Csapak
@ 2026-07-07 11:01 ` Fiona Ebner
2026-07-07 11:06 ` Dominik Csapak
0 siblings, 1 reply; 5+ messages in thread
From: Fiona Ebner @ 2026-07-07 11:01 UTC (permalink / raw)
To: Dominik Csapak, pve-devel
Am 07.07.26 um 12:21 PM schrieb Dominik Csapak:
> On 7/7/26 11:11 AM, Fiona Ebner wrote:
>> Am 06.07.26 um 3:10 PM schrieb Dominik Csapak:
>>> + if (($flags & 0x200) != 0) {
>>> + # $end is inclusive, so + 1 for the overall size
>>> + my $size = $end - $start + 1;
>>> + if ($size > $biggest_size) {
>>> + $biggest_size = $size;
>>> + }
>>> + }
>>> + }
>>
>> Should we add an else branch warning about unexpected lines just to be
>> sure?
>
> i'm not convinced we'd gain anything by that. the format of these
> kernel provided sysfs files are rather static, not sure how likely
> it is that this changes
The issue is that there is no additional fallout/failure later, but the
check would just quietly stop working if it does change for whatever
reason (maybe if they add a new entry at the end?).
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions
2026-07-07 11:01 ` Fiona Ebner
@ 2026-07-07 11:06 ` Dominik Csapak
0 siblings, 0 replies; 5+ messages in thread
From: Dominik Csapak @ 2026-07-07 11:06 UTC (permalink / raw)
To: Fiona Ebner, pve-devel
On 7/7/26 1:01 PM, Fiona Ebner wrote:
> Am 07.07.26 um 12:21 PM schrieb Dominik Csapak:
>> On 7/7/26 11:11 AM, Fiona Ebner wrote:
>>> Am 06.07.26 um 3:10 PM schrieb Dominik Csapak:
>>>> + if (($flags & 0x200) != 0) {
>>>> + # $end is inclusive, so + 1 for the overall size
>>>> + my $size = $end - $start + 1;
>>>> + if ($size > $biggest_size) {
>>>> + $biggest_size = $size;
>>>> + }
>>>> + }
>>>> + }
>>>
>>> Should we add an else branch warning about unexpected lines just to be
>>> sure?
>>
>> i'm not convinced we'd gain anything by that. the format of these
>> kernel provided sysfs files are rather static, not sure how likely
>> it is that this changes
>
> The issue is that there is no additional fallout/failure later, but the
> check would just quietly stop working if it does change for whatever
> reason (maybe if they add a new entry at the end?).
fair, i'll add a short warning
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-07 11:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-06 13:08 [PATCH qemu-server v3] fix #7711: pci: try to detect large memory region preconditions Dominik Csapak
2026-07-07 9:11 ` Fiona Ebner
2026-07-07 10:21 ` Dominik Csapak
2026-07-07 11:01 ` Fiona Ebner
2026-07-07 11:06 ` Dominik Csapak
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.