Re: [PATCH/RFC] PCI prepare/activate instead of enable to avoid IRQstorm and rogue DMA access

From: Tejun Heo
Date: Thu Mar 15 2007 - 01:48:19 EST


[cc'ing Andi, Hi!]

Hello,

Russell King wrote:
> On Wed, Mar 14, 2007 at 06:34:11PM -0400, Jeff Garzik wrote:
>> Russell King wrote:
>>> pci_enable_device() doesn't deal with this; in most PCI setups I've
>>> seen, there is no control at PCI level over whether a device generates
>>> an interrupt on the bus. Certainly the memory and io command enables
>> PCI grew an interrupt enable while you weren't looking:
>> PCI_COMMAND_INTX_DISABLE
>
> That's fine for devices which conform to the later PCI specs, but not
> all do.
>
>> It was added in PCI 2.3 I think.
>
> Correct.
>
>> Older PCI devices certainly do not have this standardized bit.
>
> No PCI device that I have has that bit - including the raid card I
> bought last year...

Many recent ATA and network controllers do and most new ones will
probably do.

> In any case, relying on such a new control bit to implement this kind
> of functionality would result in a very hit and miss result; Linux
> tends to get used on things other than the bleeding edge of hardware
> technology.

I don't think INTX_DISABLE is on the bleeding edge of hardware
technology and many common cases will benefit from using it (just think
about the number of newish notebook users). The problem with
INTX_DISABLE is that there doesn't seem to be any way to tell whether
writing to that bit is safe or not.

You are right in that turning off IRQ mechanisms in pci_enable_device()
doesn't fix all the problems as PCI-wise it only enables IO and memory
address space access, but to some extent it does because in the arch
code, it enables the IRQ line and the physical IRQ line might not be
shared even if the final IRQ number is shared (Andi, am I correct)?

Anyways, I think the proper solution is to make sure all generic IRQ
controls including INTX turned off early in the boot during PCI
subsystem initialization (ie. do the disable part of
pcim_prepare_device() early in the boot before any IRQ line is
requested) and let each driver enable after initialization as necessary
and do similar things during resume. Note that drivers still need to be
modified to signify when the device is initialized enough to enable IRQ,
and bus mastering.

We can also arch-dep IRQ enabling to the activation time. That will
give us more protection even when INTX_DISABLE is not available.

Thanks.

--
tejun
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/