[RFC][PATCH] docs: note exception for PCIe IO port access

[RFC][PATCH] docs: note exception for PCIe IO port access

[Kevin Locke]

ioport access is required for VESA BIOS Extensions (VBE).  Since ioport
access is not forwarded over PCI(e) bridges, graphics adapters must be
attached directly to the Root Complex in order for the BIOS to provide
VBE modes.  I'm very grateful to Gerd Hoffmann for explaining this on
the SeaBIOS mailing list.[1]

Update the PCI Express Guidelines to document this as an exception to
the recommendation to "Place only legacy PCI devices on the Root
Complex."

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

Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
---

This suggested documentation change is the result of my struggles to
understand why I was seeing a very limited set of display modes in one
of my virtual machines, as explained in the seabios ML post linked above
and an earlier post to qemu-discuss.[2]  I hope it may help avoid some
of these hassles for future users.

I'm far from being an expert in PCI(e), BIOS/VBE, or virtualization in
general, and would appreciate any suggestions on these docs changes.
I'm also curious about whether graphics devices are the only exception
and whether "Guest OSes are suspected to behave strangely when PCI
Express devices are integrated with the Root Complex" is still the case.
David Gibson had previously noted that graphics cards appear on the Root
Complex on real hardware.[3]

I notice that the HD Audio Controller on my T430 is an "Express Root
Complex Integrated Endpoint".  Is this a non-graphics example, or am I
misinterpreting the lspci output?

00:1b.0 Audio device: Intel Corporation 7 Series/C216 Chipset Family High Definition Audio Controller (rev 04)
	Subsystem: Lenovo 7 Series/C216 Chipset Family High Definition Audio Controller
	Flags: bus master, fast devsel, latency 0, IRQ 29, IOMMU group 6
	Memory at f1530000 (64-bit, non-prefetchable) [size=16K]
	Capabilities: [50] Power Management version 2
	Capabilities: [60] MSI: Enable+ Count=1/1 Maskable- 64bit+
	Capabilities: [70] Express Root Complex Integrated Endpoint, MSI 00
	Capabilities: [100] Virtual Channel
	Capabilities: [130] Root Complex Link
	Kernel driver in use: snd_hda_intel
	Kernel modules: snd_hda_intel

Thanks,
Kevin

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

diff --git a/docs/pcie.txt b/docs/pcie.txt
index 89e3502075..a23d93849b 100644
--- a/docs/pcie.txt
+++ b/docs/pcie.txt
@@ -48,6 +48,11 @@ Place only the following kinds of devices directly on the Root Complex:
         strangely when PCI Express devices are integrated
         with the Root Complex.
 
+        An exception to this rule is PCI Express devices which will be
+        accessed using IO ports.  For example, guests using BIOS firmware
+        require IO port access for graphics devices to provide VESA BIOS
+        Extensions (VBE).
+
     (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
         hierarchies.
 
-- 
2.35.1

Re: [RFC][PATCH] docs: note exception for PCIe IO port access

[Laszlo Ersek]

+Alex

On 06/09/22 04:00, Kevin Locke wrote:
> ioport access is required for VESA BIOS Extensions (VBE).  Since ioport
> access is not forwarded over PCI(e) bridges, graphics adapters must be
> attached directly to the Root Complex in order for the BIOS to provide
> VBE modes.  I'm very grateful to Gerd Hoffmann for explaining this on
> the SeaBIOS mailing list.[1]
>
> Update the PCI Express Guidelines to document this as an exception to
> the recommendation to "Place only legacy PCI devices on the Root
> Complex."
>
> [1]: https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/XG2RN3HKVRDEDTLA2PRELLIENIIH7II7/#XVP3I2KQVZHSTDA4SNVKOITWGRGSDU3F
>
> Signed-off-by: Kevin Locke <kevin@kevinlocke.name>
> ---

I think we're discussing three separate things here:

(a) plugging a legacy PCI device into an Express slot
(b) the inverse: plugging an Express device into a legacy PCI slot
(c) plugging an Express device into a slot on the PCIe root complex

Telling these apart is not easy because:

> QEMU does not have a clear socket-device matching mechanism
> and allows any PCI/PCI Express device to be plugged into any
> PCI/PCI Express slot.

Now,

(a) your original (non-functional) use case:

qemu-system-x86_64 \
    -no-user-config \
    -nodefaults \
    -machine q35,accel=kvm \
    -m 1G \
    -cdrom "$iso" \
    -device pcie-root-port,id=pci.1,bus=pcie.0 \
    -device VGA,bus=pci.1

violates the following part of "pcie.txt":

> Plugging a PCI device into a PCI Express slot might not always work and
> is weird anyway since it cannot be done for "bare metal".

AIUI, what Gerd explains in that SeaBIOS thread is why and how exactly
such an attempt breaks. The statement that it would break is already
spelled out in "pcie.txt".

So I think that, given strictly your original report on the SeaBIOS
list, no updates to "pcie.txt" are necessary.

Note that your original (functional) use case:

qemu-system-x86_64 \
    -no-user-config \
    -nodefaults \
    -machine q35,accel=kvm \
    -m 1G \
    -cdrom "$iso" \
    -device VGA

does not conflict with

> Place only the following kinds of devices directly on the Root Complex:
>     (1) PCI Devices (e.g. network card, graphics card, IDE controller),
>         not controllers. Place only legacy PCI devices on
>         the Root Complex. These will be considered Integrated Endpoints.
>         Note: Integrated Endpoints are not hot-pluggable.
>
>         Although the PCI Express spec does not forbid PCI Express devices as
>         Integrated Endpoints, existing hardware mostly integrates legacy PCI
>         devices with the Root Complex. Guest OSes are suspected to behave
>         strangely when PCI Express devices are integrated
>         with the Root Complex.

because "-device VGA" is a legacy PCI device, not a PCI Express device.
So the second quoted paragraph does not apply to it at all, and the
first paragraph is in sync with your functional use case.

Then,

On 06/09/22 04:00, Kevin Locke wrote:
> This suggested documentation change is the result of my struggles to
> understand why I was seeing a very limited set of display modes in one
> of my virtual machines, as explained in the seabios ML post linked above
> and an earlier post to qemu-discuss.[2]  I hope it may help avoid some
> of these hassles for future users.

Per your original report, the misconfig on your side was not that you
placed a PCIe display controller on the root complex, but that you
placed a legacy PCI device into a PCIe root port (which is physically
impossible on a physical machine).

>
> I'm far from being an expert in PCI(e), BIOS/VBE, or virtualization in
> general, and would appreciate any suggestions on these docs changes.
> I'm also curious about whether graphics devices are the only exception

While VGA is quirky ("there was only ever intended to be one device" --
see
<http://vfio.blogspot.com/2014/08/whats-deal-with-vga-arbitration.html>),
I maintain that it's sufficiently covered already (although not
specifically) by "pcie.txt"; see above. "-device VGA" is a legacy PCI
device, you can't plug it in an Express slot (root complex or port
alike).

In the blog post at
<https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/>, Gerd
explains that (effectively) only "bochs-display" and "virtio-gpu-pci"
(from QEMU's emulated devices) may be placed in PCI Express slots (root
complex or separate port, alike).

*In theory*, scenario (b) applies to "bochs-display" and
"virtio-gpu-pci" when you plug them into a legacy PCI slot:

> Plugging a PCI Express device into a PCI slot will hide the Extended
> Configuration Space thus is also not recommended.

But this is fine, as these device models don't have extended
capabilities in practice. IOW, I'd imagine them (if they were physical
cards) as PCI Express devices without any use for their extended config
spaces. You could plug them into legacy PCI slots, but would see no
difference in behavior (apart from speed perhaps).

But, we might want to update "pcie.txt" for scenario (c):

On 06/09/22 04:00, Kevin Locke wrote:
> and whether "Guest OSes are suspected to behave strangely when PCI
> Express devices are integrated with the Root Complex" is still the case.

I vaguely recall that Alex didn't fully agree with this statement when
the document was being reviewed.

> David Gibson had previously noted that graphics cards appear on the Root
> Complex on real hardware.[3]

You forgot to provide the link for [3].

Anyway, the full paragraph in the document goes like

>         Although the PCI Express spec does not forbid PCI Express devices as
>         Integrated Endpoints, existing hardware mostly integrates legacy PCI
>         devices with the Root Complex. Guest OSes are suspected to behave
>         strangely when PCI Express devices are integrated
>         with the Root Complex.

So the recommendation is based on motherboards seen in the wild, and the
potential consequent assumptions in OSes. Whether that still holds up
with today's motherboards (and OSes), I have no clue about.

>
> I notice that the HD Audio Controller on my T430 is an "Express Root
> Complex Integrated Endpoint".  Is this a non-graphics example, or am I
> misinterpreting the lspci output?
>
> 00:1b.0 Audio device: Intel Corporation 7 Series/C216 Chipset Family High Definition Audio Controller (rev 04)
> 	Subsystem: Lenovo 7 Series/C216 Chipset Family High Definition Audio Controller
> 	Flags: bus master, fast devsel, latency 0, IRQ 29, IOMMU group 6
> 	Memory at f1530000 (64-bit, non-prefetchable) [size=16K]
> 	Capabilities: [50] Power Management version 2
> 	Capabilities: [60] MSI: Enable+ Count=1/1 Maskable- 64bit+
> 	Capabilities: [70] Express Root Complex Integrated Endpoint, MSI 00
> 	Capabilities: [100] Virtual Channel
> 	Capabilities: [130] Root Complex Link
> 	Kernel driver in use: snd_hda_intel
> 	Kernel modules: snd_hda_intel
>
> Thanks,
> Kevin
>
>  docs/pcie.txt | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/docs/pcie.txt b/docs/pcie.txt
> index 89e3502075..a23d93849b 100644
> --- a/docs/pcie.txt
> +++ b/docs/pcie.txt
> @@ -48,6 +48,11 @@ Place only the following kinds of devices directly on the Root Complex:
>          strangely when PCI Express devices are integrated
>          with the Root Complex.
>
> +        An exception to this rule is PCI Express devices which will be
> +        accessed using IO ports.

I find this too general; a PCI Express device is supposed to work
without IO resources. Graphics cards with legacy VGA compatibility are
the exception AIUI (see again Alex's blog about VGA arbitration), so we
should spell that out.

Furthermore, from QEMU's emulated graphics cards, all device models that
provide VGA compatibility are legacy PCI (not PCI Express) devices (see
again Gerd's post: VGA, virtio-vga, qxl-vga, cirrus-vga, ati-vga), so
I'd even restrict this docs update to assigned GPUs.

>                                   For example, guests using BIOS firmware
> +        require IO port access for graphics devices to provide VESA BIOS
> +        Extensions (VBE).
> +
>      (2) PCI Express Root Ports (ioh3420), for starting exclusively PCI Express
>          hierarchies.
>
>

How about inserting a new paragraph here, instead:

    (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).

Thanks
Laszlo

Re: [RFC][PATCH] docs: note exception for PCIe IO port access

[Laszlo Ersek]

On 06/09/22 09:45, Laszlo Ersek wrote:
> +Alex
> 
> On 06/09/22 04:00, Kevin Locke wrote:

> *In theory*, scenario (b) applies to "bochs-display" and
> "virtio-gpu-pci" when you plug them into a legacy PCI slot:
> 
>> Plugging a PCI Express device into a PCI slot will hide the Extended
>> Configuration Space thus is also not recommended.
> 
> But this is fine, as these device models don't have extended
> capabilities in practice. IOW, I'd imagine them (if they were physical
> cards) as PCI Express devices without any use for their extended config
> spaces. You could plug them into legacy PCI slots, but would see no
> difference in behavior (apart from speed perhaps).

Ignore the part starting with "IOW", that seems like brain fart on my
part, apologies! :/

Re: [RFC][PATCH] docs: note exception for PCIe IO port access

[Gerd Hoffmann]

  Hi,

> I find this too general; a PCI Express device is supposed to work
> without IO resources. Graphics cards with legacy VGA compatibility are
> the exception AIUI (see again Alex's blog about VGA arbitration), so we
> should spell that out.

Yes, it's an exception specifically for VGA ports.  Can be turned on and
off here ...

    $ sudo lspci -vvs00:1b.0 | grep BridgeCtl
	BridgeCtl: Parity- SERR+ NoISA- VGA- VGA16+ MAbort- >Reset- FastB2B-
                                        ^^^^^^^^^^^

... and this is what vgaarb uses to route vga register access to devices
behind pci bridges.

take care,
  Gerd

Re: [RFC][PATCH] docs: note exception for PCIe IO port access

[Kevin Locke]

Thanks Laszlo, I really appreciate the detailed explanation and
feedback!

On Thu, 2022-06-09 at 09:45 +0200, Laszlo Ersek wrote:
> (a) your original (non-functional) use case:
> 
> qemu-system-x86_64 \
>     -no-user-config \
>     -nodefaults \
>     -machine q35,accel=kvm \
>     -m 1G \
>     -cdrom "$iso" \
>     -device pcie-root-port,id=pci.1,bus=pcie.0 \
>     -device VGA,bus=pci.1
> 
> violates the following part of "pcie.txt":
> 
>> Plugging a PCI device into a PCI Express slot might not always work and
>> is weird anyway since it cannot be done for "bare metal".
> 
> AIUI, what Gerd explains in that SeaBIOS thread is why and how exactly
> such an attempt breaks. The statement that it would break is already
> spelled out in "pcie.txt".
> 
> So I think that, given strictly your original report on the SeaBIOS
> list, no updates to "pcie.txt" are necessary.
> 
> Note that your original (functional) use case:
> 
> qemu-system-x86_64 \
>     -no-user-config \
>     -nodefaults \
>     -machine q35,accel=kvm \
>     -m 1G \
>     -cdrom "$iso" \
>     -device VGA
> 
> does not conflict with
> 
>> Place only the following kinds of devices directly on the Root Complex:
>>     (1) PCI Devices (e.g. network card, graphics card, IDE controller),
>>         not controllers. Place only legacy PCI devices on
>>         the Root Complex. These will be considered Integrated Endpoints.
>>         Note: Integrated Endpoints are not hot-pluggable.
>>
>>         Although the PCI Express spec does not forbid PCI Express devices as
>>         Integrated Endpoints, existing hardware mostly integrates legacy PCI
>>         devices with the Root Complex. Guest OSes are suspected to behave
>>         strangely when PCI Express devices are integrated
>>         with the Root Complex.
> 
> because "-device VGA" is a legacy PCI device, not a PCI Express device.
> So the second quoted paragraph does not apply to it at all, and the
> first paragraph is in sync with your functional use case.

Ah, right you are.  My mistake.  Sorry for the noise.

I had thought virtio-vga might be an exception, since there was
discussion about changing virtio-vga to PCIe when connected to the
Root Complex[4], but it looks like the change was not made due to
compatibility concerns[5].  As you note below, this is currently the
case for all devices with VGA compatibility, so this docs change is
mostly a non-issue as long as this continues to be the case.

On Thu, 2022-06-09 at 09:45 +0200, Laszlo Ersek wrote:
> While VGA is quirky ("there was only ever intended to be one device" --
> see
> <http://vfio.blogspot.com/2014/08/whats-deal-with-vga-arbitration.html>),
> I maintain that it's sufficiently covered already (although not
> specifically) by "pcie.txt"; see above. "-device VGA" is a legacy PCI
> device, you can't plug it in an Express slot (root complex or port
> alike).
> 
> In the blog post at
> <https://www.kraxel.org/blog/2019/09/display-devices-in-qemu/>, Gerd
> explains that (effectively) only "bochs-display" and "virtio-gpu-pci"
> (from QEMU's emulated devices) may be placed in PCI Express slots (root
> complex or separate port, alike).

Those posts are both enlightening, thanks!

On Thu, 2022-06-09 at 09:45 +0200, Laszlo Ersek wrote:
> On 06/09/22 04:00, Kevin Locke wrote:
>> and whether "Guest OSes are suspected to behave strangely when PCI
>> Express devices are integrated with the Root Complex" is still the case.
> 
> I vaguely recall that Alex didn't fully agree with this statement when
> the document was being reviewed.
> 
>> David Gibson had previously noted that graphics cards appear on the Root
>> Complex on real hardware.[3]
> 
> You forgot to provide the link for [3].

Whoops.  Links to my qemu-discuss post[2] and David Gibson's post[3]
mentioning "the GPU shows up as an integrated PCI Express endpoint" on
his laptop are included in this email below.

On Thu, 2022-06-09 at 09:45 +0200, Laszlo Ersek wrote:
> On 06/09/22 04:00, Kevin Locke wrote:
> Anyway, the full paragraph in the document goes like
> 
>>         Although the PCI Express spec does not forbid PCI Express devices as
>>         Integrated Endpoints, existing hardware mostly integrates legacy PCI
>>         devices with the Root Complex. Guest OSes are suspected to behave
>>         strangely when PCI Express devices are integrated
>>         with the Root Complex.
> 
> So the recommendation is based on motherboards seen in the wild, and the
> potential consequent assumptions in OSes. Whether that still holds up
> with today's motherboards (and OSes), I have no clue about.
> 
> [...]
> 
>> --- a/docs/pcie.txt
>> +++ b/docs/pcie.txt
>> @@ -48,6 +48,11 @@ Place only the following kinds of devices directly on the Root Complex:
>>          strangely when PCI Express devices are integrated
>>          with the Root Complex.
>>
>> +        An exception to this rule is PCI Express devices which will be
>> +        accessed using IO ports.
> 
> I find this too general; a PCI Express device is supposed to work
> without IO resources. Graphics cards with legacy VGA compatibility are
> the exception AIUI (see again Alex's blog about VGA arbitration), so we
> should spell that out.
> 
> Furthermore, from QEMU's emulated graphics cards, all device models that
> provide VGA compatibility are legacy PCI (not PCI Express) devices (see
> again Gerd's post: VGA, virtio-vga, qxl-vga, cirrus-vga, ati-vga), so
> I'd even restrict this docs update to assigned GPUs.
> 
> [...]
> 
> How about inserting a new paragraph here, instead:
> 
>     (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).

That sounds great to me.  Would you like me to send that as a v2
patch, or would you prefer to make the change?

Thanks again,
Kevin

[2]: https://lists.nongnu.org/archive/html/qemu-discuss/2022-05/msg00053.html
[3]: https://lore.kernel.org/qemu-devel/20170209041634.GC14524@umbus/
[4]: https://lore.kernel.org/qemu-devel/20170208061602.17666-1-david@gibson.dropbear.id.au/T/
[5]: https://lore.kernel.org/qemu-devel/5ea3785c-b979-8b8c-3ab0-243d69384697@redhat.com/

Re: [RFC][PATCH] docs: note exception for PCIe IO port access

[Laszlo Ersek]

On 06/09/22 16:03, Kevin Locke wrote:
> On Thu, 2022-06-09 at 09:45 +0200, Laszlo Ersek wrote:

>>     (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).
> 
> That sounds great to me.  Would you like me to send that as a v2
> patch, or would you prefer to make the change?

Please send v2 :)

Thanks
Laszlo

[PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Kevin Locke]

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

Re: [PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Laszlo Ersek]

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
> 

[PATCH v3] docs: add PCIe root bus for VGA compat guideline

[Kevin Locke]

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

Re: [PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Gerd Hoffmann]

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

Re: [PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Kevin Locke]

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

Re: [PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Gerd Hoffmann]

  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

Re: [PATCH v2] docs: add PCIe root bus for VGA compat guideline

[Alex Williamson]

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

[PATCH v4] docs: mention devices with VBE on Root Complex

[Kevin Locke]

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