PCI Express devices which use legacy VGA compatibility should be placed
on the Root Complex.  This simplifies ioport access to VGA registers,
which requires use of a special exception bit to work across PCI(e)
bridges.  It is also necessary for ioport access to VESA BIOS Extension
(VBE) registers, which is not forwarded over PCI(e) bridges, even with
the special exception bit for VGA register access.[1]

Update the PCI Express Guidelines to add these to the list of devices
which can be placed directly on the Root Complex.

Note that the only PCI Express display devices currently supported
(bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
documented as allowed on the Root Complex by the first item in the list.
However, this item documents an additional consideration for placing
devices which was not previously mentioned, and may be relevant for PCIe
devices offering VGA compatibility in the future.

[1]: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/XG2RN3HKVRDEDTLA2PRELLIENIIH7II7/#XVP3I2KQVZHSTDA4SNVKOITWGRGSDU3F

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
Changes since v1:
 * Replace my overly-broad exception for devices requiring ioport access
   with a list item specifically for PCI Express devices offering VGA
   Compatibility provided by Laszlo Ersek.
 * Rewrite the commit message based on my improved understanding of the
   issue and the improved scope of the change.

P.S. Let me know if the Signed-off-by tag is not appropriate for either
of us.  I'm not clear on the etiquette of including someone else's
sign-off, but also don't want to misrepresent myself as the source of
your work.

 docs/pcie.txt | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 89e3502075..59b26817f9 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -48,13 +48,17 @@ Place only the following kinds of devices directly on the Root Complex:
         strangely when PCI Express devices are integrated
         with the Root Complex.
 
-    (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
+    (2) Assigned PCI Express GPUs that offer legacy VGA compatibility, and
+        that such compatibility is expected of (due to booting with SeaBIOS,
+        or due to UEFI driver bugs or native OS driver bugs).
+
+    (3) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
         hierarchies.
 
-    (3) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
+    (4) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
         hierarchies.
 
-    (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
+    (5) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
         are needed.
 
    pcie.0 bus
-- 
2.35.1

On 06/12/22 19:32, Kevin Locke wrote:
> PCI Express devices which use legacy VGA compatibility should be placed
> on the Root Complex.  This simplifies ioport access to VGA registers,
> which requires use of a special exception bit to work across PCI(e)
> bridges.  It is also necessary for ioport access to VESA BIOS Extension
> (VBE) registers, which is not forwarded over PCI(e) bridges, even with
> the special exception bit for VGA register access.[1]
> 
> Update the PCI Express Guidelines to add these to the list of devices
> which can be placed directly on the Root Complex.
> 
> Note that the only PCI Express display devices currently supported
> (bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
> Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
> documented as allowed on the Root Complex by the first item in the list.
> However, this item documents an additional consideration for placing
> devices which was not previously mentioned, and may be relevant for PCIe
> devices offering VGA compatibility in the future.
> 
> [1]: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/XG2RN3HKVRDEDTLA2PRELLIENIIH7II7/#XVP3I2KQVZHSTDA4SNVKOITWGRGSDU3F
> 
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

Please make this a "Suggested-by: Laszlo Ersek <lersek@redhat.com>"
(concerning the text in the patch body).

The commit message looks OK to me, but I'd like Gerd and/or Alex to
approve it.

Thanks!
Laszlo

> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---
> Changes since v1:
>  * Replace my overly-broad exception for devices requiring ioport access
>    with a list item specifically for PCI Express devices offering VGA
>    Compatibility provided by Laszlo Ersek.
>  * Rewrite the commit message based on my improved understanding of the
>    issue and the improved scope of the change.
> 
> P.S. Let me know if the Signed-off-by tag is not appropriate for either
> of us.  I'm not clear on the etiquette of including someone else's
> sign-off, but also don't want to misrepresent myself as the source of
> your work.
> 
>  docs/pcie.txt | 10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/pcie.txt b/docs/pcie.txt
> index 89e3502075..59b26817f9 100644
> --- a/docs/pcie.txt
> +++ b/docs/pcie.txt
> @@ -48,13 +48,17 @@ Place only the following kinds of devices directly on the Root Complex:
>          strangely when PCI Express devices are integrated
>          with the Root Complex.
>  
> -    (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
> +    (2) Assigned PCI Express GPUs that offer legacy VGA compatibility, and
> +        that such compatibility is expected of (due to booting with SeaBIOS,
> +        or due to UEFI driver bugs or native OS driver bugs).
> +
> +    (3) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
>          hierarchies.
>  
> -    (3) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
> +    (4) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
>          hierarchies.
>  
> -    (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
> +    (5) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
>          are needed.
>  
>     pcie.0 bus
>

On Mon, Jun 13, 2022 at 03:47:04PM +0200, Laszlo Ersek wrote:
> On 06/12/22 19:32, Kevin Locke wrote:
> > PCI Express devices which use legacy VGA compatibility should be placed
> > on the Root Complex.  This simplifies ioport access to VGA registers,
> > which requires use of a special exception bit to work across PCI(e)
> > bridges.  It is also necessary for ioport access to VESA BIOS Extension
> > (VBE) registers, which is not forwarded over PCI(e) bridges, even with
> > the special exception bit for VGA register access.[1]
> > 
> > Update the PCI Express Guidelines to add these to the list of devices
> > which can be placed directly on the Root Complex.
> > 
> > Note that the only PCI Express display devices currently supported
> > (bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
> > Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
> > documented as allowed on the Root Complex by the first item in the list.
> > However, this item documents an additional consideration for placing
> > devices which was not previously mentioned, and may be relevant for PCIe
> > devices offering VGA compatibility in the future.

Well, the *key* problem is emulated VGA devices with VBE registers in
io address space, because those are not forwarded over bridges.

For normal VGA registers this isn't much of a problem (in theory, not
fully sure whenever that holds in practice, Alex?).  The linux kernel
knows how to use the bridge control register to manage access to VGA
registers.

So, if the document already covers vga & qxl & virtio-vga (didn't check
that beforehand) I'm not sure we actually need an update ...

take care,
  Gerd

On Tue, 14 Jun 2022 10:52:52 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> On Mon, Jun 13, 2022 at 03:47:04PM +0200, Laszlo Ersek wrote:
> > On 06/12/22 19:32, Kevin Locke wrote:  
> > > PCI Express devices which use legacy VGA compatibility should be placed
> > > on the Root Complex.  This simplifies ioport access to VGA registers,
> > > which requires use of a special exception bit to work across PCI(e)
> > > bridges.  It is also necessary for ioport access to VESA BIOS Extension
> > > (VBE) registers, which is not forwarded over PCI(e) bridges, even with
> > > the special exception bit for VGA register access.[1]
> > > 
> > > Update the PCI Express Guidelines to add these to the list of devices
> > > which can be placed directly on the Root Complex.
> > > 
> > > Note that the only PCI Express display devices currently supported
> > > (bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
> > > Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
> > > documented as allowed on the Root Complex by the first item in the list.
> > > However, this item documents an additional consideration for placing
> > > devices which was not previously mentioned, and may be relevant for PCIe
> > > devices offering VGA compatibility in the future.  
> 
> Well, the *key* problem is emulated VGA devices with VBE registers in
> io address space, because those are not forwarded over bridges.
> 
> For normal VGA registers this isn't much of a problem (in theory, not
> fully sure whenever that holds in practice, Alex?).  The linux kernel
> knows how to use the bridge control register to manage access to VGA
> registers.

Yes, AUIU the issue is entirely with the extended VBE requirements, the
VGA ranges are fully routable through the VGA control registers on the
bridge.  The only bare metal issue I'm aware of with VGA routing is
that we cannot route around Intel IGD.  IIRC, this latter quirk is the
only reason that enabling VGA routing for a vfio-pci device is
considered experimental, but it very much does work when there's no
host device silently consuming those ranges.

We've also historically had issues with AMD graphics drivers assuming
an express link which can lead to driver segfaults if those devices are
placed on the root complex.  OTOH, I'm not aware of any specific issues
with placing assigned VGA class GPUs in configurations with a root port.

I'd therefore expect any configuration guidelines we're proposing to be
very specific to devices that make use of VBE, not just VGA devices in
general.  Thanks,

Alex

On Tue, 2022-06-14 at 10:52 +0200, Gerd Hoffmann wrote:
>> On 06/12/22 19:32, Kevin Locke wrote:
>>> PCI Express devices which use legacy VGA compatibility should be placed
>>> on the Root Complex.  This simplifies ioport access to VGA registers,
>>> which requires use of a special exception bit to work across PCI(e)
>>> bridges.  It is also necessary for ioport access to VESA BIOS Extension
>>> (VBE) registers, which is not forwarded over PCI(e) bridges, even with
>>> the special exception bit for VGA register access.[1]
>>> 
>>> Update the PCI Express Guidelines to add these to the list of devices
>>> which can be placed directly on the Root Complex.
>>> 
>>> Note that the only PCI Express display devices currently supported
>>> (bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
>>> Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
>>> documented as allowed on the Root Complex by the first item in the list.
>>> However, this item documents an additional consideration for placing
>>> devices which was not previously mentioned, and may be relevant for PCIe
>>> devices offering VGA compatibility in the future.
> 
> Well, the *key* problem is emulated VGA devices with VBE registers in
> io address space, because those are not forwarded over bridges.
> 
> For normal VGA registers this isn't much of a problem (in theory, not
> fully sure whenever that holds in practice, Alex?).  The linux kernel
> knows how to use the bridge control register to manage access to VGA
> registers.
> 
> So, if the document already covers vga & qxl & virtio-vga (didn't check
> that beforehand) I'm not sure we actually need an update ...

Section 2.1 Root Bus mentions attaching legacy PCI devices to the Root
Complex.  VGA/qxl-vga/virtio-vga are implicitly included (if the
reader is aware they are PCI, not PCIe), but they are not specifically
mentioned in the document.  By my reading, the document does not
recommend for or against attaching legacy PCI devices to the Root
Complex, other than noting hot-unplugging from the Root Complex is not
supported (in Section 2.3) and the general advice to prefer flat
hierarchies.

There is currently no mention of VGA or VBE in the document.

I think documenting the issue with VBE registers would be helpful.
Doing so with a recommendation for how to avoid the issue seems even
better.  Would a recommendation to attach a device which supports VBE
to the Root Complex if VBE will be used by the guest make sense?

As you noted, applying the recommendation to all VGA compatible
devices may be too broad.  I'm not sure whether it makes sense to
recommend attaching VGA compatible devices to the Root Complex to
avoid the complexity of the VGA exception bits, or if that is a
non-issue.  In fact, if I understand correctly, it may make sense to
recommend attaching VGA compatible devices to separate PCI bridges if
the VM will have multiple VGA compatible devices so that the guest can
perform VGA arbitration.

Unless I hear otherwise, I'll plan to create a v4 which documents the
issue with VBE registers more specifically.  Any suggestions for how
best to do that would be appreciated.

Cheers,
Kevin

  Hi,

> I think documenting the issue with VBE registers would be helpful.
> Doing so with a recommendation for how to avoid the issue seems even
> better.  Would a recommendation to attach a device which supports VBE
> to the Root Complex if VBE will be used by the guest make sense?

Yes.  This affects all vga-compatible devices emulated by qemu except cirrus.

> As you noted, applying the recommendation to all VGA compatible
> devices may be too broad.  I'm not sure whether it makes sense to
> recommend attaching VGA compatible devices to the Root Complex to
> avoid the complexity of the VGA exception bits, or if that is a
> non-issue.  In fact, if I understand correctly, it may make sense to
> recommend attaching VGA compatible devices to separate PCI bridges if
> the VM will have multiple VGA compatible devices so that the guest can
> perform VGA arbitration.

Yes.  Also guest drivers for pci-attached might be confused in case they
find a pcie gpu which is *not* in a pcie root port (which is impossible
on physical hardware).

take care,
  Gerd

PCI Express devices which use legacy VGA compatibility should be placed
on the Root Complex.  This simplifies ioport access to VGA registers,
which requires use of a special exception bit to work across PCI(e)
bridges.  It is also necessary for ioport access to VESA BIOS Extension
(VBE) registers, which is not forwarded over PCI(e) bridges, even with
the special exception bit for VGA register access.[1]

Update the PCI Express Guidelines to add these to the list of devices
which can be placed directly on the Root Complex.

Note that the only PCI Express display devices currently supported
(bochs-display and virtio-gpu-pci) do not offer VGA compatibility.
Legacy PCI devices (e.g. vga, qxl-vga, virtio-vga) are already
documented as allowed on the Root Complex by the first item in the list.
However, this item documents an additional consideration for placing
devices which was not previously mentioned, and may be relevant for PCIe
devices offering VGA compatibility in the future.

[1]: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/XG2RN3HKVRDEDTLA2PRELLIENIIH7II7/#XVP3I2KQVZHSTDA4SNVKOITWGRGSDU3F

Suggested-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
Changes since v2:
 * Change Signed-off-by to Suggested-by for Laszlo Ersek, as suggested
   by Laszlo Ersek.

Changes since v1:
 * Replace my overly-broad exception for devices requiring ioport access
   with a list item specifically for PCI Express devices offering VGA
   Compatibility provided by Laszlo Ersek.
 * Rewrite the commit message based on my improved understanding of the
   issue and the improved scope of the change.

 docs/pcie.txt | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 89e3502075..59b26817f9 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -48,13 +48,17 @@ Place only the following kinds of devices directly on the Root Complex:
         strangely when PCI Express devices are integrated
         with the Root Complex.
 
-    (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
+    (2) Assigned PCI Express GPUs that offer legacy VGA compatibility, and
+        that such compatibility is expected of (due to booting with SeaBIOS,
+        or due to UEFI driver bugs or native OS driver bugs).
+
+    (3) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
         hierarchies.
 
-    (3) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
+    (4) PCI Express to PCI Bridge (pcie-pci-bridge), for starting legacy PCI
         hierarchies.
 
-    (4) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
+    (5) Extra Root Complexes (pxb-pcie), if multiple PCI Express Root Buses
         are needed.
 
    pcie.0 bus
-- 
2.35.1

Mention devices which support VESA BIOS Extensions (VBE) specifically in
the list item of the Root Bus section discussing devices placed on the
Root Complex.

VESA BIOS Extensions (VBE) present a particular challenge not currently
noted in the recommendations:  ioport access to VBE registers is not
forwarded over PCI(e) bridges, even when using the special exception bit
for VGA register access.[1]  As a result, devices supporting VBE must be
placed on the Root Complex for VBE to be usable.

The new documentation also clarifies that VBE is used on systems with
BIOS firmware, including SeaBIOS, and that it is not generally used on
systems with UEFI firmware, including OVMF, where UEFI GOP is used.  I
say "generally" because VBE may be used on UEFI systems which boot using
a Compatibility Support Module (CSM), such as SeaBIOS.[2]  This
clarification is intended to help users who may not be familiar with VBE
to determine whether it is relevant to their situation.

[1]: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/XG2RN3HKVRDEDTLA2PRELLIENIIH7II7/#XVP3I2KQVZHSTDA4SNVKOITWGRGSDU3F
[2]: https://www.mail-archive.com/edk2-devel@lists.sourceforge.net/msg03788.html

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---
Changes since v3:
 * Rewrite the doc addition, limiting it to VGA devices with VBE, as
   VBE is the only problematic feature.
 * Rewrite the commit message as a result of the new scope.
 * Mention VBE in relation to BIOS and UEFI to help users determine
   whether they may be using VBE.
 * Make the doc addition a new paragraph of the first list item, rather
   than its own list item, since it raises a consideration for PCI
   graphics devices covered by that list item, rather than a separate
   class of devices.
 * Drop Suggested-by: Laszlo Ersek.  Since I rewrote the text, I can no
   longer blame Laszlo for any issues in it.

Changes since v2:
 * Change Signed-off-by to Suggested-by for Laszlo Ersek, as suggested
   by Laszlo Ersek.

Changes since v1:
 * Replace my overly-broad exception for devices requiring ioport access
   with a list item specifically for PCI Express devices offering VGA
   Compatibility provided by Laszlo Ersek.
 * Rewrite the commit message based on my improved understanding of the
   issue and the improved scope of the change.

 docs/pcie.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 89e3502075..21c2624c44 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -48,6 +48,12 @@ Place only the following kinds of devices directly on the Root Complex:
         strangely when PCI Express devices are integrated
         with the Root Complex.
 
+        VGA compatible devices which support VESA BIOS Extensions (VBE) must
+        be placed on the Root Complex for the VBE registers to be accessible,
+        as the IO port access is not forwarded over bridges or root ports.
+        VBE may be used on systems with BIOS firmware (e.g. SeaBIOS).  It is
+        not generally used on systems with UEFI firmware (e.g. OVMF).
+
     (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
         hierarchies.
 
-- 
2.35.1