Linux Kernel Mailing List wrote:
> commit 134a11f0c37c043d3ea557ea15b95b084e3cc2c8
> tree c23bfd643913ea2d8cd01b17c1572b9602de7fd5
> parent c387fd85f84b9d89a75596325d8d6a0f730baf64
> author Takashi Iwai <[email protected]> 1163156917 +0100
> committer Linus Torvalds <[email protected]> 1163549067 -0800
>
> [PATCH] ALSA: hda-intel - Disable MSI support by default
>
> Disable MSI support on HD-audio driver as default since there are too
> many broken devices.
>
> The module option is changed from disable_msi to enable_msi, too. For
> turning MSI support on, pass enable_msi=1, instead.
>
> Signed-off-by: Takashi Iwai <[email protected]>
> Signed-off-by: Linus Torvalds <[email protected]>
:( Like AHCI, PCI MSI has -always- worked wonderfully for HD audio AFAIK.
Is a whitelist patch forthcoming?
Jeff
On Tue, 14 Nov 2006, Jeff Garzik wrote:
>
> :( Like AHCI, PCI MSI has -always- worked wonderfully for HD audio AFAIK.
That "AFAIK" is shorthand for "As Far As I haven't read any of the
bug-reports but Know", right?
> Is a whitelist patch forthcoming?
Probably not. The advantages of MSI aren't all that obvious, and the
disadvantages seem to be that it just doesn't work all that well for some
people.
The fact that it works for MOST people has absolutely zero relevance.
We've had too many frigging patches that have apparently been of the "this
works for me, I don't care if some other motherboard has problems" kind.
See for example:
http://lkml.org/lkml/2006/10/7/164
and yes, that HDA MSI _does_ seem to be causing problems.
So don't blather about "MSI never causes problems". It's broken. Please
stop living in denial.
When somebody can actually say what the huge advantages to MSI are that
it's worth using when
(a) several motherboards are apparently known broken
(b) microsoft apparently is of the same opinion and _also_ doesn't use it
(c) the old non-MSI code works fine
(d) there is apparently no fool-proof way to tell when it works and when
it doesn't.
then please holler. Btw, I'm not even _interested_ in any advantages
unless you also have a solution for (d). Not a "it should work". I want to
hear something that is _guaranteed_ to work.
Until that point, I will suggest that we always just disable MSI by
default, and people who want to use it need to enable it BY HAND.
F*ck whitelists. It's better and easier to just NOT USE IT! There are
basically zero advantages to using MSI anyway, until you get to big enough
systems that you have a system maintainer that can make the decision.
Linus
Linus Torvalds wrote:
>
> On Tue, 14 Nov 2006, Jeff Garzik wrote:
>> :( Like AHCI, PCI MSI has -always- worked wonderfully for HD audio AFAIK.
>
> That "AFAIK" is shorthand for "As Far As I haven't read any of the
> bug-reports but Know", right?
None of the bug reports indicate Intel, thus following the well
established pattern of "it works great on Intel, but not elsewhere"
>> Is a whitelist patch forthcoming?
>
> Probably not. The advantages of MSI aren't all that obvious, and the
> disadvantages seem to be that it just doesn't work all that well for some
> people.
>
> The fact that it works for MOST people has absolutely zero relevance.
> We've had too many frigging patches that have apparently been of the "this
> works for me, I don't care if some other motherboard has problems" kind.
>
> See for example:
>
> http://lkml.org/lkml/2006/10/7/164
>
> and yes, that HDA MSI _does_ seem to be causing problems.
But not on Intel, hence the obvious whitelist question.
> So don't blather about "MSI never causes problems". It's broken. Please
> stop living in denial.
>
> When somebody can actually say what the huge advantages to MSI are that
> it's worth using when
>
> (a) several motherboards are apparently known broken
several non-Intel motherboards
> (b) microsoft apparently is of the same opinion and _also_ doesn't use it
Yeah well, that's sage advice only when it's sage advice. MS lags us by
years. We do some bleeding, on the bleeding edge.
> (c) the old non-MSI code works fine
>
> (d) there is apparently no fool-proof way to tell when it works and when
> it doesn't.
>
> then please holler. Btw, I'm not even _interested_ in any advantages
> unless you also have a solution for (d). Not a "it should work". I want to
> hear something that is _guaranteed_ to work.
if (intel) ...
That has a track record of working.
It's nice not to have to deal with shared interrupts.
Jeff
On Tue, 14 Nov 2006, Jeff Garzik wrote:
>
> But not on Intel, hence the obvious whitelist question.
Hmm. Maybe. I'd be happier with a global "we can do MSI" flag, and making
it easier for people to enable it (rather than do this one driver at a
time). And yes, _if_ it's true that MSI works on all Intel SB/NB
combinations, then maybe we could enable it for those systems.
In the meantime, I'm really tired of continually hearing about MSI
problems, when there really aren't that many advantages.
> It's nice not to have to deal with shared interrupts.
I don't think "nice" is enough of an advantage to overcome "doesn't work
on God knows how many systems".
Linus
On Tue, 14 Nov 2006 21:40:33 -0500 Jeff Garzik wrote:
> Linus Torvalds wrote:
> >
> > On Tue, 14 Nov 2006, Jeff Garzik wrote:
> >> :( Like AHCI, PCI MSI has -always- worked wonderfully for HD audio AFAIK.
> >
> > That "AFAIK" is shorthand for "As Far As I haven't read any of the
> > bug-reports but Know", right?
>
> None of the bug reports indicate Intel, thus following the well
> established pattern of "it works great on Intel, but not elsewhere"
>
>
> >> Is a whitelist patch forthcoming?
> >
> > Probably not. The advantages of MSI aren't all that obvious, and the
> > disadvantages seem to be that it just doesn't work all that well for some
> > people.
> >
> > The fact that it works for MOST people has absolutely zero relevance.
> > We've had too many frigging patches that have apparently been of the "this
> > works for me, I don't care if some other motherboard has problems" kind.
> >
> > See for example:
> >
> > http://lkml.org/lkml/2006/10/7/164
> >
> > and yes, that HDA MSI _does_ seem to be causing problems.
>
> But not on Intel, hence the obvious whitelist question.
>
>
> > So don't blather about "MSI never causes problems". It's broken. Please
> > stop living in denial.
> >
> > When somebody can actually say what the huge advantages to MSI are that
> > it's worth using when
> >
> > (a) several motherboards are apparently known broken
>
> several non-Intel motherboards
>
>
> > (b) microsoft apparently is of the same opinion and _also_ doesn't use it
>
> Yeah well, that's sage advice only when it's sage advice. MS lags us by
> years. We do some bleeding, on the bleeding edge.
>
>
> > (c) the old non-MSI code works fine
> >
> > (d) there is apparently no fool-proof way to tell when it works and when
> > it doesn't.
> >
> > then please holler. Btw, I'm not even _interested_ in any advantages
> > unless you also have a solution for (d). Not a "it should work". I want to
> > hear something that is _guaranteed_ to work.
>
> if (intel) ...
>
> That has a track record of working.
>
> It's nice not to have to deal with shared interrupts.
FWIW, I concur with it always working for me on my Intel motherboard and
Intel-based Lenovo ThinkPad for hdaudio, ethernet, and SATA/AHCI.
---
~Randy
From: Linus Torvalds <[email protected]>
Date: Tue, 14 Nov 2006 18:49:43 -0800 (PST)
> In the meantime, I'm really tired of continually hearing about MSI
> problems, when there really aren't that many advantages.
>
> > It's nice not to have to deal with shared interrupts.
>
> I don't think "nice" is enough of an advantage to overcome "doesn't work
> on God knows how many systems".
>From what I see, that's not the advantage of MSI. The following
doesn't apply so well to sound, but it does apply significantly
to the networking.
The big advantage is that it eliminates the ordering issue in
interrupt handlers that use a DMA'd status block. Look at the Tigon3
driver for one good example.
If you take an INTX interrupt, it's over a pin on the motherboard and
thus can arrive before the DMA makes it to main memory, so you have to
have all of this fixup logic to handle that case and you don't even
know the interrupt is really for you so you have to actually check
the status block.
static irqreturn_t tg3_interrupt(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct tg3 *tp = netdev_priv(dev);
struct tg3_hw_status *sblk = tp->hw_status;
unsigned int handled = 1;
/* In INTx mode, it is possible for the interrupt to arrive at
* the CPU before the status block posted prior to the interrupt.
* Reading the PCI State register will confirm whether the
* interrupt is ours and will flush the status block.
*/
if ((sblk->status & SD_STATUS_UPDATED) ||
!(tr32(TG3PCI_PCISTATE) & PCISTATE_INT_NOT_ACTIVE)) {
/*
* Writing any value to intr-mbox-0 clears PCI INTA# and
* chip-internal interrupt pending events.
* Writing non-zero to intr-mbox-0 additional tells the
* NIC to stop sending us irqs, engaging "in-intr-handler"
* event coalescing.
*/
tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW,
0x00000001);
if (tg3_irq_sync(tp))
goto out;
sblk->status &= ~SD_STATUS_UPDATED;
if (likely(tg3_has_work(tp))) {
prefetch(&tp->rx_rcb[tp->rx_rcb_ptr]);
netif_rx_schedule(dev); /* schedule NAPI poll */
} else {
/* No work, shared interrupt perhaps? re-enable
* interrupts, and flush that PCI write
*/
tw32_mailbox_f(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW,
0x00000000);
}
} else { /* shared interrupt */
handled = 0;
}
out:
return IRQ_RETVAL(handled);
}
This whole initial if statement goes away with MSI, you don't even
need to check the status block, you know the interrupt is for you and
the device did generate it and has work pending.
/* MSI ISR - No need to check for interrupt sharing and no need to
* flush status block and interrupt mailbox. PCI ordering rules
* guarantee that MSI will arrive after the status block.
*/
static irqreturn_t tg3_msi(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct tg3 *tp = netdev_priv(dev);
prefetch(tp->hw_status);
prefetch(&tp->rx_rcb[tp->rx_rcb_ptr]);
/*
* Writing any value to intr-mbox-0 clears PCI INTA# and
* chip-internal interrupt pending events.
* Writing non-zero to intr-mbox-0 additional tells the
* NIC to stop sending us irqs, engaging "in-intr-handler"
* event coalescing.
*/
tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW, 0x00000001);
if (likely(!tg3_irq_sync(tp)))
netif_rx_schedule(dev); /* schedule NAPI poll */
return IRQ_RETVAL(1);
}
Taken to the next level MSI can be implemented in a "one-shot"
manner such that once the MSI is generated the chip shuts off
all further interrupts automatically.
This is perfect for networking devices since that is the first
thing we need to do in the interrupt handler for NAPI, and Tigon3
actually implements this.
/* One-shot MSI handler - Chip automatically disables interrupt
* after sending MSI so driver doesn't have to do it.
*/
static irqreturn_t tg3_msi_1shot(int irq, void *dev_id)
{
struct net_device *dev = dev_id;
struct tg3 *tp = netdev_priv(dev);
prefetch(tp->hw_status);
prefetch(&tp->rx_rcb[tp->rx_rcb_ptr]);
if (likely(!tg3_irq_sync(tp)))
netif_rx_schedule(dev); /* schedule NAPI poll */
return IRQ_HANDLED;
}
It saves an MMIO write in the interrupt handler and makes a huge
different in performance.
On Tue, 14 Nov 2006, David Miller wrote:
> >
> > I don't think "nice" is enough of an advantage to overcome "doesn't work
> > on God knows how many systems".
>
> From what I see, that's not the advantage of MSI. The following
> doesn't apply so well to sound, but it does apply significantly
> to the networking.
You're totally missing the issue.
Read that sentence of mine again.
Yours was still an example of "nice". And it had absolutely nothing to do
with the _PROBLEM_.
Linus
On Tuesday 14 November 2006 21:40, Jeff Garzik wrote:
> Linus Torvalds wrote:
> > On Tue, 14 Nov 2006, Jeff Garzik wrote:
> >> :( Like AHCI, PCI MSI has -always- worked wonderfully for HD audio
> >> : AFAIK.
> >
> > That "AFAIK" is shorthand for "As Far As I haven't read any of the
> > bug-reports but Know", right?
>
> None of the bug reports indicate Intel, thus following the well
> established pattern of "it works great on Intel, but not elsewhere"
Since when does INTEL make policy about Linux?
It'd be better to just follow Linus' suggestion and *NOT* use it.
> >> Is a whitelist patch forthcoming?
> >
> > Probably not. The advantages of MSI aren't all that obvious, and the
> > disadvantages seem to be that it just doesn't work all that well for some
> > people.
> >
> > The fact that it works for MOST people has absolutely zero relevance.
> > We've had too many frigging patches that have apparently been of the
> > "this works for me, I don't care if some other motherboard has problems"
> > kind.
> >
> > See for example:
> >
> > http://lkml.org/lkml/2006/10/7/164
> >
> > and yes, that HDA MSI _does_ seem to be causing problems.
>
> But not on Intel, hence the obvious whitelist question.
AFAICT that is the problem. You are fixated on the fact that MSI for the Intel
HD Audio works on INTEL motherboards. Pardon the language but - BIG FUCKING
SURPRISE THERE! The systems I'm running right now might be Intel chipset
boards, but that is simply a fluke. (They were given to me)
As a simple fact the only reason I see Linux users moving to systems with
Intel chipsets is that Intel is actually making a lot of its driver code open
source. Other than that... Well, let me just point out that not one of the
Linux users I personally know have a system with an Intel chipset.
> > So don't blather about "MSI never causes problems". It's broken. Please
> > stop living in denial.
> >
> > When somebody can actually say what the huge advantages to MSI are that
> > it's worth using when
> >
> > (a) several motherboards are apparently known broken
>
> several non-Intel motherboards
Again you focus on Intel. Give it up already. You've proved that you have no
real argument *FOR* having MSI in the driver.
> > (b) microsoft apparently is of the same opinion and _also_ doesn't use
> > it
>
> Yeah well, that's sage advice only when it's sage advice. MS lags us by
> years. We do some bleeding, on the bleeding edge.
and is light years ahead in some parts. Have you seen the "usermode driver"
system released as part of Vista and as a patch for XP? I, personally, cannot
see how it could work or be secure while doing so, but it does have an
advantage... Namely that a driver cannot bring the kernel down.
That Linux is ahead of Windows in a lot of cases is due more to the fact that
it is extremely easy to get new technology implemented and working in Linux
(and accepted into the kernel) than it is for the Windows. (Due more to the
*MASSIVE* size of the windows code base and the fact that its a closed
corporate project than anything)
But anyway...
> > (c) the old non-MSI code works fine
> >
> > (d) there is apparently no fool-proof way to tell when it works and when
> > it doesn't.
> >
> > then please holler. Btw, I'm not even _interested_ in any advantages
> > unless you also have a solution for (d). Not a "it should work". I want
> > to hear something that is _guaranteed_ to work.
>
> if (intel) ...
Again crowing about how something works on ONE (1) (Uno, Eins, Un, Ichi)
platform. A single working platform is no reason to complicate code with a
whitelist.
>
> That has a track record of working.
>
> It's nice not to have to deal with shared interrupts.
>
> Jeff
Yes, I'm sure it is. But until it starts working on more than one platform
it'd be stupid to add the code for a whitelist. Be easier to just *OFFER* MSI
to people as a build option. Then the people who *KNOW* it won't fubar their
system can activate it.
(BTW, as Linus pointed out, MSI isn't really needed until you get to a system
so large it has someone (or someones) there whose sole job is to make sure
that the best and fastest ways of doing things are used and are fully
functional)
DRH
From: Linus Torvalds <[email protected]>
Date: Tue, 14 Nov 2006 19:10:42 -0800 (PST)
> Yours was still an example of "nice". And it had absolutely nothing
> to do with the _PROBLEM_.
Understood.
BTW, some drivers have taken the approch to add MSI self-tests
inside of the driver to ensure correct option of MSI on a given
machine. There's a lot of resistence to that, the reasons for
which I grok fully, but I'm not sure other suggestions such as
black lists are any better.
Given current experience maybe white-lists are in fact the way
to go.
D. Hazelton wrote:
> Again crowing about how something works on ONE (1) (Uno, Eins, Un, Ichi)
> platform. A single working platform is no reason to complicate code with a
> whitelist.
A rather high volume platform with lots of users.
Jeff
On Tuesday 14 November 2006 22:30, Jeff Garzik wrote:
> D. Hazelton wrote:
> > Again crowing about how something works on ONE (1) (Uno, Eins, Un, Ichi)
> > platform. A single working platform is no reason to complicate code with
> > a whitelist.
>
> A rather high volume platform with lots of users.
>
> Jeff
Point is that it is still only one platform. Or don't you understand the point
I was making?
DRH
On Tue, 14 Nov 2006, David Miller wrote:
>
> > Yours was still an example of "nice". And it had absolutely nothing
> > to do with the _PROBLEM_.
>
> Understood.
Btw, before somebody thinks I hate MSI - I'd absolutely _love_ for it to
work, because I think MSI has the potential of getting rid of a lot of irq
routing problems.
And that's more than just a "nice" - we've obviously had issues with
machines not working right because we didn't get the magic PIRQ tables or
ACPI crud right.
So I'd actually be thrilled if we could use MSI more. (And here, "MSI"
should be considered to also include "MSI-X" etc - please, no language
lawyering).
> Given current experience maybe white-lists are in fact the way
> to go.
The thing that worries me is that I can well believe that the Intel NB/SB
combination gets MSI 100% correct, and I'd love to whitelist it.
HOWEVER - that's only true on systems with no other PCI bridges. Even if
you have an Intel NB/SB, what about other bridges in that same system, and
the devices behind them?
Now, I think that a MSI thing should look like a PCI write to a magic
address (I'm really not very up on it, so correct me if I'm wrong), and
thus maybe bridges are bound to get it right, and the only thing we really
need to worry about is the host bridge. Maybe. In that case, it might be
sensible to have a host-bridge white-table, and if we know all Intel
bridges that claim to support MSI do so correctly, then maybe we can just
say "ok, always enable it for Intel host bridges".
But right now I'm not convinced we really know what all goes wrong. Maybe
it's just broken NVidia and AMD bridges. But maybe it's also individual
devices that continue to (for example) raise _both_ the legacy IRQ line
_and_ send an MSI request.
Maybe even Intel host bridges have all the same troubles with that, and
the reason we haven't seen it is that _usually_ an Intel host bridge goes
together with certain Intel MSI-capable chips (ie e1000, Intel HDA etc).
So for all we know, it's not necessarily the Intel host bridge that is the
magic thing to make things work, it could be something that just has a
high _correlation_ with an Intel host bridge.
So call me a nervous nellie.
Linus
> If you take an INTX interrupt, it's over a pin on the motherboard and
> thus can arrive before the DMA makes it to main memory, so you have to
> have all of this fixup logic to handle that case and you don't even
> know the interrupt is really for you so you have to actually check
> the status block.
Out of curiosity. Are you sure there is no case of stupid bridge
converting the MSI into some APIC/whatever interrupt for the CPU
potentially before all previous DMA have been fully pushed to the
coherent domain (still in some internal store queue for example) ?
I suppose the Intel bridges get it right since they pretty much defined
them in the first place... but my experience with chipset designers is
that they have generally little regard for ordering issues (and the
impact those have on software) and no clue about anything related to
interrupt issues.
Ben.
> I don't think "nice" is enough of an advantage to overcome "doesn't work
> on God knows how many systems".
Note that the PCIe spec mandates MSIs while old style interrults (LSIs)
are optional... The result is that there are already platforms (though
not x86 just yet) being design with simply no support for LSIs on PCIe
and I've heard of devices doing that too (yeah, that's weird, they
wouldn't work in windows I suppose).
I suppose none of this affects x86 for now... For ppc, once we get our
new MSI layer in, however, I'll have to keep them enabled by default,
though there are fewer platforms involved and the chances at this point
are fairly high that they all use a working chipset.
Ben.
From: Benjamin Herrenschmidt <[email protected]>
Date: Wed, 15 Nov 2006 15:01:00 +1100
> Out of curiosity. Are you sure there is no case of stupid bridge
> converting the MSI into some APIC/whatever interrupt for the CPU
> potentially before all previous DMA have been fully pushed to the
> coherent domain (still in some internal store queue for example) ?
That would really suck, wouldn't it :)
However, they have to do all the work of processing the memory
transation that the MSI is on the PCI bus, I don't think they would go
so far out of their way to reorder things even if they converted the
MSI packet into a PIN to the APIC, for example.
Linus Torvalds wrote:
> HOWEVER - that's only true on systems with no other PCI bridges. Even if
> you have an Intel NB/SB, what about other bridges in that same system, and
> the devices behind them?
>
> Now, I think that a MSI thing should look like a PCI write to a magic
> address (I'm really not very up on it, so correct me if I'm wrong), and
> thus maybe bridges are bound to get it right, and the only thing we really
> need to worry about is the host bridge. Maybe. In that case, it might be
> sensible to have a host-bridge white-table, and if we know all Intel
> bridges that claim to support MSI do so correctly, then maybe we can just
> say "ok, always enable it for Intel host bridges".
That's pretty much the idea behind MSI... it looks like any other PCI
bus transaction, rather than needing a separate pin.
> But right now I'm not convinced we really know what all goes wrong. Maybe
> it's just broken NVidia and AMD bridges. But maybe it's also individual
> devices that continue to (for example) raise _both_ the legacy IRQ line
> _and_ send an MSI request.
That reminds me of a potential driver bug -- MSI-aware drivers need to
call pci_intx(pdev,0) to turn off the legacy PCI interrupt, before
enabling MSI interrupts.
So far, MSI history on x86 has always followed these rules:
* it works on Intel
* it doesn't work [well | at all] on AMD/NV
The only thing that has changed recently is that people are trying to
get it working on AMD/NV as well. (Brice Goglin's stuff starting at
6397c75cbc4d7dbc3d07278b57c82a47dafb21b5 in 'git log')
Seems to me this hda-intel driver patch is fallout from
pci_msi_enabled() not failing on enough systems (all AMD/NV?).
Anyway, if you want your master switch, put it there, not in each driver...
Jeff
> Now, I think that a MSI thing should look like a PCI write to a magic
> address (I'm really not very up on it, so correct me if I'm wrong), and
> thus maybe bridges are bound to get it right, and the only thing we really
> need to worry about is the host bridge. Maybe.
Yes, an MSI message really is just a PCI write cycle (which is why it
obeys all the PCI ordering as DaveM pointed out, and also can travel
on different PCIe VCs, etc). So it's hard to see how a bridge could
mess it up without screwing up lots of other stuff. The big thing is
the host bridge that has to turn the PCI write into an interrupt, and
that's where all the chipset problems that I know of are.
But on the other hand I don't think we have much experience with
MSI-capable devices below multiple levels of bridges, so you're right,
there is an unknown risk here.
> But right now I'm not convinced we really know what all goes wrong. Maybe
> it's just broken NVidia and AMD bridges. But maybe it's also individual
> devices that continue to (for example) raise _both_ the legacy IRQ line
> _and_ send an MSI request.
Well, since we now require drivers to "opt in" to MSI, I'm not
convinced that broken devices are a huge issue. We can take things
case-by-case and, obviously, if some random subset of devices are
broken, then clearly the driver should be really careful about
enabling MSI. But usually there's something like a device ID,
revision, FW version, etc. that we can test.
> together with certain Intel MSI-capable chips (ie e1000, Intel HDA etc).
Just for the record -- many e1000 chips that have an MSI capability in
their PCI config actually do not have working MSI. Hence the
if (adapter->hw.mac_type > e1000_82547_rev_2) {
adapter->have_msi = TRUE;
in the e1000 driver I guess.
BTW, at the end of the day I agree with the "MSI off by default"
policy, especially for the sound driver where the only realy gain is
saving IRQ routing hassles as you said. Even for the mthca InfiniBand
driver, where MSI-X with multiple vectors is a huge performance win, I
still have the user explicitly enable it.
- R.
Benjamin Herrenschmidt wrote:
> I suppose none of this affects x86 for now... For ppc, once we get our
> new MSI layer in, however, I'll have to keep them enabled by default,
> though there are fewer platforms involved and the chances at this point
> are fairly high that they all use a working chipset.
Ironically there are a shitload more MSI-capable devices out there, than
there are MSI-capable systems.
MSI simplifies things for the chip designers, so I don't doubt they are
chomping at the bit to start putting out MSI-only devices.
Jeff
From: Jeff Garzik <[email protected]>
Date: Tue, 14 Nov 2006 23:11:54 -0500
> Linus Torvalds wrote:
> > HOWEVER - that's only true on systems with no other PCI bridges. Even if
> > you have an Intel NB/SB, what about other bridges in that same system, and
> > the devices behind them?
> >
> > Now, I think that a MSI thing should look like a PCI write to a magic
> > address (I'm really not very up on it, so correct me if I'm wrong), and
> > thus maybe bridges are bound to get it right, and the only thing we really
> > need to worry about is the host bridge. Maybe. In that case, it might be
> > sensible to have a host-bridge white-table, and if we know all Intel
> > bridges that claim to support MSI do so correctly, then maybe we can just
> > say "ok, always enable it for Intel host bridges".
>
> That's pretty much the idea behind MSI... it looks like any other PCI
> bus transaction, rather than needing a separate pin.
Yep.
> > But right now I'm not convinced we really know what all goes wrong. Maybe
> > it's just broken NVidia and AMD bridges. But maybe it's also individual
> > devices that continue to (for example) raise _both_ the legacy IRQ line
> > _and_ send an MSI request.
>
> That reminds me of a potential driver bug -- MSI-aware drivers need to
> call pci_intx(pdev,0) to turn off the legacy PCI interrupt, before
> enabling MSI interrupts.
Is this absolutely true? I've never been sure about this point, and I
was rather convinced after reading various documents that once you
program up the MSI registers to start generating MSI this implicitly
disabled INTX and this was even in the PCI specification.
It would be great to get a definitive answer on this.
If it is mandatory, perhaps the driver shouldn't be doing it and
rather the PCI layer MSI enabling should.
David Miller wrote:
> Is this absolutely true? I've never been sure about this point, and I
> was rather convinced after reading various documents that once you
> program up the MSI registers to start generating MSI this implicitly
> disabled INTX and this was even in the PCI specification.
>
> It would be great to get a definitive answer on this.
>
> If it is mandatory, perhaps the driver shouldn't be doing it and
> rather the PCI layer MSI enabling should.
I can't answer for the spec, but at least two independent device vendors
recommended to write an MSI driver that way (disable intx, enable msi).
Completely independent of MSI though, a PCI 2.2 compliant driver should
be nice and disable intx on exit, just to avoid any potential interrupt
hassles after driver unload. And of course be aware that it might need
to enable intx upon entry.
Jeff
> Out of curiosity. Are you sure there is no case of stupid bridge
> converting the MSI into some APIC/whatever interrupt for the CPU
> potentially before all previous DMA have been fully pushed to the
> coherent domain (still in some internal store queue for example) ?
That's forbidden by the PCI spec:
"An MSI or MSI-X message, by virtue of being a posted memory write
(PMW) transaction, is prohibited by PCI ordering rules from
passing PMW transactions sent earlier by the function. The system
must guarantee that an interrupt service routine invoked as a
result of a given message will observe any updates performed by
PMW transactions arriving prior to that message."
which is not to say that there are no chipsets with errata in this
area. But I've never heard of such a thing...
- R.
> and I've heard of devices doing that too (yeah, that's weird, they
> wouldn't work in windows I suppose).
for example the QLogic PCIe InfiniBand adapter (drivers/infiniband/hw/ipath)
can't generate legacy INTx interrupts...
From: Jeff Garzik <[email protected]>
Date: Tue, 14 Nov 2006 23:24:04 -0500
> I can't answer for the spec, but at least two independent device vendors
> recommended to write an MSI driver that way (disable intx, enable msi).
Ok.
> Completely independent of MSI though, a PCI 2.2 compliant driver should
> be nice and disable intx on exit, just to avoid any potential interrupt
> hassles after driver unload. And of course be aware that it might need
> to enable intx upon entry.
This also sounds like it should occur in the generic PCI layer when a
PCI driver is unregistered.
> That reminds me of a potential driver bug -- MSI-aware drivers need to
> call pci_intx(pdev,0) to turn off the legacy PCI interrupt, before
> enabling MSI interrupts.
Huh? The device can't generate any legacy interrupts once MSI is
enabled. As the PCI spec says:
"While enabled for MSI or MSI-X operation, a function is prohibited
from using its INTx# pin (if implemented) to request service (MSI,
MSI-X, and INTx# are mutually exclusive)."
Although the MSI core does do pci_intx() for PCIe devices only, for
some reason I can't grok.
> The only thing that has changed recently is that people are trying to
> get it working on AMD/NV as well. (Brice Goglin's stuff starting at
> 6397c75cbc4d7dbc3d07278b57c82a47dafb21b5 in 'git log')
Actually NVidia/AMD was working on some systems long before that -- I
had it working at least 2 years ago.
- R.
Linus Torvalds <[email protected]> writes:
>
> But right now I'm not convinced we really know what all goes wrong. Maybe
> it's just broken NVidia and AMD bridges.
At least AMD (PCI-X) and Serverworks bridges are known broken with MSI
They should be both quirked though. Or rather one SVW bridge is quirked
there might be more. AMD 8131 is quirked. AMD 8132 is broken too but
should not have the capability structure in the first place.
BTW there seems to be a new ACPI FADT bit that says "MSI is broken"
We should probably check that too as a double check.
-Andi
Roland Dreier wrote:
> > That reminds me of a potential driver bug -- MSI-aware drivers need to
> > call pci_intx(pdev,0) to turn off the legacy PCI interrupt, before
> > enabling MSI interrupts.
>
> Huh? The device can't generate any legacy interrupts once MSI is
> enabled. As the PCI spec says:
>
> "While enabled for MSI or MSI-X operation, a function is prohibited
> from using its INTx# pin (if implemented) to request service (MSI,
> MSI-X, and INTx# are mutually exclusive)."
>
> Although the MSI core does do pci_intx() for PCIe devices only, for
> some reason I can't grok.
Probably the same reason I just mentioned in a reply to DaveM. Wildly
speculating, some chips may depend on software to disable INTX -- i.e.
depend on software to provide this aspect of PCI spec compliance.
> > The only thing that has changed recently is that people are trying to
> > get it working on AMD/NV as well. (Brice Goglin's stuff starting at
> > 6397c75cbc4d7dbc3d07278b57c82a47dafb21b5 in 'git log')
>
> Actually NVidia/AMD was working on some systems long before that -- I
> had it working at least 2 years ago.
Well, just noting that AMD/NV seems to be a common pattern in the
referenced bug reports, and the set of Linux platforms which claim to
support MSI had recent churn.
Jeff
> At least AMD (PCI-X) and Serverworks bridges are known broken with MSI
> They should be both quirked though. Or rather one SVW bridge is quirked
> there might be more. AMD 8131 is quirked. AMD 8132 is broken too but
> should not have the capability structure in the first place.
I thought people had AMD 8132 working? The only MSI erratum I see for
the 8132 is that a write to the MSI address with all byte-enables
deasserted is bad. But do any devices really do that? It seems like
a really odd thing to do.
- R.
On Wednesday 15 November 2006 05:57, Roland Dreier wrote:
> > At least AMD (PCI-X) and Serverworks bridges are known broken with MSI
> > They should be both quirked though. Or rather one SVW bridge is quirked
> > there might be more. AMD 8131 is quirked. AMD 8132 is broken too but
> > should not have the capability structure in the first place.
>
> I thought people had AMD 8132 working? The only MSI erratum I see for
> the 8132 is that a write to the MSI address with all byte-enables
> deasserted is bad. But do any devices really do that? It seems like
> a really odd thing to do.
Perhaps with special user space quirks, but not in mainline kernel because it's
not force enabled (see erratum #78).
-Andi
On Tue, 2006-11-14 at 20:07 -0800, David Miller wrote:
> From: Benjamin Herrenschmidt <[email protected]>
> Date: Wed, 15 Nov 2006 15:01:00 +1100
>
> > Out of curiosity. Are you sure there is no case of stupid bridge
> > converting the MSI into some APIC/whatever interrupt for the CPU
> > potentially before all previous DMA have been fully pushed to the
> > coherent domain (still in some internal store queue for example) ?
>
> That would really suck, wouldn't it :)
>
> However, they have to do all the work of processing the memory
> transation that the MSI is on the PCI bus, I don't think they would go
> so far out of their way to reorder things even if they converted the
> MSI packet into a PIN to the APIC, for example.
That would suck and not surprise me that much in fact... Take the Apple
bridge... it's unclear at which point they actually decode the MSI and
HT interrupts (the later are just internally converted to MSI-like
stores) to turn them into toggling of MPIC lines, but it probably
happens as an MMIO slave on the main xbar and I'm not 100% certain it
provides ordering vs. the previous stores to memory as they are in the
coherent domain and the MMIO is not. I just hope very much :-) Because
the store queue to memory can re-order on U4.
Ben.
> which is not to say that there are no chipsets with errata in this
> area. But I've never heard of such a thing...
Ok. Well, I've seen my load of PCI host bridges that were designed with
the PCI spec as toilet paper...
Ben.
>> However, they have to do all the work of processing the memory
>> transation that the MSI is on the PCI bus, I don't think they
>> would go
>> so far out of their way to reorder things even if they converted the
>> MSI packet into a PIN to the APIC, for example.
>
> That would suck and not surprise me that much in fact... Take the
> Apple
> bridge... it's unclear at which point they actually decode the MSI and
> HT interrupts (the later are just internally converted to MSI-like
> stores) to turn them into toggling of MPIC lines,
That's all documented just fine.
> but it probably
> happens as an MMIO slave on the main xbar
Yes indeed.
> and I'm not 100% certain it
> provides ordering vs. the previous stores to memory as they are in the
> coherent domain and the MMIO is not. I just hope very much :-)
All those DMA stores get reflected to the CPUs (the north
bridge itself doesn't keep a cache directory). All this
happens in-order. There probably _is_ a window where the
last DMA store isn't yet globally visible but the CPU
interrupt was already signaled, but the act of trying to
access that data orders it itself :-)
> Because
> the store queue to memory can re-order on U4.
That's only the store queue to the actual memory devices,
it's outside of the coherent domain anyway.
Segher
At Tue, 14 Nov 2006 19:21:17 -0800 (PST),
David Miller wrote:
>
> From: Linus Torvalds <[email protected]>
> Date: Tue, 14 Nov 2006 19:10:42 -0800 (PST)
>
> > Yours was still an example of "nice". And it had absolutely nothing
> > to do with the _PROBLEM_.
>
> Understood.
>
> BTW, some drivers have taken the approch to add MSI self-tests
> inside of the driver to ensure correct option of MSI on a given
> machine. There's a lot of resistence to that, the reasons for
> which I grok fully, but I'm not sure other suggestions such as
> black lists are any better.
The snd-hda-intel driver has a test of MSI, but it seems not working
on every machine. It caused non-cared interrupts and the kernel
disabled that irq.
> Given current experience maybe white-lists are in fact the way
> to go.
Could it be whitelisted in the PCI driver side? I don't think it's
good to have a huge white/blacklist in each device driver.
Takashi
> Point is that it is still only one platform. Or don't you understand the point
> I was making?
Given the Intel chipset platform appears to be over half the market it
makes sense to do things right there. It also encourages other vendors to
fix their stuff or send us patches to do so.
So I don't think anyone gives a damn about your personal issues with
Intel.
Alan
Jeff Garzik <[email protected]> writes:
> So far, MSI history on x86 has always followed these rules:
> * it works on Intel
> * it doesn't work [well | at all] on AMD/NV
I don't know how does it look when it doesn't work etc. but certainly
both NV Ethernet and HDA seem to work for me and:
$ cat /proc/interrupts
CPU0
0: 596146 IO-APIC-edge timer
1: 15425 IO-APIC-edge i8042
8: 103741 IO-APIC-edge rtc
9: 0 IO-APIC-fasteoi acpi
12: 25168 IO-APIC-edge i8042
14: 61 IO-APIC-edge libata
15: 0 IO-APIC-edge libata
20: 2 IO-APIC-fasteoi ehci_hcd:usb1
21: 0 IO-APIC-fasteoi libata
22: 4881 IO-APIC-fasteoi libata
23: 17010 IO-APIC-fasteoi libata, ohci_hcd:usb2
279: 598326 PCI-MSI-edge eth0
280: 21497 PCI-MSI-edge eth0
281: 20448 PCI-MSI-edge eth0
282: 4881 PCI-MSI-edge HDA Intel
NMI: 70
LOC: 596126
ERR: 0
$ /sbin/lspci|grep nV
00:00.0 RAM memory: nVidia Corporation MCP55 Memory Controller (rev a1)
00:01.0 ISA bridge: nVidia Corporation MCP55 LPC Bridge (rev a2)
00:01.1 SMBus: nVidia Corporation MCP55 SMBus (rev a2)
00:01.2 RAM memory: nVidia Corporation MCP55 Memory Controller (rev a2)
00:02.0 USB Controller: nVidia Corporation MCP55 USB Controller (rev a1)
00:02.1 USB Controller: nVidia Corporation MCP55 USB Controller (rev a2)
00:04.0 IDE interface: nVidia Corporation MCP55 IDE (rev a1)
00:05.0 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
00:05.1 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
00:05.2 IDE interface: nVidia Corporation MCP55 SATA Controller (rev a2)
00:06.0 PCI bridge: nVidia Corporation MCP55 PCI bridge (rev a2)
00:06.1 Audio device: nVidia Corporation MCP55 High Definition Audio (rev a2)
00:08.0 Bridge: nVidia Corporation MCP55 Ethernet (rev a2)
00:09.0 Bridge: nVidia Corporation MCP55 Ethernet (rev a2)
00:0b.0 PCI bridge: nVidia Corporation MCP55 PCI Express bridge (rev a2)
00:0c.0 PCI bridge: nVidia Corporation MCP55 PCI Express bridge (rev a2)
00:0d.0 PCI bridge: nVidia Corporation MCP55 PCI Express bridge (rev a2)
00:0e.0 PCI bridge: nVidia Corporation MCP55 PCI Express bridge (rev a2)
00:0f.0 PCI bridge: nVidia Corporation MCP55 PCI Express bridge (rev a2)
> Anyway, if you want your master switch, put it there, not in each driver...
Definitely.
--
Krzysztof Halasa
On Tue, 14 Nov 2006, Roland Dreier wrote:
>
> Huh? The device can't generate any legacy interrupts once MSI is
> enabled. As the PCI spec says:
PLEASE.
The spec is just so much toilet paper. The ONLY thing that matters is what
real hardware does. So please please please PLEASE don't start quoting
specs as a way to "prove your point". It is totally meaningless.
The fact is, we _know_ there are broken devices out there. Not just wrt
MSI - pretty much every single sentence of the PCI spec is probably broken
by _some_ device or subsystem. For example, I would expect that the DMA
ordering rule is likely broken by the big SGI boxes, because they do
memory coherency traffic separately from the IO traffic.
This should be the #1 rule for every kernel programmer: read the
documentation, but don't actually assume it's complete, or that it is
always true.
One of the big reasons user-level programming is so much simpler than
kernel programming is that you can basically "think" about your
algorithms. You seldom have cases where you literally just need to test
out whether all hardware really works that way.
The bottom line: MSI looks great on paper. I think we might make it a rule
that we can enable it by default on bridges we trust (which would seem to
be mainly just Intel). But even then, I think it makes sense for
per-driver rules, because
- some drivers may care more than others (sound, for example, certainly
doesn't really care at all), apart from irq routing issues, and right
now those are _worse_ with MSI due to bugs.
- we definitely have the potential for per-driver bugs (eg the "disable
INTx explicitly" kind of situation), so even if MSI works on a system
level, I think we all know that it doesn't always work on a device
level, and the safe thing tends to be to keep it disabled unless you
have some really overriding concern to use it.
End result: I think that at least for 2.6.19, and at least for the HDA
sound driver, keeping it disabled by default is the right choice. We
should probably _also_ make "pci_msi_supported()" just return an error
(probably by just clearing "pci_msi_enable") for any non-intel host bridge
for now.
I don't dispute AT ALL that we should try to enable things more in the
future. I'm just saying that we're better off being careful, and realizing
that documentation and "official rules" seldom match reality.
(People seem to trust documentation, because for _most_ devices, the
documentation is correct. But if it's wrong for 1% of all devices, you
should still realize that IT IS WRONG. You can't quote "the standard" to
prove a point - a single exception is enough to show that a standard is
incomplete)
Linus
On Wed, 15 Nov 2006, Takashi Iwai wrote:
>
> The snd-hda-intel driver has a test of MSI, but it seems not working
> on every machine. It caused non-cared interrupts and the kernel
> disabled that irq.
Yes.
Btw, this was why I was claiming that maybe some devices might raise
_both_ the MSI and the INTx interrupt, which can indeed cause problems
like that: because we see spurious interrupts on some other irq line (the
INTx one), we might decide to end up disabling that one, just because we
can't seem to shut it up.
However, the same kind of schenario may happen if the MSI irq from a
device simply doesn't _work_ - the device may claim MSI capabilities but
always uses INTx, and you'd get the same behaviour from just _testing_ the
MSI line - the irq comes in on the wrong vector, and since you're not
handling that vector, the kernel has no choice but to say "I will have to
disable this screaming irq".
So "testing" that an MSI works isn't actually goign to solve any real
problems. It may or may not show that the MSI works, but regardless of the
success of the test, it can have deadly consequences for _other_ devices
if the irq routing (which may be INTx) is broken.
This is why I'm so adamant that we need to _know_ that it works before we
use it. When irq's get mis-routed, things go downhill real fast. We're
usually talking "dead machines", and there is very little that a driver
can do about it.
Linus
> This is why I'm so adamant that we need to _know_ that it works before we
> use it. When irq's get mis-routed, things go downhill real fast. We're
> usually talking "dead machines", and there is very little that a driver
> can do about it.
well we could cheat some. And have the generic code for this just
register the irq handler for both somehow. At least the case for "only
does old style and no actual MSI" will work then.
For the duplicate issue... if the generic irq layer keeps track of the
"mirror" property of the handler it could be dealt with (eg clear the
unhandled count of the "mirror" as well on each handled irq)..
I'll grant you that it's a bit evil, and probably ulgy to keep the
mirror information, but if it gets the thing reliable..... it could
solve a lot of the mess.
(Oh and I would *love* for this to printk once if the kernel detects
cheating behavior so that we can make things like the linux firmware
test kit complain to the hw/bios folks early on)
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
On Wed, 15 Nov 2006, Arjan van de Ven wrote:
>
> well we could cheat some. And have the generic code for this just
> register the irq handler for both somehow.
Well, not generic code. It would have to be the driver itself that does
it, since generic code doesn't even know (at irq request time - and when
they are generated - it just gets the irq number).
And the thing is, once you do that, all the advantages of MSI totally go
away - both the "nice" ones and the "really good ones" (the latter being
the hopeful eventual removal of irq routing confusions). So if you do
that, the better solution is for the driver to say "I won't use MSI at
all".
Really.
It all boils down to the same thing: either we have to know that MSI works
(where "know" is obviously relative - it's not like you can avoid _all_
bugs, but dammit, even a single report of "not working" means that there
are probably a ton of machines like that, and we did something wrong), or
we shouldn't use it. There is no middle ground. You can't really safely
"test" for it, and while you _can_ say "just do both", it doesn't really
help anything (and potentially exposes you to just more bugs: if enablign
MSI actually _does_ disable INTx, but then doesn't work, at a minimum you
end up with a device that doesn't work, even if the rest of the kernel
might be ok).
And btw, I say this as a person whose new main machine used to have HDA
routed over MSI, and the decision to default to it off meant that it went
back to the regular INTx thing.
(Btw, MSI interrupts also seem to not participate in CPU balancing:
22: 41556 43005 IO-APIC-fasteoi HDA Intel
506: 110417 0 PCI-MSI-edge eth0
which is another semantic change introduced by using MSI)
Linus
Linus Torvalds wrote:
> The spec is just so much toilet paper. The ONLY thing that matters is what
> real hardware does. So please please please PLEASE don't start quoting
> specs as a way to "prove your point". It is totally meaningless.
Amen.
> End result: I think that at least for 2.6.19, and at least for the HDA
> sound driver, keeping it disabled by default is the right choice. We
> should probably _also_ make "pci_msi_supported()" just return an error
> (probably by just clearing "pci_msi_enable") for any non-intel host bridge
> for now.
If you update, pci_msi_{enable,supported} then you can -- and should --
revert the HD-audio driver change. Just reviewed the driver, and it
properly checks all the return values from PCI MSI API functions.
(though, HD-audio shouldn't be using IRQF_DISABLED at all, and shouldn't
be using IRQF_SHARED for PCI MSI interrupts)
Though maybe for 2.6.19 the current state of things is at least a stable
state.
Jeff
Takashi Iwai wrote:
> The snd-hda-intel driver has a test of MSI, but it seems not working
> on every machine. It caused non-cared interrupts and the kernel
> disabled that irq.
Possibly the test was broken. Did you have IRQF_DISABLED and
IRQF_SHARED flags set?
>> Given current experience maybe white-lists are in fact the way
>> to go.
>
> Could it be whitelisted in the PCI driver side? I don't think it's
> good to have a huge white/blacklist in each device driver.
Certainly, we should not have such lists in the driver. I don't think
anybody wants that.
Jeff
Jeff Garzik wrote:
> Takashi Iwai wrote:
>> The snd-hda-intel driver has a test of MSI, but it seems not working
>> on every machine. It caused non-cared interrupts and the kernel
>> disabled that irq.
>
> Possibly the test was broken. Did you have IRQF_DISABLED and
> IRQF_SHARED flags set?
And also call pci_intx() properly in the driver, which I noticed isn't
being done.
(PCI MSI layer does it automatically for PCI-Express devices, only)
Jeff
Linus Torvalds wrote:
> And the thing is, once you do that, all the advantages of MSI totally go
> away - both the "nice" ones and the "really good ones" (the latter being
> the hopeful eventual removal of irq routing confusions). So if you do
> that, the better solution is for the driver to say "I won't use MSI at
> all".
I certainly agree with this.
The driver should either (a) know MSI works for the device [it calls
pci_msi_enable()] or (b) never bothers with MSI.
Combine that with accurate pci_msi_enable() failure, and life is good.
No 'test if MSI works' crapola in drivers. I've disliked such tests
since the day MSI was introduced into Linux.
> It all boils down to the same thing: either we have to know that MSI works
> (where "know" is obviously relative - it's not like you can avoid _all_
> bugs, but dammit, even a single report of "not working" means that there
> are probably a ton of machines like that, and we did something wrong), or
> we shouldn't use it. There is no middle ground. You can't really safely
> "test" for it, and while you _can_ say "just do both", it doesn't really
> help anything (and potentially exposes you to just more bugs: if enablign
> MSI actually _does_ disable INTx, but then doesn't work, at a minimum you
> end up with a device that doesn't work, even if the rest of the kernel
> might be ok).
Agreed.
> And btw, I say this as a person whose new main machine used to have HDA
> routed over MSI, and the decision to default to it off meant that it went
> back to the regular INTx thing.
Yeah, but you don't care if that happens, so this is an ineffective
'btw' It's not like you went to sleep crying over the loss, like I did
[just kidding!]
> (Btw, MSI interrupts also seem to not participate in CPU balancing:
>
> 22: 41556 43005 IO-APIC-fasteoi HDA Intel
> 506: 110417 0 PCI-MSI-edge eth0
>
> which is another semantic change introduced by using MSI)
No, that's most likely because ethernet is always intentionally locked
to a single CPU by irqbalance.
Jeff
On 15 Nov 2006 05:49:41 +0100 Andi Kleen wrote:
> BTW there seems to be a new ACPI FADT bit that says "MSI is broken"
> We should probably check that too as a double check.
Do you mean more than just use that bit when you say "double check"?
I wonder why this hasn't already been done. (?)
How's this look? Build-tested only.
---
From: Randy Dunlap <[email protected]>
Implement use of FADT boot_flags.MSI_NOT_SUPPORTED bit to prevent
assigning MSIs.
Force MSI_NOT_SUPPORTED for ACPI 1.0 tables.
Signed-off-by: Randy Dunlap <[email protected]>
---
drivers/acpi/bus.c | 6 ++++++
drivers/acpi/tables/tbconvrt.c | 3 ++-
include/acpi/actbl.h | 4 +++-
include/linux/pci.h | 2 ++
4 files changed, 13 insertions(+), 2 deletions(-)
--- linux-2619-rc5.orig/include/acpi/actbl.h
+++ linux-2619-rc5/include/acpi/actbl.h
@@ -290,7 +290,7 @@ struct fadt_descriptor_rev1 {
ACPI_FADT_COMMON u32 flags;
};
-/* FADT: Prefered Power Management Profiles */
+/* FADT: Preferred Power Management Profiles */
#define PM_UNSPECIFIED 0
#define PM_DESKTOP 1
@@ -304,6 +304,8 @@ struct fadt_descriptor_rev1 {
#define BAF_LEGACY_DEVICES 0x0001
#define BAF_8042_KEYBOARD_CONTROLLER 0x0002
+#define BAF_VGA_NOT_PRESENT 0x0004
+#define BAF_MSI_NOT_SUPPORTED 0x0008
#define FADT2_REVISION_ID 3
#define FADT2_MINUS_REVISION_ID 2
--- linux-2619-rc5.orig/drivers/acpi/tables/tbconvrt.c
+++ linux-2619-rc5/drivers/acpi/tables/tbconvrt.c
@@ -289,7 +289,8 @@ acpi_tb_convert_fadt1(struct fadt_descri
* Since there isn't any equivalence in 1.0 and since it is highly
* likely that a 1.0 system has legacy support.
*/
- local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES;
+ local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES |
+ BAF_MSI_NOT_SUPPORTED;
}
/*
--- linux-2619-rc5.orig/include/linux/pci.h
+++ linux-2619-rc5/include/linux/pci.h
@@ -610,6 +610,7 @@ static inline int pci_enable_msix(struct
struct msix_entry *entries, int nvec) {return -1;}
static inline void pci_disable_msix(struct pci_dev *dev) {}
static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev) {}
+static inline void pci_no_msi(void) {}
#else
extern void pci_scan_msi_device(struct pci_dev *dev);
extern int pci_enable_msi(struct pci_dev *dev);
@@ -618,6 +619,7 @@ extern int pci_enable_msix(struct pci_de
struct msix_entry *entries, int nvec);
extern void pci_disable_msix(struct pci_dev *dev);
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
+extern void pci_no_msi(void);
#endif
#ifdef CONFIG_HT_IRQ
--- linux-2619-rc5.orig/drivers/acpi/bus.c
+++ linux-2619-rc5/drivers/acpi/bus.c
@@ -28,6 +28,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/sched.h>
+#include <linux/pci.h>
#include <linux/pm.h>
#include <linux/pm_legacy.h>
#include <linux/device.h>
@@ -637,6 +638,11 @@ void __init acpi_early_init(void)
}
#endif
+ if (acpi_fadt.iapc_boot_arch & BAF_MSI_NOT_SUPPORTED) {
+ printk(KERN_DEBUG PREFIX "MSI not supported, disabling it\n");
+ pci_no_msi();
+ }
+
status =
acpi_enable_subsystem(~
(ACPI_NO_HARDWARE_INIT |
Krzysztof Halasa wrote:
> Jeff Garzik <[email protected]> writes:
>
>> So far, MSI history on x86 has always followed these rules:
>> * it works on Intel
>> * it doesn't work [well | at all] on AMD/NV
>
> I don't know how does it look when it doesn't work etc. but certainly
> both NV Ethernet and HDA seem to work for me and:
Oh I certainly agree (and it appears that Roland agrees) that MSI works
/somewhere/ on NV. I give kudos to NV to working on forcedeth to make
sure it works well with MSI. But unfortunately NV was also in the bug
report(s) linked.
Though OTOH, the driver wasn't calling pci_intx() nor setting irq flags
correctly, so who knows.
Jeff
> > Huh? The device can't generate any legacy interrupts once MSI is
> > enabled. As the PCI spec says:
>
> PLEASE.
>
> The spec is just so much toilet paper. The ONLY thing that matters is what
> real hardware does. So please please please PLEASE don't start quoting
> specs as a way to "prove your point". It is totally meaningless.
Sorry, I think you misunderstood. I've certainly seen my share of
crazy devices doing things like endian-reversing the MSI address and
sending interrupt messages off into random space. The only thing I
was disagreeing with was that Jeff suggested it was a driver bug not
to do pci_intx(0) when enabling MSI.
I don't doubt that there are broken devices out there that do generate
wire interrupts even when MSI is enabled. I just don't think we
should go around "fixing" working, correct drivers for devices that
_do_ follow the spec.
- R.
On Wednesday 15 November 2006 19:35, Randy Dunlap wrote:
> On 15 Nov 2006 05:49:41 +0100 Andi Kleen wrote:
>
> > BTW there seems to be a new ACPI FADT bit that says "MSI is broken"
> > We should probably check that too as a double check.
>
> Do you mean more than just use that bit when you say "double check"?
>
> I wonder why this hasn't already been done. (?)
Nobody coded it yet.
> How's this look? Build-tested only.
There should be probably a command line option to overwrite it
-Andi
On Wed, 15 Nov 2006, Jeff Garzik wrote:
>
> > And btw, I say this as a person whose new main machine used to have HDA
> > routed over MSI, and the decision to default to it off meant that it went
> > back to the regular INTx thing.
>
> Yeah, but you don't care if that happens, so this is an ineffective 'btw'
> It's not like you went to sleep crying over the loss, like I did [just
> kidding!]
Heh. I'm really hoping that nobody will cry themselves to sleep over MSI
not being enabled..
> > (Btw, MSI interrupts also seem to not participate in CPU balancing:
> >
> > 22: 41556 43005 IO-APIC-fasteoi HDA Intel
> > 506: 110417 0 PCI-MSI-edge eth0
> >
> > which is another semantic change introduced by using MSI)
>
> No, that's most likely because ethernet is always intentionally locked to a
> single CPU by irqbalance.
Nope, the same thing happened to "HDA Intel" when it was an MSI interrupt
(ie before I applied the change that made it not use MSI by default).
So I think it's either: (a) irqbalance doesn't balance MSI interrupts at
all or (b) the MSI interrupt code doesn't honor balancing requests even if
it does.
I didn't look any closer. It's not like it's a huge problem for me (or
likely for anybody else), but it was interesting to see another
"unintended consequence" of the "use MSI or not" choice.
The suspend problem reported by Stephen is another such thing - where MSI
itself wasn't a problem, but stupid (probably broken) firmware code at
wakeup broke it by an unforseen interaction. Again, that is probably
related to the fact that nobody has ever really tested it (ie firmware
"engineers" obviously didn't actually ever test anything with MSI enabled
and in use, and there really is no excuse for firmware messing with the
MSI setting - other than the usual "firmware is inevitably buggy" thing).
Linus
At Wed, 15 Nov 2006 13:32:02 -0500,
Jeff Garzik wrote:
>
> Takashi Iwai wrote:
> > The snd-hda-intel driver has a test of MSI, but it seems not working
> > on every machine. It caused non-cared interrupts and the kernel
> > disabled that irq.
>
> Possibly the test was broken. Did you have IRQF_DISABLED and
> IRQF_SHARED flags set?
I think IRQF_* is irrelevant there.
It looks like that the hardware issues INTX regardless whether MSI is
enabled or not. Thus it ends up with unexpected irqs which are never
caught by the driver (the driver expects a different irq from MSI),
and eventually the kernel kills this irq.
Possibly calling pci_intx() as you suggested might help to avoid this
situation. Can anyone test the patch below?
Takashi
---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e35cfd3..bdb92b3 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -544,6 +544,7 @@ static unsigned int azx_rirb_get_respons
free_irq(chip->irq, chip);
chip->irq = -1;
pci_disable_msi(chip->pci);
+ pci_intx(chip->pci, 1);
chip->msi = 0;
if (azx_acquire_irq(chip, 1) < 0)
return -1;
@@ -830,14 +831,15 @@ static irqreturn_t azx_interrupt(int irq
{
struct azx *chip = dev_id;
struct azx_dev *azx_dev;
+ unsigned long flags;
u32 status;
int i;
- spin_lock(&chip->reg_lock);
+ spin_lock_irqsave(&chip->reg_lock, flags);
status = azx_readl(chip, INTSTS);
if (status == 0) {
- spin_unlock(&chip->reg_lock);
+ spin_unlock_irqrestore(&chip->reg_lock, flags);
return IRQ_NONE;
}
@@ -867,7 +869,7 @@ static irqreturn_t azx_interrupt(int irq
if (azx_readb(chip, STATESTS) & 0x04)
azx_writeb(chip, STATESTS, 0x04);
#endif
- spin_unlock(&chip->reg_lock);
+ spin_unlock_irqrestore(&chip->reg_lock, flags);
return IRQ_HANDLED;
}
@@ -1380,7 +1382,8 @@ static int __devinit azx_init_stream(str
static int azx_acquire_irq(struct azx *chip, int do_disconnect)
{
- if (request_irq(chip->pci->irq, azx_interrupt, IRQF_DISABLED|IRQF_SHARED,
+ if (request_irq(chip->pci->irq, azx_interrupt,
+ chip->msi ? 0 : IRQF_SHARED,
"HDA Intel", chip)) {
printk(KERN_ERR "hda-intel: unable to grab IRQ %d, "
"disabling device\n", chip->pci->irq);
@@ -1435,9 +1438,12 @@ static int azx_resume(struct pci_dev *pc
return -EIO;
}
pci_set_master(pci);
- if (chip->msi)
+ if (chip->msi) {
if (pci_enable_msi(pci) < 0)
chip->msi = 0;
+ else
+ pci_intx(pci, 0);
+ }
if (azx_acquire_irq(chip, 1) < 0)
return -EIO;
azx_init_chip(chip);
@@ -1561,9 +1567,12 @@ static int __devinit azx_create(struct s
goto errout;
}
- if (chip->msi)
+ if (chip->msi) {
if (pci_enable_msi(pci) < 0)
chip->msi = 0;
+ else
+ pci_intx(pci, 0);
+ }
if (azx_acquire_irq(chip, 0) < 0) {
err = -EBUSY;
On Wed, 2006-11-15 at 10:51 -0800, Linus Torvalds wrote:
> So I think it's either: (a) irqbalance doesn't balance MSI interrupts at
> all or (b) the MSI interrupt code doesn't honor balancing requests even if
> it does.
it's (A) right now for sure for irqbalanced at least.
> The suspend problem reported by Stephen is another such thing - where MSI
> itself wasn't a problem, but stupid (probably broken) firmware code at
> wakeup broke it by an unforseen interaction. Again, that is probably
> related to the fact that nobody has ever really tested it (ie firmware
> "engineers" obviously didn't actually ever test anything with MSI enabled
> and in use, and there really is no excuse for firmware messing with the
> MSI setting - other than the usual "firmware is inevitably buggy" thing).
ok so maybe this should be in the linux firmware kit.. it has
suspend/resume tests after all ;)
if there is a "do this then this to reproduce" scenario it's even likely
it's trivial to put into a testcase...
>
> Linus
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
On Wed, 15 Nov 2006, Jeff Garzik wrote:
>
> Though OTOH, the driver wasn't calling pci_intx() nor setting irq flags
> correctly, so who knows.
The thing is, if the HDA driver happened to be alone on its legacy INTx
vector, and it is now empty, then the kernel won't ever care about the
fact that INTx may be still being generated. The legacy vector won't be
enabled unless somebody else is on that vector.
So yes, this NV HDA sound breakage could easily be because of the
interrupt being sent both over legacy INTx _and_ MSI. It would only break
for people who happened to have another device sharing the INTx pin (like
the report that caused us to disable it by default).
However, I really think that this should be a generic PCI layer thing. If
some device asks for MSI interrupts, the PCI layer should try to turn off
a INTx routing on its own. Asking drivers to do both is just silly,
especially since driver writers really shouldn't be expected to know about
all these issues (sure, the best ones do, but a lot of driver writers will
just say "it works for me").
So I don't think the HDA driver should need disable INTx on its own
explicitly.
If somebody were to write up such a patch and send it to Olivier (and
others - I think there was another report of this) for testing (he'd now
need to ask for msi irqs explicitly), that might be interesting. Hint,
hint.
(See the original http://lkml.org/lkml/2006/11/8/98 report for one broken
case, although it does seem different: we literally have a
hda_intel: No response from codec, disabling MSI...
indicating that the MSI case simply didn't work at _all_ rather than
duplicating interrupts).
Linus
Takashi Iwai wrote:
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e35cfd3..bdb92b3 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -544,6 +544,7 @@ static unsigned int azx_rirb_get_respons
> free_irq(chip->irq, chip);
> chip->irq = -1;
> pci_disable_msi(chip->pci);
> + pci_intx(chip->pci, 1);
> chip->msi = 0;
> if (azx_acquire_irq(chip, 1) < 0)
> return -1;
> @@ -830,14 +831,15 @@ static irqreturn_t azx_interrupt(int irq
> {
> struct azx *chip = dev_id;
> struct azx_dev *azx_dev;
> + unsigned long flags;
> u32 status;
> int i;
>
> - spin_lock(&chip->reg_lock);
> + spin_lock_irqsave(&chip->reg_lock, flags);
>
> status = azx_readl(chip, INTSTS);
> if (status == 0) {
> - spin_unlock(&chip->reg_lock);
> + spin_unlock_irqrestore(&chip->reg_lock, flags);
> return IRQ_NONE;
> }
>
> @@ -867,7 +869,7 @@ static irqreturn_t azx_interrupt(int irq
> if (azx_readb(chip, STATESTS) & 0x04)
> azx_writeb(chip, STATESTS, 0x04);
> #endif
> - spin_unlock(&chip->reg_lock);
> + spin_unlock_irqrestore(&chip->reg_lock, flags);
ACK the pci_intx() calls, NAK the obviously overweight spinlock changes.
The spinlock changes are completely unnecessary. Just look at any
other (non-ALSA) PCI driver. Existing "spin_lock()" is fine for both
PCI shared irq handlers and MSI irq handlers.
It sounds like you are trying to work around a reentrancy problem that
does not exist.
Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(),
where separate interrupt sources call the same function.
Jeff
Linus Torvalds wrote:
> However, I really think that this should be a generic PCI layer thing. If
> some device asks for MSI interrupts, the PCI layer should try to turn off
> a INTx routing on its own. Asking drivers to do both is just silly,
> especially since driver writers really shouldn't be expected to know about
> all these issues (sure, the best ones do, but a lot of driver writers will
> just say "it works for me").
>
> So I don't think the HDA driver should need disable INTx on its own
> explicitly.
Your thinking is correct, but there is one hitch.
As Roland noted, PCI layer /already/ does this for PCI-Express devices.
The reason we cannot do this in the generic layer for non-PCI-Ex is only
the driver knows whether that PCI 2.2 bit was actually implemented in
the device or mapped to some other weird behavior we don't want to
touch. DISABLE-INTX is a new bit not present in PCI 2.1 (alas!!).
pci_intx() was my five minute solution to this problem, and it got moved
outside of libata as soon as somebody needed the same thing :)
Maybe a better solution is letting the driver say "pci_dev->intx_ok = 1"
right before it calls pci_enable_device().
And if we do this, we can follow through on another suggestion I made:
disabling INTx on driver exit, to help eliminate any possibility of
screaming interrupts after driver unload.
Jeff
Linus Torvalds wrote:
>On Wed, 15 Nov 2006, Arjan van de Ven wrote:
>
>
>>well we could cheat some. And have the generic code for this just
>>register the irq handler for both somehow.
>>
>>
>
>Well, not generic code. It would have to be the driver itself that does
>it, since generic code doesn't even know (at irq request time - and when
>they are generated - it just gets the irq number).
>
>And the thing is, once you do that, all the advantages of MSI totally go
>away - both the "nice" ones and the "really good ones" (the latter being
>the hopeful eventual removal of irq routing confusions). So if you do
>that, the better solution is for the driver to say "I won't use MSI at
>all".
>
>Really.
>
>It all boils down to the same thing: either we have to know that MSI works
>(where "know" is obviously relative - it's not like you can avoid _all_
>bugs, but dammit, even a single report of "not working" means that there
>are probably a ton of machines like that, and we did something wrong), or
>we shouldn't use it. There is no middle ground. You can't really safely
>"test" for it, and while you _can_ say "just do both", it doesn't really
>help anything (and potentially exposes you to just more bugs: if enablign
>MSI actually _does_ disable INTx, but then doesn't work, at a minimum you
>end up with a device that doesn't work, even if the rest of the kernel
>might be ok).
>
>And btw, I say this as a person whose new main machine used to have HDA
>routed over MSI, and the decision to default to it off meant that it went
>back to the regular INTx thing.
>
>(Btw, MSI interrupts also seem to not participate in CPU balancing:
>
> 22: 41556 43005 IO-APIC-fasteoi HDA Intel
>506: 110417 0 PCI-MSI-edge eth0
>
>
>
mine do:$ cat /proc/interrupts
CPU0 CPU1
0: 7986702 7971263 IO-APIC-edge timer
...
20: 90626 95073 IO-APIC-fasteoi uhci_hcd:usb2
220: 1722 1415 PCI-MSI-edge HDA Intel
NMI: 0 0
LOC: 15957069 15957071
ERR: 0
MIS: 0
Also, I find it disturbing that we are forcing users to have know about
all these
magic options that have to be put on the kernel boot line. My hard drive
on my
new laptop would only run at 1.2mbs until I found out I had to use
combined_mode=libata
and build a new ramdisk that included ata_piix.
My $.02
Steve
>which is another semantic change introduced by using MSI)
>
> Linus
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
--
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety." (Ben Franklin)
"The course of history shows that as a government grows, liberty
decreases." (Thomas Jefferson)
On Wed, 15 Nov 2006, Jeff Garzik wrote:
>
> The reason we cannot do this in the generic layer for non-PCI-Ex is only the
> driver knows whether that PCI 2.2 bit was actually implemented in the device
> or mapped to some other weird behavior we don't want to touch. DISABLE-INTX
> is a new bit not present in PCI 2.1 (alas!!).
Ok, I would have a few suggestions, then:
- devices that don't support INTx disable should basically be considered
to not support MSI at all (ie a driver simply shouldn't even try to
enable MSI, since enabling MSI includes the implicit DISABLE-INTX)
This is likely ok, since MSI wasn't in PCI 2.1 _either_ afaik. So if
your hardware has MSI, it probably _does_ have DISABLE-INTX (or at
least that bit doesn't do anything bad, which is the other case)
- add a flag to "pci_enable_msi()". There really aren't that many users,
and they basically _all_ want this functionality. Making them call
another function is just a recipe for disaster, since somebody will
forget. Having to just say "do I support INTx disable" is much better,
since it makes the driver writer aware of having to _explicitly_ make
that choice.
> Maybe a better solution is letting the driver say "pci_dev->intx_ok = 1" right
> before it calls pci_enable_device().
I hate that, for exactly the same reason I hate "pci_intx()". It just
means that most drivers won't do it, because it's not even part of the
normal sequence, and most people don't care. So again, it would actually
be better in that case to just add a "flags" field to pci_enable_device(),
although that's a _hell_ of a lot more painful than it would be to do the
same to "pci_enable_msi()".
> And if we do this, we can follow through on another suggestion I made:
> disabling INTx on driver exit, to help eliminate any possibility of screaming
> interrupts after driver unload.
The thing is, I think that's a bad idea for the same reason it's a bad
idea to disable the BAR's (which we tried and then reverted). It's just
going to cause problems for soft rebooting etc with firmware that doesn't
expect it.
A driver should obviously quiesce the device on shutdown, and if it leaves
a device in a state where it may still generate interrupts, that's a
_bug_, so disabling INTx is just papering over a much more serious issue.
Linus
On Wed, 15 Nov 2006 19:44:12 +0100 Andi Kleen wrote:
> On Wednesday 15 November 2006 19:35, Randy Dunlap wrote:
> > On 15 Nov 2006 05:49:41 +0100 Andi Kleen wrote:
> >
> > > BTW there seems to be a new ACPI FADT bit that says "MSI is broken"
> > > We should probably check that too as a double check.
> >
> > Do you mean more than just use that bit when you say "double check"?
> >
> > I wonder why this hasn't already been done. (?)
>
> Nobody coded it yet.
>
> > How's this look? Build-tested only.
>
> There should be probably a command line option to overwrite it
OK, anything else?
---
From: Randy Dunlap <[email protected]>
- Implement use of FADT boot_flags.MSI_NOT_SUPPORTED bit to prevent
assigning MSIs.
- Force MSI_NOT_SUPPORTED for ACPI 1.0 tables.
- Add "pci=allowmsi" to override (ignore) the MSI_NOT_SUPPORTED bit
from the APCI FADT.
Signed-off-by: Randy Dunlap <[email protected]>
---
Documentation/kernel-parameters.txt | 6 ++++++
drivers/acpi/bus.c | 7 +++++++
drivers/acpi/tables/tbconvrt.c | 3 ++-
drivers/pci/pci.c | 4 ++++
include/acpi/actbl.h | 4 +++-
include/linux/pci.h | 3 +++
6 files changed, 25 insertions(+), 2 deletions(-)
--- linux-2619-rc5.orig/include/acpi/actbl.h
+++ linux-2619-rc5/include/acpi/actbl.h
@@ -290,7 +290,7 @@ struct fadt_descriptor_rev1 {
ACPI_FADT_COMMON u32 flags;
};
-/* FADT: Prefered Power Management Profiles */
+/* FADT: Preferred Power Management Profiles */
#define PM_UNSPECIFIED 0
#define PM_DESKTOP 1
@@ -304,6 +304,8 @@ struct fadt_descriptor_rev1 {
#define BAF_LEGACY_DEVICES 0x0001
#define BAF_8042_KEYBOARD_CONTROLLER 0x0002
+#define BAF_VGA_NOT_PRESENT 0x0004
+#define BAF_MSI_NOT_SUPPORTED 0x0008
#define FADT2_REVISION_ID 3
#define FADT2_MINUS_REVISION_ID 2
--- linux-2619-rc5.orig/drivers/acpi/tables/tbconvrt.c
+++ linux-2619-rc5/drivers/acpi/tables/tbconvrt.c
@@ -289,7 +289,8 @@ acpi_tb_convert_fadt1(struct fadt_descri
* Since there isn't any equivalence in 1.0 and since it is highly
* likely that a 1.0 system has legacy support.
*/
- local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES;
+ local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES |
+ BAF_MSI_NOT_SUPPORTED;
}
/*
--- linux-2619-rc5.orig/include/linux/pci.h
+++ linux-2619-rc5/include/linux/pci.h
@@ -610,6 +610,7 @@ static inline int pci_enable_msix(struct
struct msix_entry *entries, int nvec) {return -1;}
static inline void pci_disable_msix(struct pci_dev *dev) {}
static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev) {}
+static inline void pci_no_msi(void) {}
#else
extern void pci_scan_msi_device(struct pci_dev *dev);
extern int pci_enable_msi(struct pci_dev *dev);
@@ -618,6 +619,8 @@ extern int pci_enable_msix(struct pci_de
struct msix_entry *entries, int nvec);
extern void pci_disable_msix(struct pci_dev *dev);
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
+extern void pci_no_msi(void);
+extern int pci_allow_msi;
#endif
#ifdef CONFIG_HT_IRQ
--- linux-2619-rc5.orig/drivers/acpi/bus.c
+++ linux-2619-rc5/drivers/acpi/bus.c
@@ -28,6 +28,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/sched.h>
+#include <linux/pci.h>
#include <linux/pm.h>
#include <linux/pm_legacy.h>
#include <linux/device.h>
@@ -637,6 +638,12 @@ void __init acpi_early_init(void)
}
#endif
+ if (!pci_allow_msi &&
+ (acpi_fadt.iapc_boot_arch & BAF_MSI_NOT_SUPPORTED)) {
+ printk(KERN_DEBUG PREFIX "MSI not supported, disabling it\n");
+ pci_no_msi();
+ }
+
status =
acpi_enable_subsystem(~
(ACPI_NO_HARDWARE_INIT |
--- linux-2619-rc5.orig/Documentation/kernel-parameters.txt
+++ linux-2619-rc5/Documentation/kernel-parameters.txt
@@ -1178,6 +1178,12 @@ and is between 256 and 4096 characters.
nomsi [MSI] If the PCI_MSI kernel config parameter is
enabled, this kernel boot option can be used to
disable the use of MSI interrupts system-wide.
+ allowmsi [MSI,ACPI] If ACPI FADT says that MSI is not
+ supported on this platform but the user wants
+ to use it anyway, this option tells the kernel
+ to ignore the ACPI FADT MSI flag and allow MSI
+ to be used if there are devices that want to
+ use it.
nosort [IA-32] Don't sort PCI devices according to
order given by the PCI BIOS. This sorting is
done to get a device order compatible with
--- linux-2619-rc5.orig/drivers/pci/pci.c
+++ linux-2619-rc5/drivers/pci/pci.c
@@ -20,6 +20,8 @@
#include "pci.h"
unsigned int pci_pm_d3_delay = 10;
+int pci_allow_msi; /* when set, this overrides the ACPI FADT boot_arch
+ * MSI_NOT_SUPPORTED bit so that MSI can still be used */
/**
* pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
@@ -998,6 +1000,8 @@ static int __devinit pci_setup(char *str
if (*str && (str = pcibios_setup(str)) && *str) {
if (!strcmp(str, "nomsi")) {
pci_no_msi();
+ } else if (!strcmp(str, "allowmsi"))
+ pci_allow_msi = 1;
} else {
printk(KERN_ERR "PCI: Unknown option `%s'\n",
str);
Stephen Clark wrote:
> Also, I find it disturbing that we are forcing users to have know about
> all these
> magic options that have to be put on the kernel boot line. My hard drive
> on my
> new laptop would only run at 1.2mbs until I found out I had to use
> combined_mode=libata
> and build a new ramdisk that included ata_piix.
That's what happens when two drivers want to drive the same hardware.
The "slow and safe" default is the only proven-stable option, with the
proven-stable PATA driver. The other two options (drivers/ide for
PATA+SATA -> leads to SATA locksup) and (libata for PATA -> ok but
breaks existing configs, and less field time) are considered less safe.
Combined mode is ugly no matter how you look at it. Just turn it off in
BIOS (or pressure system vendor for this ability if BIOS lacks it, e.g.
some Dell servers)
And throw some annoyance at Intel for creating such a headache.
Jeff
On Wed, 15 Nov 2006 11:41:55 -0800 Randy Dunlap wrote:
> On Wed, 15 Nov 2006 19:44:12 +0100 Andi Kleen wrote:
>
> > On Wednesday 15 November 2006 19:35, Randy Dunlap wrote:
> > > On 15 Nov 2006 05:49:41 +0100 Andi Kleen wrote:
> > >
> > > > BTW there seems to be a new ACPI FADT bit that says "MSI is broken"
> > > > We should probably check that too as a double check.
> > >
> > > Do you mean more than just use that bit when you say "double check"?
> > >
> > > I wonder why this hasn't already been done. (?)
> >
> > Nobody coded it yet.
> >
> > > How's this look? Build-tested only.
> >
> > There should be probably a command line option to overwrite it
>
> OK, anything else?
(oops, v2 didn't quite build)
---
From: Randy Dunlap <[email protected]>
- Implement use of FADT boot_flags.MSI_NOT_SUPPORTED bit to prevent
assigning MSIs.
- Force MSI_NOT_SUPPORTED for ACPI 1.0 tables.
- Add "pci=allowmsi" to override (ignore) the MSI_NOT_SUPPORTED bit
from the APCI FADT.
Signed-off-by: Randy Dunlap <[email protected]>
---
Documentation/kernel-parameters.txt | 6 ++++++
drivers/acpi/bus.c | 7 +++++++
drivers/acpi/tables/tbconvrt.c | 3 ++-
drivers/pci/pci.c | 6 +++++-
include/acpi/actbl.h | 4 +++-
include/linux/pci.h | 3 +++
6 files changed, 26 insertions(+), 3 deletions(-)
--- linux-2619-rc5.orig/include/acpi/actbl.h
+++ linux-2619-rc5/include/acpi/actbl.h
@@ -290,7 +290,7 @@ struct fadt_descriptor_rev1 {
ACPI_FADT_COMMON u32 flags;
};
-/* FADT: Prefered Power Management Profiles */
+/* FADT: Preferred Power Management Profiles */
#define PM_UNSPECIFIED 0
#define PM_DESKTOP 1
@@ -304,6 +304,8 @@ struct fadt_descriptor_rev1 {
#define BAF_LEGACY_DEVICES 0x0001
#define BAF_8042_KEYBOARD_CONTROLLER 0x0002
+#define BAF_VGA_NOT_PRESENT 0x0004
+#define BAF_MSI_NOT_SUPPORTED 0x0008
#define FADT2_REVISION_ID 3
#define FADT2_MINUS_REVISION_ID 2
--- linux-2619-rc5.orig/drivers/acpi/tables/tbconvrt.c
+++ linux-2619-rc5/drivers/acpi/tables/tbconvrt.c
@@ -289,7 +289,8 @@ acpi_tb_convert_fadt1(struct fadt_descri
* Since there isn't any equivalence in 1.0 and since it is highly
* likely that a 1.0 system has legacy support.
*/
- local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES;
+ local_fadt->iapc_boot_arch = BAF_LEGACY_DEVICES |
+ BAF_MSI_NOT_SUPPORTED;
}
/*
--- linux-2619-rc5.orig/include/linux/pci.h
+++ linux-2619-rc5/include/linux/pci.h
@@ -610,6 +610,7 @@ static inline int pci_enable_msix(struct
struct msix_entry *entries, int nvec) {return -1;}
static inline void pci_disable_msix(struct pci_dev *dev) {}
static inline void msi_remove_pci_irq_vectors(struct pci_dev *dev) {}
+static inline void pci_no_msi(void) {}
#else
extern void pci_scan_msi_device(struct pci_dev *dev);
extern int pci_enable_msi(struct pci_dev *dev);
@@ -618,6 +619,8 @@ extern int pci_enable_msix(struct pci_de
struct msix_entry *entries, int nvec);
extern void pci_disable_msix(struct pci_dev *dev);
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
+extern void pci_no_msi(void);
+extern int pci_allow_msi;
#endif
#ifdef CONFIG_HT_IRQ
--- linux-2619-rc5.orig/drivers/acpi/bus.c
+++ linux-2619-rc5/drivers/acpi/bus.c
@@ -28,6 +28,7 @@
#include <linux/kernel.h>
#include <linux/list.h>
#include <linux/sched.h>
+#include <linux/pci.h>
#include <linux/pm.h>
#include <linux/pm_legacy.h>
#include <linux/device.h>
@@ -637,6 +638,12 @@ void __init acpi_early_init(void)
}
#endif
+ if (!pci_allow_msi &&
+ (acpi_fadt.iapc_boot_arch & BAF_MSI_NOT_SUPPORTED)) {
+ printk(KERN_DEBUG PREFIX "MSI not supported, disabling it\n");
+ pci_no_msi();
+ }
+
status =
acpi_enable_subsystem(~
(ACPI_NO_HARDWARE_INIT |
--- linux-2619-rc5.orig/Documentation/kernel-parameters.txt
+++ linux-2619-rc5/Documentation/kernel-parameters.txt
@@ -1178,6 +1178,12 @@ and is between 256 and 4096 characters.
nomsi [MSI] If the PCI_MSI kernel config parameter is
enabled, this kernel boot option can be used to
disable the use of MSI interrupts system-wide.
+ allowmsi [MSI,ACPI] If ACPI FADT says that MSI is not
+ supported on this platform but the user wants
+ to use it anyway, this option tells the kernel
+ to ignore the ACPI FADT MSI flag and allow MSI
+ to be used if there are devices that want to
+ use it.
nosort [IA-32] Don't sort PCI devices according to
order given by the PCI BIOS. This sorting is
done to get a device order compatible with
--- linux-2619-rc5.orig/drivers/pci/pci.c
+++ linux-2619-rc5/drivers/pci/pci.c
@@ -20,6 +20,8 @@
#include "pci.h"
unsigned int pci_pm_d3_delay = 10;
+int pci_allow_msi; /* when set, this overrides the ACPI FADT boot_arch
+ * MSI_NOT_SUPPORTED bit so that MSI can still be used */
/**
* pci_bus_max_busnr - returns maximum PCI bus number of given bus' children
@@ -998,7 +1000,9 @@ static int __devinit pci_setup(char *str
if (*str && (str = pcibios_setup(str)) && *str) {
if (!strcmp(str, "nomsi")) {
pci_no_msi();
- } else {
+ } else if (!strcmp(str, "allowmsi"))
+ pci_allow_msi = 1;
+ else {
printk(KERN_ERR "PCI: Unknown option `%s'\n",
str);
}
On Wednesday 15 November 2006 20:35, Linus Torvalds wrote:
>
> On Wed, 15 Nov 2006, Jeff Garzik wrote:
> >
> > The reason we cannot do this in the generic layer for non-PCI-Ex is only the
> > driver knows whether that PCI 2.2 bit was actually implemented in the device
> > or mapped to some other weird behavior we don't want to touch. DISABLE-INTX
> > is a new bit not present in PCI 2.1 (alas!!).
>
> Ok, I would have a few suggestions, then:
>
> - devices that don't support INTx disable should basically be considered
> to not support MSI at all (ie a driver simply shouldn't even try to
> enable MSI, since enabling MSI includes the implicit DISABLE-INTX)
>
> This is likely ok, since MSI wasn't in PCI 2.1 _either_ afaik. So if
> your hardware has MSI, it probably _does_ have DISABLE-INTX (or at
> least that bit doesn't do anything bad, which is the other case)
>
> - add a flag to "pci_enable_msi()". There really aren't that many users,
> and they basically _all_ want this functionality. Making them call
> another function is just a recipe for disaster, since somebody will
> forget. Having to just say "do I support INTx disable" is much better,
> since it makes the driver writer aware of having to _explicitly_ make
> that choice.
>
> > Maybe a better solution is letting the driver say "pci_dev->intx_ok = 1" right
> > before it calls pci_enable_device().
>
> I hate that, for exactly the same reason I hate "pci_intx()". It just
> means that most drivers won't do it, because it's not even part of the
> normal sequence, and most people don't care. So again, it would actually
> be better in that case to just add a "flags" field to pci_enable_device(),
> although that's a _hell_ of a lot more painful than it would be to do the
> same to "pci_enable_msi()".
>
> > And if we do this, we can follow through on another suggestion I made:
> > disabling INTx on driver exit, to help eliminate any possibility of screaming
> > interrupts after driver unload.
>
> The thing is, I think that's a bad idea for the same reason it's a bad
> idea to disable the BAR's (which we tried and then reverted). It's just
> going to cause problems for soft rebooting etc with firmware that doesn't
> expect it.
>
> A driver should obviously quiesce the device on shutdown, and if it leaves
> a device in a state where it may still generate interrupts, that's a
> _bug_, so disabling INTx is just papering over a much more serious issue.
>
> Linus
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
i just recognised this thread, and i found a solution for some weird stuff
also reported days ago on alsa-users list.
short summary for your information:
asus m2n32 ws professional
amd x2 5000+
everything compiled as 64 bit.
i recognised strange behaviour from 2.6.19-rc1 on.
sound got crappy and if i tried to use snd_hda_intel as modules
rmmod & afterwards modprobe froze the machine completely.
even the kernel sys req key combination didn't work anymore.
after some small discussions on alsa-user ml i recognised this
thread today.
i thought my problem could also exist on this msi stuff.
i disabled msi in kernel config, reboot, and, after starting x & kde
i got immediately a freeze.
last and maybe important last try has been to
enable msi support _but_ boot kernel with cmdline pci=nomsi
this finally did work out. i got a working sound environment again.
if i should supply more information on that, please contact me.
i find it a bit abnormal that the disabling msi in kernel config behaviour
is different from kernel cmdline pci=nomsi option.
best regards
marcel
ps. i don't want to break this thread up into something different, but maybe my
report works out as usefull.
Jeff Garzik wrote:
>Stephen Clark wrote:
>
>
>>Also, I find it disturbing that we are forcing users to have know about
>>all these
>>magic options that have to be put on the kernel boot line. My hard drive
>>on my
>>new laptop would only run at 1.2mbs until I found out I had to use
>>combined_mode=libata
>>and build a new ramdisk that included ata_piix.
>>
>>
>
>That's what happens when two drivers want to drive the same hardware.
>The "slow and safe" default is the only proven-stable option, with the
>proven-stable PATA driver. The other two options (drivers/ide for
>PATA+SATA -> leads to SATA locksup) and (libata for PATA -> ok but
>breaks existing configs, and less field time) are considered less safe.
>
>
>
The problem with this approach is the average user will just think linux
sucks - it is so
slow, they won't know how to investigate or trouble shoot the problem,
and just go back to winblows.
I've got an ich7 chipset that supports sata and ide. My laptop has only
ide devices so why
is there a conflict that has to be resolved by combined mode? Why can't
the sata driver
be smart enough to know there are no sata devices for it to handle?
>Combined mode is ugly no matter how you look at it. Just turn it off in
>BIOS (or pressure system vendor for this ability if BIOS lacks it, e.g.
>some Dell servers)
>
>And throw some annoyance at Intel for creating such a headache.
>
> Jeff
>
>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
--
"They that give up essential liberty to obtain temporary safety,
deserve neither liberty nor safety." (Ben Franklin)
"The course of history shows that as a government grows, liberty
decreases." (Thomas Jefferson)
On Wed, 15 Nov 2006, Mws wrote:
>
> after some small discussions on alsa-user ml i recognised this
> thread today.
> i thought my problem could also exist on this msi stuff.
> i disabled msi in kernel config, reboot, and, after starting x & kde
> i got immediately a freeze.
> last and maybe important last try has been to
> enable msi support _but_ boot kernel with cmdline pci=nomsi
> this finally did work out. i got a working sound environment again.
I expect that you have the exact same issue as Olivier: the "hang" is
probably because you started using the sound device (beeping on the
console is handled by the old built-in speaker, but in X a single beep
tends to be due to sound device drivers), and because of sound irq
misrouting you had some other device (like your harddisk) that got their
irq disabled due to the "nobody cared" issue.
> i find it a bit abnormal that the disabling msi in kernel config behaviour
> is different from kernel cmdline pci=nomsi option.
Now, that does actually worry me. It _should_ have worked with CONFIG_MSI
disabled. I wonder if some of the MSI workarounds actually broke the HDA
driver subtly.
Anyway, it would be a good idea to test the current -git tree if you can,
both with CONFIG_MSI and without (and _without_ any "pci=nomsi" kernel
command line). It should hopefully work, exactly because the HDA driver
now shouldn't even try to do any MSI stuff by default.
Knock wood.
Linus
Has anybody found a system with the MSI_NOT_SUPPORTED
reported by ACPI?
Here is how to check:
http://marc.theaimsgroup.com/?l=linux-acpi&m=115941857423136&w=2
thanks,
-Len
Linus Torvalds wrote:
>
> On Wed, 15 Nov 2006, Mws wrote:
>> after some small discussions on alsa-user ml i recognised this
>> thread today.
>> i thought my problem could also exist on this msi stuff.
>> i disabled msi in kernel config, reboot, and, after starting x & kde
>> i got immediately a freeze.
>> last and maybe important last try has been to
>> enable msi support _but_ boot kernel with cmdline pci=nomsi
>> this finally did work out. i got a working sound environment again.
Usually, the shared IRQ is disabled but it has happened that the system
boots without disabling the IRQ. In that case, the sound is distorted
and the distortion increase when I use the device that shares the IRQ.
(see the boot messages:
http://olivn.trollprod.org/irq_not_disabled_sound_distortion.dmesg )
I can boot the system with pci=nomsi without any problem.
Yinghai Lu who send me some patches to trace irq assignment
said:
"Then it is not caused by transition from MSI to IOAPIC.
Weird,
MSI is enabled, but chip still send int to ioapic.
YH"
Olivier
>
> I expect that you have the exact same issue as Olivier: the "hang" is
> probably because you started using the sound device (beeping on the
> console is handled by the old built-in speaker, but in X a single beep
> tends to be due to sound device drivers), and because of sound irq
> misrouting you had some other device (like your harddisk) that got their
> irq disabled due to the "nobody cared" issue.
>
>> i find it a bit abnormal that the disabling msi in kernel config behaviour
>> is different from kernel cmdline pci=nomsi option.
>
> Now, that does actually worry me. It _should_ have worked with CONFIG_MSI
> disabled. I wonder if some of the MSI workarounds actually broke the HDA
> driver subtly.
>
> Anyway, it would be a good idea to test the current -git tree if you can,
> both with CONFIG_MSI and without (and _without_ any "pci=nomsi" kernel
> command line). It should hopefully work, exactly because the HDA driver
> now shouldn't even try to do any MSI stuff by default.
>
> Knock wood.
>
> Linus
On Wednesday 15 November 2006 21:14, Linus Torvalds wrote:
>
> On Wed, 15 Nov 2006, Mws wrote:
> >
> > after some small discussions on alsa-user ml i recognised this
> > thread today.
> > i thought my problem could also exist on this msi stuff.
> > i disabled msi in kernel config, reboot, and, after starting x & kde
> > i got immediately a freeze.
> > last and maybe important last try has been to
> > enable msi support _but_ boot kernel with cmdline pci=nomsi
> > this finally did work out. i got a working sound environment again.
>
> I expect that you have the exact same issue as Olivier: the "hang" is
> probably because you started using the sound device (beeping on the
> console is handled by the old built-in speaker, but in X a single beep
> tends to be due to sound device drivers), and because of sound irq
> misrouting you had some other device (like your harddisk) that got their
> irq disabled due to the "nobody cared" issue.
>
> > i find it a bit abnormal that the disabling msi in kernel config behaviour
> > is different from kernel cmdline pci=nomsi option.
>
> Now, that does actually worry me. It _should_ have worked with CONFIG_MSI
> disabled. I wonder if some of the MSI workarounds actually broke the HDA
> driver subtly.
>
> Anyway, it would be a good idea to test the current -git tree if you can,
> both with CONFIG_MSI and without (and _without_ any "pci=nomsi" kernel
> command line). It should hopefully work, exactly because the HDA driver
> now shouldn't even try to do any MSI stuff by default.
>
> Knock wood.
>
> Linus
hi,
linus, just cloned current git, but will test it tomorrow morning.
will inform you on the results.
thanks
marcel
> Perhaps with special user space quirks, but not in mainline kernel because it's
> not force enabled (see erratum #78).
I just looked at the revision guide again, and I don't see why erratum
#78 would matter -- that seems to be saying that the bridge itself
doesn't have an MSI capability so it can't generate MSI interrupts
itself. But why does that matter for devices below the bridge?
Anyway I have some HP DL145 G2 servers with 8132 bridges in them.
I'll plug an MSI-capable PCI-X card in and see what happens. So far
I've only used PCIe cards in my systems (but MSI-X works fine there,
with nVidia host bridges).
- R.
> HOWEVER - that's only true on systems with no other PCI bridges. Even if
> you have an Intel NB/SB, what about other bridges in that same system, and
> the devices behind them?
Well... MSIs are just normal stores as far as PCI is concerned. Thus, if
you have P2P bridges in your system and they can't get store ordering
right, I think you have a much bigger problem than getting MSIs wrong.
I think the problem is mostly with host bridges doing crappy stuffs like
decoding MSIs up front and sending them to the CPU out of order or
HT<->PCI (or some home made backbone <-> PCI) bridges where MSIs gets
turned into something else with potentially ordering breakage at the
transformation point or the upstream bus.
> Now, I think that a MSI thing should look like a PCI write to a magic
> address (I'm really not very up on it, so correct me if I'm wrong), and
> thus maybe bridges are bound to get it right, and the only thing we really
> need to worry about is the host bridge.
Host bridges or something <-> PCI (something = HT, or some of the fancy
NB<->SB links other vendors use).
> Maybe. In that case, it might be sensible to have a host-bridge white-table,
> and if we know all Intel
> bridges that claim to support MSI do so correctly, then maybe we can just
> say "ok, always enable it for Intel host bridges".
>
> But right now I'm not convinced we really know what all goes wrong. Maybe
> it's just broken NVidia and AMD bridges. But maybe it's also individual
> devices that continue to (for example) raise _both_ the legacy IRQ line
> _and_ send an MSI request.
Yeah, well, the later can be solved in software by masking the LSI when
MSI is enabled for a device.
Cheers,
Ben.
> Is this absolutely true? I've never been sure about this point, and I
> was rather convinced after reading various documents that once you
> program up the MSI registers to start generating MSI this implicitly
> disabled INTX and this was even in the PCI specification.
I think it is in the spec, that doesn't mean all device vendors get it
right. I have a vendor spec under my eyes at the moment (sorry, can't
say what it is) which has exactly this bug.
Ben.
On Tue, 2006-11-14 at 20:28 -0800, David Miller wrote:
> From: Jeff Garzik <[email protected]>
> Date: Tue, 14 Nov 2006 23:24:04 -0500
>
> > I can't answer for the spec, but at least two independent device vendors
> > recommended to write an MSI driver that way (disable intx, enable msi).
>
> Ok.
>
> > Completely independent of MSI though, a PCI 2.2 compliant driver should
> > be nice and disable intx on exit, just to avoid any potential interrupt
> > hassles after driver unload. And of course be aware that it might need
> > to enable intx upon entry.
>
> This also sounds like it should occur in the generic PCI layer when a
> PCI driver is unregistered.
Is this disable_intx() thingy something x86 specific ? I mean, you can't
just call disable_irq() for LSIs since you can be sharing it. If you
aren't sharing, free_irq() will mask for you.
Ben.
> Is this disable_intx() thingy something x86 specific ? I mean, you can't
> just call disable_irq() for LSIs since you can be sharing it. If you
> aren't sharing, free_irq() will mask for you.
Oops .. my bad, forgot about that new command register bit. When was it
added ? pci 2.3 ?
Also, it should probably be done by pci_disable_device() ... in fact, to
be safe, the driver should disable that before free_irq().
Ben.
On Tue, 2006-11-14 at 20:30 -0800, Roland Dreier wrote:
> > That reminds me of a potential driver bug -- MSI-aware drivers need to
> > call pci_intx(pdev,0) to turn off the legacy PCI interrupt, before
> > enabling MSI interrupts.
>
> Huh? The device can't generate any legacy interrupts once MSI is
> enabled. As the PCI spec says:
No, the device is _not_supposed_to_ :-)
Cheers,
Ben.
Benjamin Herrenschmidt wrote:
> On Tue, 2006-11-14 at 20:28 -0800, David Miller wrote:
>> From: Jeff Garzik <[email protected]>
>> Date: Tue, 14 Nov 2006 23:24:04 -0500
>>
>>> I can't answer for the spec, but at least two independent device vendors
>>> recommended to write an MSI driver that way (disable intx, enable msi).
>> Ok.
>>
>>> Completely independent of MSI though, a PCI 2.2 compliant driver should
>>> be nice and disable intx on exit, just to avoid any potential interrupt
>>> hassles after driver unload. And of course be aware that it might need
>>> to enable intx upon entry.
>> This also sounds like it should occur in the generic PCI layer when a
>> PCI driver is unregistered.
>
> Is this disable_intx() thingy something x86 specific ? I mean, you can't
> just call disable_irq() for LSIs since you can be sharing it. If you
> aren't sharing, free_irq() will mask for you.
We are referring to the standard PCI 2.2 bit, PCI_COMMAND_INTX_DISABLE.
Jeff
On Wednesday 15 November 2006 06:40, Alan wrote:
> > Point is that it is still only one platform. Or don't you understand the
> > point I was making?
>
> Given the Intel chipset platform appears to be over half the market it
> makes sense to do things right there. It also encourages other vendors to
> fix their stuff or send us patches to do so.
>
> So I don't think anyone gives a damn about your personal issues with
> Intel.
>
> Alan
I have no personal issues with Intel. I just always avoided their stuff
because it always cost more and wasn't as well supported in Linux back when I
first started using Linux.
Anyway, my point still stands. One platform does not a whitelist make. Instead
why not make the MSI support optional and a build-option? I do believe that I
even suggested such. This would allow for the people that want it (or need
it) to have it and would keep a potentially dangerous option from being used.
Now, since I'm at the end of options in explaining my point I'm going to start
ignoring further messages on this topic.
DRH
> We are referring to the standard PCI 2.2 bit, PCI_COMMAND_INTX_DISABLE.
Yeah, I figured it, I somewhat forgot about it ... it got introduced in
2.3 though, no ?
Cheers,
Ben.
Please try the patch about using pci_intx.
it could use msi.
YH
Benjamin Herrenschmidt wrote:
>> We are referring to the standard PCI 2.2 bit, PCI_COMMAND_INTX_DISABLE.
>
> Yeah, I figured it, I somewhat forgot about it ... it got introduced in
> 2.3 though, no ?
It's pretty new. 2.2 or 2.3.
Jeff
At Wed, 15 Nov 2006 14:15:45 -0500,
Jeff Garzik wrote:
>
> ACK the pci_intx() calls, NAK the obviously overweight spinlock changes.
> The spinlock changes are completely unnecessary. Just look at any
> other (non-ALSA) PCI driver. Existing "spin_lock()" is fine for both
> PCI shared irq handlers and MSI irq handlers.
>
> It sounds like you are trying to work around a reentrancy problem that
> does not exist.
>
> Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(),
> where separate interrupt sources call the same function.
OK, I revised it, also referring to a similar patch by Yinghai.
I think we can simplify the change like below.
Olivier, could you test this patch, too?
(Here the first chunk (enable_msi=1) is just for testing.
The patch isn't for merge yet.)
Takashi
---
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index e35cfd3..130ee12 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -55,7 +55,7 @@ static char *model;
static int position_fix;
static int probe_mask = -1;
static int single_cmd;
-static int enable_msi;
+static int enable_msi = 1;
module_param(index, int, 0444);
MODULE_PARM_DESC(index, "Index value for Intel HD audio interface.");
@@ -1380,7 +1380,8 @@ static int __devinit azx_init_stream(str
static int azx_acquire_irq(struct azx *chip, int do_disconnect)
{
- if (request_irq(chip->pci->irq, azx_interrupt, IRQF_DISABLED|IRQF_SHARED,
+ if (request_irq(chip->pci->irq, azx_interrupt,
+ chip->msi ? 0 : IRQF_SHARED,
"HDA Intel", chip)) {
printk(KERN_ERR "hda-intel: unable to grab IRQ %d, "
"disabling device\n", chip->pci->irq);
@@ -1389,6 +1390,7 @@ static int azx_acquire_irq(struct azx *c
return -1;
}
chip->irq = chip->pci->irq;
+ pci_intx(chi->pci, !chip->msi);
return 0;
}
On Wednesday 15 November 2006 22:10, Mws wrote:
> On Wednesday 15 November 2006 21:14, Linus Torvalds wrote:
> >
> > On Wed, 15 Nov 2006, Mws wrote:
> > >
> > > after some small discussions on alsa-user ml i recognised this
> > > thread today.
> > > i thought my problem could also exist on this msi stuff.
> > > i disabled msi in kernel config, reboot, and, after starting x & kde
> > > i got immediately a freeze.
> > > last and maybe important last try has been to
> > > enable msi support _but_ boot kernel with cmdline pci=nomsi
> > > this finally did work out. i got a working sound environment again.
> >
> > I expect that you have the exact same issue as Olivier: the "hang" is
> > probably because you started using the sound device (beeping on the
> > console is handled by the old built-in speaker, but in X a single beep
> > tends to be due to sound device drivers), and because of sound irq
> > misrouting you had some other device (like your harddisk) that got their
> > irq disabled due to the "nobody cared" issue.
> >
> > > i find it a bit abnormal that the disabling msi in kernel config behaviour
> > > is different from kernel cmdline pci=nomsi option.
> >
> > Now, that does actually worry me. It _should_ have worked with CONFIG_MSI
> > disabled. I wonder if some of the MSI workarounds actually broke the HDA
> > driver subtly.
> >
> > Anyway, it would be a good idea to test the current -git tree if you can,
> > both with CONFIG_MSI and without (and _without_ any "pci=nomsi" kernel
> > command line). It should hopefully work, exactly because the HDA driver
> > now shouldn't even try to do any MSI stuff by default.
> >
> > Knock wood.
> >
> > Linus
> hi,
>
> linus, just cloned current git, but will test it tomorrow morning.
>
> will inform you on the results.
>
> thanks
> marcel
hi,
i just tried the git that i cloned yesterday, when you asked me to, linus.
i don't get any freezes and sound is fine.
no cmdline option pci=nomsi and msi enabled in .config
thanks for your help.
best regards
marcel
Jeff Garzik <[email protected]> writes:
>>> We are referring to the standard PCI 2.2 bit, PCI_COMMAND_INTX_DISABLE.
>> Yeah, I figured it, I somewhat forgot about it ... it got introduced
>> in
>> 2.3 though, no ?
>
> It's pretty new. 2.2 or 2.3.
2.3.
PCI 2.2 defines bits 0-9 only (bit 7 = Stepping Control)
PCI 2.3 and 3.0: bit 7 = Reserved, bit 10 = Interrupt Disable.
OTOH many devices have "interrupt disable" bit somewhere else, in
their specific PCI config registers or in regular config registers
(accessible with normal Memory Read/Write cycles).
MSI was first defined in PCI 2.2.
Perhaps I can check NV (MCP55) for that problem with a module claiming
all free interrupts. Tomorrow maybe.
--
Krzysztof Halasa
Krzysztof Halasa wrote:
> OTOH many devices have "interrupt disable" bit somewhere else, in
> their specific PCI config registers or in regular config registers
> (accessible with normal Memory Read/Write cycles).
Most interrupt-driven devices have an interrupt mask register of some
sort. The nice thing about PCI_COMMAND_INTX_DISABLE is that it is
standardized.
Jeff
> Most interrupt-driven devices have an interrupt mask register of some
> sort. The nice thing about PCI_COMMAND_INTX_DISABLE is that it is
> standardized.
You all got on me about quoting the spec about not generating INTx
interrupts after MSI is enabled. What makes you think that setting
some bit in the command register, which wasn't even standardized until
PCI 2.3 and which most hardware designers probably forgot about, is
going to work on broken devices?
- R.
Takashi Iwai wrote:
> At Wed, 15 Nov 2006 14:15:45 -0500,
> Jeff Garzik wrote:
>> ACK the pci_intx() calls, NAK the obviously overweight spinlock changes.
>> The spinlock changes are completely unnecessary. Just look at any
>> other (non-ALSA) PCI driver. Existing "spin_lock()" is fine for both
>> PCI shared irq handlers and MSI irq handlers.
>>
>> It sounds like you are trying to work around a reentrancy problem that
>> does not exist.
>>
>> Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(),
>> where separate interrupt sources call the same function.
>
> OK, I revised it, also referring to a similar patch by Yinghai.
> I think we can simplify the change like below.
> Olivier, could you test this patch, too?
Applied to 2.6.19-rc6, the module tries MSI but this time no IRQ get
disabled. The result is equivalent to 2.6.19-rc6
ALSA sound/pci/hda/hda_intel.c:543: hda_intel: No response from codec,
disabling MSI...
hda_codec: Unknown model for AD1988, trying auto-probe from BIOS...
Full details:
http://olivn.trollprod.com/19-rc6/19-rc6-takashi-routeirq1.dmesg
http://olivn.trollprod.com/19-rc6/19-rc6-takashi-routeirq1.irq
Olivier
>
> (Here the first chunk (enable_msi=1) is just for testing.
> The patch isn't for merge yet.)
>
>
> Takashi
>
> ---
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e35cfd3..130ee12 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -55,7 +55,7 @@ static char *model;
> static int position_fix;
> static int probe_mask = -1;
> static int single_cmd;
> -static int enable_msi;
> +static int enable_msi = 1;
>
> module_param(index, int, 0444);
> MODULE_PARM_DESC(index, "Index value for Intel HD audio interface.");
> @@ -1380,7 +1380,8 @@ static int __devinit azx_init_stream(str
>
> static int azx_acquire_irq(struct azx *chip, int do_disconnect)
> {
> - if (request_irq(chip->pci->irq, azx_interrupt, IRQF_DISABLED|IRQF_SHARED,
> + if (request_irq(chip->pci->irq, azx_interrupt,
> + chip->msi ? 0 : IRQF_SHARED,
> "HDA Intel", chip)) {
> printk(KERN_ERR "hda-intel: unable to grab IRQ %d, "
> "disabling device\n", chip->pci->irq);
> @@ -1389,6 +1390,7 @@ static int azx_acquire_irq(struct azx *c
> return -1;
> }
> chip->irq = chip->pci->irq;
> + pci_intx(chi->pci, !chip->msi);
> return 0;
> }
>
--
Laudator temporis acti (Horace, 173)
Donner c'est donner, repeindre ses volets
Yinghai Lu wrote:
> Please try the patch about using pci_intx.
>
> it could use msi.
It can't cold boot with this one (with pci=routeirq or not)
The last visible boot messages are
Interrupt Link [AAZA] enabled at IRQ 20
Interrupt 0000:00:0e.1[B]->Link [AAZA] -> GSI 20 (level,low) -> IRQ 20
Olivier
>
> YH
>
>
> ------------------------------------------------------------------------
>
> diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
> index e35cfd3..88b99ab 100644
> --- a/sound/pci/hda/hda_intel.c
> +++ b/sound/pci/hda/hda_intel.c
> @@ -547,6 +547,7 @@ static unsigned int azx_rirb_get_respons
> chip->msi = 0;
> if (azx_acquire_irq(chip, 1) < 0)
> return -1;
> + pci_intx(chip->pci, 1);
> goto again;
> }
>
> @@ -1435,11 +1436,16 @@ static int azx_resume(struct pci_dev *pc
> return -EIO;
> }
> pci_set_master(pci);
> - if (chip->msi)
> + if (chip->msi) {
> + pci_intx(pci, 0);
> if (pci_enable_msi(pci) < 0)
> chip->msi = 0;
> + }
> if (azx_acquire_irq(chip, 1) < 0)
> return -EIO;
> +
> + if (!chip->msi)
> + pci_intx(pci, 1);
> azx_init_chip(chip);
> snd_hda_resume(chip->bus);
> snd_power_change_state(card, SNDRV_CTL_POWER_D0);
> @@ -1531,7 +1537,8 @@ static int __devinit azx_create(struct s
> chip->pci = pci;
> chip->irq = -1;
> chip->driver_type = driver_type;
> - chip->msi = enable_msi;
> +// chip->msi = enable_msi;
> + chip->msi = 1;
>
> chip->position_fix = position_fix;
> chip->single_cmd = single_cmd;
> @@ -1561,14 +1568,19 @@ #endif
> goto errout;
> }
>
> - if (chip->msi)
> + if (chip->msi) {
> + pci_intx(pci, 0);
> if (pci_enable_msi(pci) < 0)
> chip->msi = 0;
> + }
>
> if (azx_acquire_irq(chip, 0) < 0) {
> err = -EBUSY;
> goto errout;
> }
> +
> + if(!chip->msi)
> + pci_intx(pci, 1);
>
> pci_set_master(pci);
> synchronize_irq(chip->irq);
--
Laudator temporis acti (Horace, 173)
Donner c'est donner, repeindre ses volets
At Fri, 17 Nov 2006 00:01:25 +0100,
Olivier Nicolas wrote:
>
> Takashi Iwai wrote:
> > At Wed, 15 Nov 2006 14:15:45 -0500,
> > Jeff Garzik wrote:
> >> ACK the pci_intx() calls, NAK the obviously overweight spinlock changes.
> >> The spinlock changes are completely unnecessary. Just look at any
> >> other (non-ALSA) PCI driver. Existing "spin_lock()" is fine for both
> >> PCI shared irq handlers and MSI irq handlers.
> >>
> >> It sounds like you are trying to work around a reentrancy problem that
> >> does not exist.
> >>
> >> Only weird drivers like ps2kbd/mouse or IDE need spin_lock_irqsave(),
> >> where separate interrupt sources call the same function.
> >
> > OK, I revised it, also referring to a similar patch by Yinghai.
> > I think we can simplify the change like below.
> > Olivier, could you test this patch, too?
>
> Applied to 2.6.19-rc6, the module tries MSI but this time no IRQ get
> disabled. The result is equivalent to 2.6.19-rc6
Does it mean that the driver works as expected with this patch?
>
> ALSA sound/pci/hda/hda_intel.c:543: hda_intel: No response from codec,
> disabling MSI...
> hda_codec: Unknown model for AD1988, trying auto-probe from BIOS...
>
>
> Full details:
> http://olivn.trollprod.com/19-rc6/19-rc6-takashi-routeirq1.dmesg
> http://olivn.trollprod.com/19-rc6/19-rc6-takashi-routeirq1.irq
These look OK to me.
thanks,
Takashi
the fallback path from MSI test to ioapic still not look good.
I think you could seperate azx_interrupt_test later.
It seems on C51+MCP55 has problem to use MSI for hda.
and I have tried two MCP55 only systems, the MSI for hda works well.
YH
On Fri, 17 Nov 2006, Yinghai Lu wrote:
>
> the fallback path from MSI test to ioapic still not look good.
I don't think there should be a fallback at all.
Let's just:
- keep MSI disabled by default for now, until we know what's really going
on (and we can try enabling it every once in a while in a -rc1, just to
get more information ;)
- when MSI is selected (ie both the HDA driver _and_ the system decides
that MSI is ok), it should just get used. No testing. No fallbacks. No
strange code at all. Just use it.
The whole notion of trying to see if MSI works is very suspect, and likely
fragile. We should never _ever_ "switch" between the two modes. We choose
one mode, and we stick to it.
Eventually, MSI will hopefully be the more robust of the two methods.
Maybe it will take a year. And maybe we'll end up not using MSI on a lot
of the _current_ crop of motherboards. We don't want to carry the "fall
back from MSI to INTx" code around. We just want a flag saying "use MSI"
or "use INTx".
Linus