[pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
Stefan Reiter
s.reiter at proxmox.com
Mon Jun 22 10:17:04 CEST 2020
On 6/18/20 5:00 PM, Thomas Lamprecht wrote:
> Am 6/18/20 um 4:36 PM schrieb Stefan Reiter:
>> Legacy IGD passthrough requires address 00:1f.0 to not be assigned to
>> anything on QEMU startup (currently it's assigned to bridge pci.2).
>> Changing this in general would break live-migration, so introduce a new
>> hostpci parameter "legacy-igd", which if set to 1 will move that bridge
>> to be nested under bridge 1.
>>
>> This is safe because:
>> * Bridge 1 is unconditionally created on i440fx, so nesting is ok
>> * Defaults are not changed, i.e. PCI layout only changes when the new
>> parameter is specified manually
>> * hostpci forbids migration anyway
>>
>> Additionally, the PT device has to be assigned address 00:02.0 in the
>> guest as well, which is usually used for VGA assignment. Luckily, IGD PT
>> requires vga=none, so that is not an issue either.
>>
>> See https://git.qemu.org/?p=qemu.git;a=blob;f=docs/igd-assign.txt
>>
>> Signed-off-by: Stefan Reiter <s.reiter at proxmox.com>
>> ---
>> PVE/QemuServer.pm | 10 ++++++++--
>> PVE/QemuServer/PCI.pm | 45 +++++++++++++++++++++++++++++++++++++------
>> 2 files changed, 47 insertions(+), 8 deletions(-)
>>
>> diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
>> index 1c08222..1abe64b 100644
>> --- a/PVE/QemuServer.pm
>> +++ b/PVE/QemuServer.pm
>> @@ -3102,7 +3102,7 @@ sub config_to_command {
>> }
>>
>> # host pci device passthrough
>> - my ($kvm_off, $gpu_passthrough) = PVE::QemuServer::PCI::print_hostpci_devices(
>> + my ($kvm_off, $gpu_passthrough, $legacy_igd) = PVE::QemuServer::PCI::print_hostpci_devices(
>> $conf, $devices, $winversion, $q35, $bridges, $arch, $machine_type);
>>
>> # usb devices
>> @@ -3458,7 +3458,13 @@ sub config_to_command {
>>
>> for my $k (sort {$b cmp $a} keys %$bridges) {
>> next if $q35 && $k < 4; # q35.cfg already includes bridges up to 3
>> - $pciaddr = print_pci_addr("pci.$k", undef, $arch, $machine_type);
>> +
>> + my $k_name = $k;
>> + if ($k == 2 && $legacy_igd) {
>> + $k_name = "$k-igd";
>> + }
>> + $pciaddr = print_pci_addr("pci.$k_name", undef, $arch, $machine_type);
>> +
>
> ugh, hacks all the way down, but well it seems that pass through stuff relies on that -.-
>
The entire legacy-igd thing is pretty much a hack (on QEMUs side too I
mean), and I don't really see a way around this part if we do want to
support it...
The $k_name thing could be done away with if we hardcode the address
here, but I'd rather have it in the pci_addr_map, so we know to not use
it in the future.
>> my $devstr = "pci-bridge,id=pci.$k,chassis_nr=$k$pciaddr";
>> if ($q35) {
>> # add after -readconfig pve-q35.cfg
>> diff --git a/PVE/QemuServer/PCI.pm b/PVE/QemuServer/PCI.pm
>> index 39f9970..e835f39 100644
>> --- a/PVE/QemuServer/PCI.pm
>> +++ b/PVE/QemuServer/PCI.pm
>> @@ -55,6 +55,14 @@ EODESCR
>> optional => 1,
>> default => 0,
>> },
>> + 'legacy-igd' => {
>> + type => 'boolean',
>> + description => "Pass this device in legacy IGD mode (allows required"
>> + . " 1f.0 PCI bridge and assigns correct address)."
>> + . " Requires i440fx machine type and VGA set to 'none'.",
>
> I'd rather use 100 cc lines for that and do not align subsequent concatenated lines indentation
> but just indent them 4 spaces more than the prev level.
>
Ok
>> + optional => 1,
>> + default => 0,
>> + },
>> 'mdev' => {
>> type => 'string',
>> format_description => 'string',
>> @@ -89,7 +97,8 @@ sub get_pci_addr_map {
>> $pci_addr_map = {
>> piix3 => { bus => 0, addr => 1, conflict_ok => qw(ehci) },
>> ehci => { bus => 0, addr => 1, conflict_ok => qw(piix3) }, # instead of piix3 on arm
>> - vga => { bus => 0, addr => 2 },
>> + vga => { bus => 0, addr => 2, conflict_ok => qw(legacy-igd) },
>
> sorry, but how do they conflict? the address differs so the check can't hit.
> Or is this just for documentation purpose?
>
What do you mean the address differs? 'legacy-igd' and 'vga' both share
bus 0 addr 2, so without the conflict_ok the 'run_pci_addr_checks.pl'
test fails. This is only save because legacy-igd requires vga=none
anyway, as documented by the comment below.
>> + 'legacy-igd' => { bus => 0, addr => 2, conflict_ok => qw(vga) }, # legacy-igd requires vga=none
>> balloon0 => { bus => 0, addr => 3 },
>> watchdog => { bus => 0, addr => 4 },
>> scsihw0 => { bus => 0, addr => 5, conflict_ok => qw(pci.3) },
>> @@ -149,6 +158,7 @@ sub get_pci_addr_map {
>> 'xhci' => { bus => 1, addr => 27 },
>> 'pci.4' => { bus => 1, addr => 28 },
>> 'rng0' => { bus => 1, addr => 29 },
>> + 'pci.2-igd' => { bus => 1, addr => 30 }, # replaces pci.2 in case a legacy IGD device is passed through
>> 'virtio6' => { bus => 2, addr => 1 },
>> 'virtio7' => { bus => 2, addr => 2 },
>> 'virtio8' => { bus => 2, addr => 3 },
>> @@ -351,6 +361,7 @@ sub print_hostpci_devices {
>>
>> my $kvm_off = 0;
>> my $gpu_passthrough = 0;
>> + my $legacy_igd = 0;
>>
>> for (my $i = 0; $i < $MAX_HOSTPCI_DEVICES; $i++) {
>> my $id = "hostpci$i";
>> @@ -372,7 +383,32 @@ sub print_hostpci_devices {
>> $pciaddr = print_pcie_addr($id);
>> }
>> } else {
>> - $pciaddr = print_pci_addr($id, $bridges, $arch, $machine_type);
>> + my $pci_name = $d->{'legacy-igd'} ? 'legacy-igd' : $id;
>> + $pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
>> + }
>> +
>> + my $pcidevices = $d->{pciid};
>> + my $multifunction = 1 if @$pcidevices > 1;
>> +
>> + if ($d->{'legacy-igd'}) {
>> + die "only one device can be assigned in legacy-igd mode\n"
>> + if $legacy_igd;
>> + $legacy_igd = 1;
>> +
>> + die "legacy IGD assignment requires VGA mode to be 'none'\n"
>> + if !defined($conf->{'vga'}) || $conf->{'vga'} ne 'none';
>> + die "legacy IGD assignment requires rombar to be enabled\n"
>> + if defined($d->{rombar}) && !$d->{rombar};
>> + die "legacy IGD assignment is not compatible with x-vga\n"
>> + if $d->{'x-vga'};
>> + die "legacy IGD assignment is not compatible with mdev\n"
>> + if $d->{mdev};
>> + die "legacy IGD assignment is not compatible with q35\n"
>> + if $q35;
>> + die "legacy IGD assignment is not compatible with multifunction devices\n"
>> + if $multifunction;
>> + die "legacy IGD assignment only works for devices on host bus 00:02.0\n"
>> + if $pcidevices->[0]->{id} !~ m/02\.0$/;
>> }
>>
>> my $xvga = '';
>> @@ -383,9 +419,6 @@ sub print_hostpci_devices {
>> $gpu_passthrough = 1;
>> }
>>
>> - my $pcidevices = $d->{pciid};
>> - my $multifunction = 1 if @$pcidevices > 1;
>> -
>> my $sysfspath;
>> if ($d->{mdev} && scalar(@$pcidevices) == 1) {
>> my $pci_id = $pcidevices->[0]->{id};
>> @@ -420,7 +453,7 @@ sub print_hostpci_devices {
>> }
>> }
>>
>> - return ($kvm_off, $gpu_passthrough);
>> + return ($kvm_off, $gpu_passthrough, $legacy_igd);
>> }
>>
>> 1;
>>
>
More information about the pve-devel
mailing list