Re: [pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
On 6/24/20 9:46 AM, Thomas Lamprecht wrote:
Am 6/22/20 um 10:17 AM schrieb Stefan Reiter:
@@ -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
ignore me, confused with non legacy IGD, sorry.
So the only thing left then is the description formatting. Should I send
a v2 for that or do you want to do a followup?
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
Am 6/22/20 um 10:17 AM schrieb Stefan Reiter:
>>> @@ -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
ignore me, confused with non legacy IGD, sorry.
___
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
Re: [pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
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
---
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;
Re: [pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
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
> ---
> 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 -.-
> 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.
> + 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?
> + '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
[pve-devel] [PATCH qemu-server 3/3] fix #2794: allow legacy IGD passthrough
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
---
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);
+
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'.",
+ 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) },
+ '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
