From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 8F6461FF0B2 for ; Mon, 24 Aug 2026 15:56:07 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 63DC6215FF; Mon, 24 Aug 2026 15:56:05 +0200 (CEST) Message-ID: <547e153f-7799-4d1c-856e-460b08477e8a@proxmox.com> Date: Mon, 24 Aug 2026 15:55:59 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH qemu-server v4] fix #7711: pci: try to detect large memory region preconditions To: Dominik Csapak , pve-devel@lists.proxmox.com References: <20260709115349.3822301-1-d.csapak@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20260709115349.3822301-1-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1787579729610 X-SPAM-LEVEL: Spam detection results: 0 AWL -0.212 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_MAILER 2 Automated Mailer Tag Left in Email PROLO_LEO1 0.1 Meta Catches all Leo drug variations so far RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: PNVYY2CO5FZFFUI3YUGO5TOGWTLZ2VU3 X-Message-ID-Hash: PNVYY2CO5FZFFUI3YUGO5TOGWTLZ2VU3 X-MailFrom: f.ebner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: Am 09.07.26 um 1:54 PM schrieb Dominik Csapak: > 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. > > Mediated devices are skipped, since they don't have a memory region to > check against, and using the parent device is always wrong (e.g. a 64G > GPU split into 1G parts don't need the MMIO space for 64G VRAM). The > only exception here is the special nvidia vGPU interface (for newer > cards) since that passes through the whole VF (which has regions to > check). > > 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 > --- > changes from v3: > * split out the phys bits heuristic into CPUConfig.pm > * improve wording/comments/pod > * add functionality for custom cpu models > * change the check point so mdevs (except nvidia special ones) are > * more tests > > src/PVE/QemuServer.pm | 43 +++- > src/PVE/QemuServer/CPUConfig.pm | 79 ++++++ > src/PVE/QemuServer/PCI.pm | 120 +++++++++ > src/test/Makefile | 5 +- > src/test/run_pci_memory_detection_tests.pl | 268 +++++++++++++++++++++ > 5 files changed, 511 insertions(+), 4 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 59a37098..a80d56a1 100644 > --- a/src/PVE/QemuServer.pm > +++ b/src/PVE/QemuServer.pm > @@ -5688,21 +5688,43 @@ sub vm_start_nolock { > PVE::QemuServer::PCI::reserve_pci_usage($pci_reserve_list, $vmid, $start_timeout); > > my $uuid; > + my $need_min_phys_bits = 0; > + my $arch = PVE::QemuServer::Helpers::get_vm_arch($conf); > + my $available_phys_bits = > + PVE::QemuServer::CPUConfig::available_phys_bits_heuristic($conf, $arch); > 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 $info = eval { > + PVE::QemuServer::PCI::prepare_pci_device($vmid, $dev->{id}, $index, $d); > + }; > if ($d->{mdev} || $d->{nvidia}) { > warn $@ if $@; > $chosen_mdev = $info; > - last if $chosen_mdev; # if successful, we're done > } else { > die $@ if $@; > } > + > + # don't check the phys bits requirements for mediated devices (except new nvidia interface) > + if (!$d->{mdev} || $d->{nvidia}) { > + my $phys_bits_for_dev = eval { > + PVE::QemuServer::PCI::min_phys_bits_needed( > + $conf, $dev->{id}, $available_phys_bits, > + ); > + }; > + 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; > + } > + } > + last if $chosen_mdev; # if successful, we're done > } > > next if !$d->{mdev} && !$d->{nvidia}; > @@ -5719,6 +5741,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'" Nit: I'd suggest: Consider using CPU type 'host' or setting the 'phys-bits' CPU value (at least ...) > + . " (at least $need_min_phys_bits)."; > + > + if ($need_min_phys_bits > 40) { Should also check for $conf->{bios} && $conf->{bios} eq 'ovmf', or? > + $warn_text .= " You also need to set the 'pdpe1gb' flag."; Nit: I'd suggest: You also need to enable the 'pdpe1gb' CPU flag. > + } > + > + log_warn($warn_text); > + } > }; > if (my $err = $@) { > eval { PVE::Storage::deactivate_volumes($storecfg, $vollist); }; > diff --git a/src/PVE/QemuServer/CPUConfig.pm b/src/PVE/QemuServer/CPUConfig.pm > index df3e2b92..adab14bc 100644 > --- a/src/PVE/QemuServer/CPUConfig.pm > +++ b/src/PVE/QemuServer/CPUConfig.pm > @@ -860,6 +860,85 @@ my sub check_phys_bits_above_40_compat($bios, $cpu_type, $cpu_flags) { > } > } > > +=pod > + > +=head3 available_phys_bits_heuristic > + > + my $bits = available_phys_bits_heuristic($conf); Missing $arch in example > + > +Heuristically determine the number of physical address bits that will (most > +likely) be available to the guest, based on the VM configuration C<$conf>. > +Avoids full CPU model expansion, but uses info from custom cpu models. > + > +If the available phys bits cannot be easily determined, C is returned. > +Dies if the C property of the configuration cannot be parsed. > + > +Parameters: > + > +=over 4 > + > +=item C<$conf>: The VM configuration hash. > + > +=item C<$arch>: The VM architecture. > + > +=back > + > +=cut > + > +sub available_phys_bits_heuristic($conf, $arch) { > + return 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 $cputype = $cpu->{cputype} // ''; > + my $host_phys_bits = PVE::QemuServer::Helpers::get_host_phys_address_bits(); > + > + my $custom_cpu = {}; > + > + my $builtin_models = $builtin_models_by_arch->{$arch}; > + if (my $model = $builtin_models->{$cputype}) { > + $cputype = $model->{'reported-model'}; > + } elsif (is_custom_model($cputype)) { > + $custom_cpu = get_custom_model($cputype); > + $cputype = $custom_cpu->{'reported-model'} // $cpu_fmt->{'reported-model'}->{default}; > + } Maybe we should add a helper for this, since it's the third usage (i.e. in print_cpu_device() and in get_cpu_bitness() - the one in get_cpu_options() is interleaved so cannot easily be replaced)? It's also missing the replacement type mapping, which the helper could have. > + > + return $host_phys_bits if $cputype eq 'host'; > + > + my $phys_bits = $cpu->{'phys-bits'} // $custom_cpu->{'phys-bits'}; > + > + if (defined($phys_bits)) { > + return $host_phys_bits if $phys_bits eq 'host'; > + # if it's not 'host' it must be a number between 8 and 64 Why mention the limits here? If they change, it's one more place to adapt and I don't think it's relevant for the code below. > + > + my $custom_flags = $custom_cpu->{flags} // ''; > + my $cpu_flags = $cpu->{flags} // ''; > + > + # 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. > + # > + # triggers if neither the custom nor local conf has the flag, or if the > + # custom one has the flag added but it's removed locally > + if ( > + ($cputype eq 'qemu64' || $cputype eq 'kvm64') Please add: && ($conf->{bios} && $conf->{bios} eq 'ovmf') > + && $cpu_flags !~ m/\+pdpe1gb/ > + && $custom_flags !~ m/\+pdpe1gb/ > + || $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 $phys_bits; > + } > + > + return; > +} > + > # Calculate QEMU's '-cpu' argument from a given VM configuration > sub get_cpu_options( > $conf, $arch, $kvm, $kvm_off, $machine_version, $winversion, $gpu_passthrough, > diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm > index 0b67943c..3f7ebd48 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; This can be dropped now :) > use PVE::QemuServer::Helpers; > use PVE::QemuServer::Machine; > use PVE::QemuServer::PCI::Mdev; > @@ -871,4 +873,122 @@ 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)) { > + $line =~ s/^\s+|\s+$//g; > + 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; > + } > + } > + } elsif ($line ne "") { > + warn "Unrecognized line format in '$resource_file'\n"; > + } > + } > + > + return $biggest_size; > +} > + > +=pod > + > +=head3 min_phys_bits_needed > + > + my $bits = min_phys_bits_needed($conf, $pci_id); > + > +Returns the minimum C 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: > + > +=over > + > +=item * boot with SeaBIOS > + > +=item * use C type CPU > + > +=item * use a high enough C value (or C) and (possibly) C > + > +=back > + > +Returns C<0> if the VM config already has enough MMIO space configured. > + > +Parameters: > + > +=over > + > +=item C<$conf>: The VM configuration hash. > + > +=item C<$pci_id>: The PCI ID to check. > + > +=back > + > +=cut > + > +sub min_phys_bits_needed { > + my ($conf, $pci_id, $available_phys_bits) = @_; Nit: passing in $available_phys_bits here feels a bit unintuitive to me. I'd slightly prefer having the comparision be done at the call site once in the end, when we gathered the max for all devices. > + > + 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 number of bits required to represent the number in binary (via > + # snprintf). edk2 limits the mmio space to an eighth of the overall space. > + # (PhysMemAddressWidth - 3) > + # > + # see edk2 source code: OvmfPkg/Library/PlatformInitLib/MemDetect.c > + > + my $needed_phys_bits = length(sprintf("%b", $size)) + 3; > + > + return $needed_phys_bits if !defined($available_phys_bits); > + > + return 0 if $available_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..568a8ca3 > --- /dev/null > +++ b/src/test/run_pci_memory_detection_tests.pl > @@ -0,0 +1,268 @@ > +#!/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 < +0x0000000000000000 0x0000000000000000 0x0000000000000000 > +EOF > + } elsif ($path =~ m/01:01.0/) { > + # 16 G region Nit: I'd say GiB so that it's immediately clear that it's a power of 2, same for the others below. > + return < +0x0000017000000000 0x00000173ffffffff 0x000000000014220c > +EOF > + } elsif ($path =~ m/02:00.0/) { > + # 32 G region > + return < +0x0000017000000000 0x00000177ffffffff 0x000000000014220c > +EOF > + } elsif ($path =~ m/03:00.0/) { > + # 512 G region > + return < +0x0000000000000000 0x0000007FFFFFFFFF 0x0000000000000200 > +EOF > + } > + }, > +); > + > +my $helpers_module; > +$helpers_module = Test::MockModule->new("PVE::QemuServer::Helpers"); > +$helpers_module->mock( > + 'get_host_phys_address_bits' => sub () { > + return 48; > + }, > +); > + > +my $cpuconfig_module; > +$cpuconfig_module = Test::MockModule->new("PVE::QemuServer::CPUConfig"); > +$cpuconfig_module->mock( > + 'load_custom_cpu_model_config' => sub () { > + return { > + ids => { > + with43bits => { > + type => 'cpu-model', > + 'cpu-type' => 'custom-with43bits', > + 'reported-model' => 'kvm64', > + 'phys-bits' => 43, > + }, > + with43bitsandflags => { > + type => 'cpu-model', > + 'cpu-type' => 'custom-with43bitsandflags', > + 'reported-model' => 'kvm64', > + 'phys-bits' => 43, > + 'flags' => '+pdpe1gb', > + }, > + with20bits => { > + type => 'cpu-model', > + 'cpu-type' => 'custom-with43bitsandflags', > + 'reported-model' => 'kvm64', > + 'phys-bits' => 20, > + }, > + }, > + }; > + }, > +); > + > +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", Nit: maybe note that this means SeaBIOS? > + 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, > + }, > + }, > + { > + name => "OVMF - custom-cpu model with 43 bits", > + conf => { > + bios => 'ovmf', > + cpu => 'custom-with43bits', > + }, > + expected => { > + "0G" => 0, > + "16G" => 0, > + "32G" => 0, > + "512G" => 43, > + }, > + }, > + { > + name => "OVMF - custom-cpu model with 43 bits and pdpe1gb", > + conf => { > + bios => 'ovmf', > + cpu => 'custom-with43bitsandflags', > + }, > + expected => { > + "0G" => 0, > + "16G" => 0, > + "32G" => 0, > + "512G" => 0, > + }, > + }, > + { > + name => "OVMF - custom-cpu model with 20bits", > + conf => { > + bios => 'ovmf', > + cpu => 'custom-with20bits,phys-bits=43,flags=+pdpe1gb', > + }, > + expected => { > + "0G" => 0, > + "16G" => 0, > + "32G" => 0, > + "512G" => 0, > + }, > + }, > + { > + name => "OVMF - custom-cpu model with 20bits and local overrides", > + conf => { > + bios => 'ovmf', > + cpu => 'custom-with20bits', > + }, > + expected => { > + "0G" => 0, > + "16G" => 0, > + "32G" => 39, > + "512G" => 43, > + }, > + }, > + { > + name => "OVMF - custom-cpu model with 43bits and pdpe1gb and local removal of pdpe1gb", > + conf => { > + bios => 'ovmf', > + cpu => 'custom-with43bitsandflags,flags=-pdpe1gb', > + }, > + expected => { > + "0G" => 0, > + "16G" => 0, > + "32G" => 0, > + "512G" => 43, > + }, > + }, > +]; > + > +foreach my $test (@{$tests}) { > + my $name = $test->{name}; > + my $expected = $test->{expected}; > + my $conf = $test->{conf}; > + my $available_phys_bits = > + PVE::QemuServer::CPUConfig::available_phys_bits_heuristic($conf, 'x86_64'); > + 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, $available_phys_bits); > + > + is($actual, $expected->{$size}, "$name - $size"); > + } > + > +} > + > +done_testing();