When a PCI device is disabled via pci_disable_device(), it's still
left decoding its BAR resource ranges even though its driver
will have likely released those regions (and may even have
unloaded). pci_enable_device() already explicitly enables
BAR resource decode for the device being enabled. This patch
disables resource decode for the PCI device being disabled,
making it symmetric with the enable call.
I saw this while doing something else, not because of a
problem report. Still, seems to be the correct thing to do.
Signed-off-by: Rajesh Shah <[email protected]>
arch/i386/pci/common.c | 1 +
arch/i386/pci/i386.c | 9 +++++++++
arch/i386/pci/pci.h | 1 +
3 files changed, 11 insertions(+)
Index: linux-2.6.17-rc3-git7/arch/i386/pci/common.c
===================================================================
--- linux-2.6.17-rc3-git7.orig/arch/i386/pci/common.c
+++ linux-2.6.17-rc3-git7/arch/i386/pci/common.c
@@ -288,6 +288,7 @@ int pcibios_enable_device(struct pci_dev
void pcibios_disable_device (struct pci_dev *dev)
{
+ pcibios_disable_resources(dev);
if (pcibios_disable_irq)
pcibios_disable_irq(dev);
}
Index: linux-2.6.17-rc3-git7/arch/i386/pci/i386.c
===================================================================
--- linux-2.6.17-rc3-git7.orig/arch/i386/pci/i386.c
+++ linux-2.6.17-rc3-git7/arch/i386/pci/i386.c
@@ -242,6 +242,15 @@ int pcibios_enable_resources(struct pci_
return 0;
}
+void pcibios_disable_resources(struct pci_dev *dev)
+{
+ u16 cmd;
+
+ pci_read_config_word(dev, PCI_COMMAND, &cmd);
+ cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY);
+ pci_write_config_word(dev, PCI_COMMAND, cmd);
+}
+
/*
* If we set up a device for bus mastering, we need to check the latency
* timer as certain crappy BIOSes forget to set it properly.
Index: linux-2.6.17-rc3-git7/arch/i386/pci/pci.h
===================================================================
--- linux-2.6.17-rc3-git7.orig/arch/i386/pci/pci.h
+++ linux-2.6.17-rc3-git7/arch/i386/pci/pci.h
@@ -35,6 +35,7 @@ extern unsigned int pcibios_max_latency;
void pcibios_resource_survey(void);
int pcibios_enable_resources(struct pci_dev *, int);
+void pcibios_disable_resources(struct pci_dev *);
/* pci-pc.c */
>
> When a PCI device is disabled via pci_disable_device(), it's still
> left decoding its BAR resource ranges even though its driver
> will have likely released those regions (and may even have
> unloaded). pci_enable_device() already explicitly enables
> BAR resource decode for the device being enabled. This patch
> disables resource decode for the PCI device being disabled,
> making it symmetric with the enable call.
>
> I saw this while doing something else, not because of a
> problem report. Still, seems to be the correct thing to do.
I'm just wondering how this will react with VGA devices being run by
fbdev or the drm, I know the DRM never calls pci_disable_device, as
the card might require the bars enabled so it can do VGA, and which if
it is your primary VGA card, can cause you all kinds of troubles...
(like losing text mode)..
Alan Cox mentioned this somewhere before in relation to video cards..
Dave.
Dave Airlie wrote:
>>
>> When a PCI device is disabled via pci_disable_device(), it's still
>> left decoding its BAR resource ranges even though its driver
>> will have likely released those regions (and may even have
>> unloaded). pci_enable_device() already explicitly enables
>> BAR resource decode for the device being enabled. This patch
>> disables resource decode for the PCI device being disabled,
>> making it symmetric with the enable call.
>>
>> I saw this while doing something else, not because of a
>> problem report. Still, seems to be the correct thing to do.
>
> I'm just wondering how this will react with VGA devices being run by
> fbdev or the drm, I know the DRM never calls pci_disable_device, as
> the card might require the bars enabled so it can do VGA, and which if
> it is your primary VGA card, can cause you all kinds of troubles...
> (like losing text mode)..
Most, if not all PCI-based framebuffer drivers call pci_disable_device()
in their unload routine. Although it's very rare that framebuffer drivers
are unloaded, if you do, and the the resources are also disabled, it will
kill the VGA core of the card. You lose your text console and even
hang the machine.
>
> Alan Cox mentioned this somewhere before in relation to video cards..
Alan Cox, if I remember correctly, advises against calling pci_disable_device()
for video drivers when they unload.
Tony
On Thu, May 04, 2006 at 05:44:21PM +0800, Antonino A. Daplas wrote:
>
> Most, if not all PCI-based framebuffer drivers call pci_disable_device()
> in their unload routine. Although it's very rare that framebuffer drivers
> are unloaded, if you do, and the the resources are also disabled, it will
> kill the VGA core of the card. You lose your text console and even
> hang the machine.
>
> >
> > Alan Cox mentioned this somewhere before in relation to video cards..
>
> Alan Cox, if I remember correctly, advises against calling pci_disable_device()
> for video drivers when they unload.
>
Yeah, that's also what some other drivers do. For example, PCI/PCIE
bridges may support capabilities (like hotplug) that are controlled
by separate drivers. These drivers don't do pci_disable_device()
when they unload, since the bridge must continue to decode even
when the other capability driver is gone.
The problem is that most PCI bridges don't have any "extra"
resources padded into the address ranges they pass down. It
would be nice to be able to reuse address space released when
a device is disabled (e.g. for future hot-add), if it's really
no longer needed.
Rajesh
On Thu, May 04, 2006 at 01:01:57PM -0700, Rajesh Shah wrote:
> Yeah, that's also what some other drivers do. For example, PCI/PCIE
> bridges may support capabilities (like hotplug) that are controlled
> by separate drivers. These drivers don't do pci_disable_device()
> when they unload, since the bridge must continue to decode even
> when the other capability driver is gone.
>
> The problem is that most PCI bridges don't have any "extra"
> resources padded into the address ranges they pass down. It
> would be nice to be able to reuse address space released when
> a device is disabled (e.g. for future hot-add), if it's really
> no longer needed.
You could always reprogram the BARs. But I really wouldn't recommend
this; you'll just fragment the address space.