linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Jeff Garzik <jeff@garzik.org>, Tejun Heo <htejun@gmail.com>,
	gregkh@suse.de, linux-kernel@vger.kernel.org,
	linux-pci@atrey.karlin.mff.cuni.cz,
	michal.k.k.piotrowski@gmail.com, linux-ide@vger.kernel.org,
	tglx@linutronix.de, shemminger@linux-foundation.org,
	mlord@pobox.com, linux-pm@lists.osdl.org
Cc: Andi Kleen <ak@suse.de>
Subject: Re: [PATCH/RFC] PCI prepare/activate instead of enable to avoid IRQ storm and rogue DMA access
Date: Thu, 15 Mar 2007 14:47:42 +0900	[thread overview]
Message-ID: <45F8DDFE.4080504@gmail.com> (raw)
In-Reply-To: <20070314225818.GB7194@flint.arm.linux.org.uk>

[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

      parent reply	other threads:[~2007-03-15  5:47 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-03-14 15:23 [PATCH/RFC] PCI prepare/activate instead of enable to avoid IRQ storm and rogue DMA access Tejun Heo
2007-03-14 17:04 ` Alan Stern
2007-03-14 17:07 ` Stephen Hemminger
2007-03-15  2:37   ` Tejun Heo
2007-03-15  6:45     ` Grant Grundler
2007-03-16 21:21     ` Stephen Hemminger
2007-03-14 21:46 ` Andi Kleen
2007-03-15  2:39   ` Tejun Heo
2007-03-15 10:17     ` Andi Kleen
2007-03-15 11:41   ` Vivek Goyal
2007-03-14 21:56 ` Russell King
2007-03-14 22:34   ` Jeff Garzik
2007-03-14 22:58     ` Russell King
2007-03-14 23:16       ` Jeff Garzik
2007-03-15  5:47       ` Tejun Heo [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45F8DDFE.4080504@gmail.com \
    --to=htejun@gmail.com \
    --cc=ak@suse.de \
    --cc=gregkh@suse.de \
    --cc=jeff@garzik.org \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@atrey.karlin.mff.cuni.cz \
    --cc=linux-pm@lists.osdl.org \
    --cc=michal.k.k.piotrowski@gmail.com \
    --cc=mlord@pobox.com \
    --cc=shemminger@linux-foundation.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).