Special handling of display/VGA devices in hotplug drivers

Special handling of display/VGA devices in hotplug drivers

[Bjorn Helgaas]

It looks like you added the initial pciehp driver [1], which includes
the following code in pciehp_disable_slot():

+ if (class_code == PCI_BASE_CLASS_DISPLAY) {
+ /* Display/Video adapter (not supported) */
+ rc = REMOVE_NOT_SUPPORTED;

+ /* If it's a bridge, check the VGA Enable bit */
+ if ((header_type & 0x7F) == PCI_HEADER_TYPE_BRIDGE) {
+ rc = pci_bus_read_config_byte (pci_bus, devfn, PCI_BRIDGE_CONTROL, &BCR);
+ if (rc)
+ return rc;
+
+ /* If the VGA Enable bit is set, remove isn't supported */
+ if (BCR & PCI_BRIDGE_CTL_VGA) {
+ rc = REMOVE_NOT_SUPPORTED;

I'm trying to figure out why VGA devices are handled specially.  I
can't find anything in the PCI specs that mentions this.  Most of the
other PCI hotplug drivers have similar code.  Do you remember anything
about this?

Bjorn


[1] https://git.kernel.org/cgit/linux/kernel/git/tglx/history.git/commit/drivers/pci/hotplug/pciehp_ctrl.c?id=c16b4b14d9806e639f4afefa2d651a857a212afe

Re: Special handling of display/VGA devices in hotplug drivers

[Greg Kroah-Hartman]

On Thu, Dec 11, 2014 at 10:34:30AM -0700, Bjorn Helgaas wrote:
> It looks like you added the initial pciehp driver [1], which includes
> the following code in pciehp_disable_slot():
> 
> + if (class_code == PCI_BASE_CLASS_DISPLAY) {
> + /* Display/Video adapter (not supported) */
> + rc = REMOVE_NOT_SUPPORTED;
> 
> + /* If it's a bridge, check the VGA Enable bit */
> + if ((header_type & 0x7F) == PCI_HEADER_TYPE_BRIDGE) {
> + rc = pci_bus_read_config_byte (pci_bus, devfn, PCI_BRIDGE_CONTROL, &BCR);
> + if (rc)
> + return rc;
> +
> + /* If the VGA Enable bit is set, remove isn't supported */
> + if (BCR & PCI_BRIDGE_CTL_VGA) {
> + rc = REMOVE_NOT_SUPPORTED;
> 
> I'm trying to figure out why VGA devices are handled specially.  I
> can't find anything in the PCI specs that mentions this.  Most of the
> other PCI hotplug drivers have similar code.  Do you remember anything
> about this?

The PCI spec said that you were not allowed to hotplug VGA drivers.  The
big issue is that POST usually needs to run on those things, and there
is no way to POST a PCI hotplugged device.

Does the spec not say that anymore?  I haven't looked in years at it...

Do you want to hot-add a VGA device?  Are these lines causing a problem
with something?

thanks,

greg k-h

Re: Special handling of display/VGA devices in hotplug drivers

[Jesse Barnes]

On Thu, 11 Dec 2014 13:11:36 -0500
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Thu, Dec 11, 2014 at 10:34:30AM -0700, Bjorn Helgaas wrote:
> > It looks like you added the initial pciehp driver [1], which
> > includes the following code in pciehp_disable_slot():
> > 
> > + if (class_code == PCI_BASE_CLASS_DISPLAY) {
> > + /* Display/Video adapter (not supported) */
> > + rc = REMOVE_NOT_SUPPORTED;
> > 
> > + /* If it's a bridge, check the VGA Enable bit */
> > + if ((header_type & 0x7F) == PCI_HEADER_TYPE_BRIDGE) {
> > + rc = pci_bus_read_config_byte (pci_bus, devfn,
> > PCI_BRIDGE_CONTROL, &BCR);
> > + if (rc)
> > + return rc;
> > +
> > + /* If the VGA Enable bit is set, remove isn't supported */
> > + if (BCR & PCI_BRIDGE_CTL_VGA) {
> > + rc = REMOVE_NOT_SUPPORTED;
> > 
> > I'm trying to figure out why VGA devices are handled specially.  I
> > can't find anything in the PCI specs that mentions this.  Most of
> > the other PCI hotplug drivers have similar code.  Do you remember
> > anything about this?
> 
> The PCI spec said that you were not allowed to hotplug VGA drivers.
> The big issue is that POST usually needs to run on those things, and
> there is no way to POST a PCI hotplugged device.
> 
> Does the spec not say that anymore?  I haven't looked in years at
> it...
> 
> Do you want to hot-add a VGA device?  Are these lines causing a
> problem with something?

Yeah, the legacy I/O regions get routed through the bridge with the VGA
bit set, and most legacy code probably can't handle that (whether POST,
VBIOS, or VGA drivers).

There is some code for moving the VGA routing around, so that might be
an option if you wanted to remove such a bridge.  You'd have to find a
VGA device under another bridge, and enable routing to that first, then
you could do the remove.

Jesse

Re: Special handling of display/VGA devices in hotplug drivers

[Guenter Roeck]

On Thu, Dec 11, 2014 at 11:32:43AM -0800, Jesse Barnes wrote:
> On Thu, 11 Dec 2014 13:11:36 -0500
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> > On Thu, Dec 11, 2014 at 10:34:30AM -0700, Bjorn Helgaas wrote:
> > > It looks like you added the initial pciehp driver [1], which
> > > includes the following code in pciehp_disable_slot():
> > > 
> > > + if (class_code == PCI_BASE_CLASS_DISPLAY) {
> > > + /* Display/Video adapter (not supported) */
> > > + rc = REMOVE_NOT_SUPPORTED;
> > > 
> > > + /* If it's a bridge, check the VGA Enable bit */
> > > + if ((header_type & 0x7F) == PCI_HEADER_TYPE_BRIDGE) {
> > > + rc = pci_bus_read_config_byte (pci_bus, devfn,
> > > PCI_BRIDGE_CONTROL, &BCR);
> > > + if (rc)
> > > + return rc;
> > > +
> > > + /* If the VGA Enable bit is set, remove isn't supported */
> > > + if (BCR & PCI_BRIDGE_CTL_VGA) {
> > > + rc = REMOVE_NOT_SUPPORTED;
> > > 
> > > I'm trying to figure out why VGA devices are handled specially.  I
> > > can't find anything in the PCI specs that mentions this.  Most of
> > > the other PCI hotplug drivers have similar code.  Do you remember
> > > anything about this?
> > 
> > The PCI spec said that you were not allowed to hotplug VGA drivers.
> > The big issue is that POST usually needs to run on those things, and
> > there is no way to POST a PCI hotplugged device.
> > 
> > Does the spec not say that anymore?  I haven't looked in years at
> > it...
> > 
> > Do you want to hot-add a VGA device?  Are these lines causing a
> > problem with something?
> 
> Yeah, the legacy I/O regions get routed through the bridge with the VGA
> bit set, and most legacy code probably can't handle that (whether POST,
> VBIOS, or VGA drivers).
> 
> There is some code for moving the VGA routing around, so that might be
> an option if you wanted to remove such a bridge.  You'd have to find a
> VGA device under another bridge, and enable routing to that first, then
> you could do the remove.
> 

The problem at hand though is that the current code may mis-detect a device as
VGA device after it was removed. In that case, reading a configuration register
returns 0xff, meaning the VGA bit is set, and the kernel refuses to remove it.

See [1] for a description of the problem and its impact.

Guenter
---
[1] https://lkml.org/lkml/2014/11/20/714

Re: Special handling of display/VGA devices in hotplug drivers

[Bjorn Helgaas]

[+cc Praveen]

On Thu, Dec 11, 2014 at 12:32 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Thu, 11 Dec 2014 13:11:36 -0500
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
>> On Thu, Dec 11, 2014 at 10:34:30AM -0700, Bjorn Helgaas wrote:
>> > It looks like you added the initial pciehp driver [1], which
>> > includes the following code in pciehp_disable_slot():
>> >
>> > + if (class_code == PCI_BASE_CLASS_DISPLAY) {
>> > + /* Display/Video adapter (not supported) */
>> > + rc = REMOVE_NOT_SUPPORTED;
>> >
>> > + /* If it's a bridge, check the VGA Enable bit */
>> > + if ((header_type & 0x7F) == PCI_HEADER_TYPE_BRIDGE) {
>> > + rc = pci_bus_read_config_byte (pci_bus, devfn,
>> > PCI_BRIDGE_CONTROL, &BCR);
>> > + if (rc)
>> > + return rc;
>> > +
>> > + /* If the VGA Enable bit is set, remove isn't supported */
>> > + if (BCR & PCI_BRIDGE_CTL_VGA) {
>> > + rc = REMOVE_NOT_SUPPORTED;
>> >
>> > I'm trying to figure out why VGA devices are handled specially.  I
>> > can't find anything in the PCI specs that mentions this.  Most of
>> > the other PCI hotplug drivers have similar code.  Do you remember
>> > anything about this?
>>
>> The PCI spec said that you were not allowed to hotplug VGA drivers.
>> The big issue is that POST usually needs to run on those things, and
>> there is no way to POST a PCI hotplugged device.

I don't think this is a problem any more, is it?  I think X can
execute option ROMs, and if we assign the guest to a VM, the guest
BIOS can also do it.

> Yeah, the legacy I/O regions get routed through the bridge with the VGA
> bit set, and most legacy code probably can't handle that (whether POST,
> VBIOS, or VGA drivers).
>
> There is some code for moving the VGA routing around, so that might be
> an option if you wanted to remove such a bridge.  You'd have to find a
> VGA device under another bridge, and enable routing to that first, then
> you could do the remove.

The legacy code thing does seem like an issue.  Since ac81860ea073, we
don't actually fail when removing a VGA device; we only fail when
removing a bridge with VGA routing enabled.  Maybe that should be
tweaked so we fail when removing either a bridge or a VGA device to
which the legacy ranges are currently routed.  Then we could still
remove secondary VGA devices like the computational GPUs that
motivated ac81860ea073.

Failing in pciehp_unconfigure_device() makes sense if the user pressed
the attention button to request removal.  But if the bridge was
surprise-removed, e.g., user pulled out an ExpressCard, the bridge is
already gone, and failing just means we can't clean things up, so
we're going to leave drivers bound to the bridge and downstream
devices.

Bjorn

Re: Special handling of display/VGA devices in hotplug drivers

[One Thousand Gnomes]

> >> The PCI spec said that you were not allowed to hotplug VGA drivers.
> >> The big issue is that POST usually needs to run on those things, and
> >> there is no way to POST a PCI hotplugged device.
> 
> I don't think this is a problem any more, is it?  I think X can
> execute option ROMs, and if we assign the guest to a VM, the guest
> BIOS can also do it.

Providing the legacy I/O is routed to that device, it doesn't use DMA and
a few other things. X has been able to POST cards with vm86 or the 8086
emulator stuff for a long time (it had to in order to use most PC cards on
non x86 boxes).

Your bigger problem is the marvellous reliable way that X handles a GPU
simply vanishing, especially a legacy one.

Alan