Bug 10819

Michael, does this look like a driver bug or a problem elsewhere?

Christian, please attach the outputs of "dmesg -s 1000000" for both 2.6.25.4 and 2.6.26-rc4.

I've also seen a suspicious sonic DMA init message in the 2.6.26 log.
I'm appending the whole dmesg output for both kernel.

Created attachment 16328 [details]
dmesg output for 2.6.26-rc4

FYI, I've also activated debug output for this kernel, as well as kernel hacking options.

Created attachment 16329 [details]
dmesg output for 2.6.25.4

This output is without b43 debug options, since it's my production kernel.
Could redo it if necessary of course.

This entry is being used for tracking a regression from 2.6.25.  Please don't
close it until the problem is fixed in the mainline.

Can you please bisect. I have no idea what could be causing this regression.

Well, I couldn't go very far:
2.6.26-rc1 does not boot on my laptop,
2.6.26-rc2 also fails with DMA errors.
I'm suspicious toward the sonic error:

ACPI: PCI Interrupt Link [LNK4] enabled at IRQ 19
ACPI: PCI Interrupt 0000:02:08.0[A] -> Link [LNK4] -> GSI 19 (level, low) -> IRQ 19
ssb: Sonics Silicon Backplane found on PCI device 0000:02:08.0
PCI: region 0000:02:07.0/9 too large: 0x0000000000000000-0x0000000003ffffff
PCI: Failed to allocate mem resource #10:4000000@d4000000 for 0000:02:07.0
PCI: Bus 3, cardbus bridge: 0000:02:07.0
  IO window: 0x00003000-0x000030ff
  IO window: 0x00003400-0x000034ff

which I don't have in 2.6.25.4.

This sounds like a problem anywhere but in b43. We had that in the past that subsystem changes silently broke b43. The only chance to find the cause really is to bisect this.

(In reply to comment #8)
> This sounds like a problem anywhere but in b43. We had that in the past that
> subsystem changes silently broke b43. The only chance to find the cause
> really
> is to bisect this.
> 

The reporter already said that bisection is difficult because of other, unrelated problems.

Can you at least tell us the significance of these numbers, in particular the single bit that has been set in the first number?

b43-phy0 ERROR: Fatal DMA error: 0x00001000, 0x00000000, 0x00000000,
0x00000000, 0x00000000, 0x00000000

These purpose of these flags are not so easy to determine by looking at the source code...

#define B43_DMAIRQ_FATALMASK    ((1 << 10) | (1 << 11) | (1 << 12) \
                                         | (1 << 14) | (1 << 15))
#define B43_DMAIRQ_NONFATALMASK (1 << 13)
#define B43_DMAIRQ_RX_DONE              (1 << 16)

It would also be really nice with some pointers to b43 hardware documentation if such a thing exists.

Thanks!


Vegard

(In reply to comment #9)
> b43-phy0 ERROR: Fatal DMA error: 0x00001000, 0x00000000, 0x00000000,
> 0x00000000, 0x00000000, 0x00000000
> 
> These purpose of these flags are not so easy to determine by looking at the
> source code...
> 
> #define B43_DMAIRQ_FATALMASK    ((1 << 10) | (1 << 11) | (1 << 12) \
>                                          | (1 << 14) | (1 << 15))
> #define B43_DMAIRQ_NONFATALMASK (1 << 13)
> #define B43_DMAIRQ_RX_DONE              (1 << 16)
> 
> It would also be really nice with some pointers to b43 hardware documentation
> if such a thing exists.

0x00001000     Descriptor Protocol Error      Fatal

It basically says DMA is not working. Really the only way to debug this is bisecting.

Hi Christian,

I have now backported the b43 driver from v2.6.26-rc4 on top of v2.6.25. You can find the git tree here:

http://git.kernel.org/?p=linux/kernel/git/vegard/linux-2.6.25-b43-backport.git

Can you try checking it out and compiling the HEAD (latest commit, basically what you get when you check it out)?

Then report back whether the error is still present or not. If the DMA error is not there, then Michael is probably right and the error was introduced by something outside the b43 driver.

On the other hand, if the DMA error _is_ there, it means the real error was introduced in one of the mac80211 or bc43 changes that I backported. This tree should be a lot easier to bisect as it only contains the relevant commits. Also, if you while bisecting reach an unbootable or uncompilable state, you can use "git bisect skip" to skip that stage (this will take slightly longer than a normal bisect, however).

Please report back your results. I cannot guarantee that the backported driver will compile in all stages or even boot (but I do hope that it will). I have only test-compiled the HEAD here. Good luck!


Vegard

(In reply to comment #11)
> Hi Christian,
> 
> I have now backported the b43 driver from v2.6.26-rc4 on top of v2.6.25. You
> can find the git tree here:
> 
>
> http://git.kernel.org/?p=linux/kernel/git/vegard/linux-2.6.25-b43-backport.git
> 
> Can you try checking it out and compiling the HEAD (latest commit, basically
> what you get when you check it out)?

Actually, it is having a hard time compiling correctly for me. I'll send a new notification when it's fixed :-(

(The "git bisect skip" comment is still valid, though, if you want to still try bisecting the mainline kernel.)

(In reply to comment #12)
> (In reply to comment #11)
> >
> http://git.kernel.org/?p=linux/kernel/git/vegard/linux-2.6.25-b43-backport.git
> > 
> > Can you try checking it out and compiling the HEAD (latest commit,
> basically
> > what you get when you check it out)?
> 
> Actually, it is having a hard time compiling correctly for me. I'll send a
> new
> notification when it's fixed :-(

Updated, compiled, booted, and works for me :-)

Euh, well, I'm very awkward, but I'm not accustomed to Unix dev tools, and I didn't managed to get the repository...
I did:
git clone http://git.kernel.org/?p=linux/kernel/git/vegard/linux-2.6.25-b43-backport.git

in a fresh new directory, and it says:
/usr/bin/git-clone: line 108: /usr/bin/git-http-fetch: Liste d'arguments trop longue
/usr/bin/git-clone: line 478: cd: /home/christian/az/linux-2.6.25-b43-backport/.git/refs/remotes/origin: Aucun fichier ou répertoire de ce type
Warning: Remote HEAD refers to nonexistent ref, unable to checkout.

(the "argument list was too long" error [in French, sorry] was the problem I guess).
I didn't find a way to get it work. Any idea?

(In reply to comment #14)
> Euh, well, I'm very awkward, but I'm not accustomed to Unix dev tools, and I
> didn't managed to get the repository...
> I did:
> git clone
>
> http://git.kernel.org/?p=linux/kernel/git/vegard/linux-2.6.25-b43-backport.git

Ah, hehe, no need to worry. That is just the gitweb link, to be used for online browsing. To actually clone it, try:

git clone git://git.kernel.org/pub/scm/linux/kernel/git/vegard/linux-2.6.25-b43-backport.git

I also forgot to thank you for persisting in this matter; you have found an error in the kernel and it is very important to fix each and every error we encounter. So thank you very much for reporting and helping us resolve the error!

Well, I booted your kernel and the b43 driver indeed works. So it's in the core kernel code...
Maybe it can be instrumentation code (last time I got problems that was because IOMMU wasn't honoring DMA masks flags). I will retest 2.6.26 without this code tomorrow to see, but I really think it will still fail this time.

(In reply to comment #16)
> (last time I got problems that was because IOMMU wasn't honoring DMA masks
> flags).

Yes, something like that is most likely also the case here.
b43 depends on the DMA masks to get honored. It implements fallbacks through GFP_DMA, but most systems don't support that anyway.

As I said. The only way to debug this is bisecting.

(In reply to comment #17)
> (In reply to comment #16)
> > (last time I got problems that was because IOMMU wasn't honoring DMA masks
> flags).
> 
> Yes, something like that is most likely also the case here.
> b43 depends on the DMA masks to get honored. It implements fallbacks through
> GFP_DMA, but most systems don't support that anyway.
> 
> As I said. The only way to debug this is bisecting.
> 

Michael,

I did not include SSB patches in my backport. I've noticed now that SSB also gives an error in the log, so I looked through the SSB patches between v2.6.25 and v2.6.26-rc4.

This patch looks interesting:

commit 9788ba7500c3a6268ceb63296a0f37f34d450beb
Author: Michael Buesch <mb@bu3sch.de>
Date:   Fri Mar 28 10:34:55 2008 +0100

    ssb-pcmcia: IRQ and DMA related fixes

It has this change:
-       /* Init IRQ routing */
-       if (bus->chip_id == 0x4306)
-               offset = SSB_PCMCIA_CORECTL;
-       else
-               offset = SSB_PCMCIA_CORECTL2;
-       err = ssb_pcmcia_cfg_read(bus, offset, &val);
+       /* Init the COR register. */
+       err = ssb_pcmcia_cor_setup(bus, CISREG_COR);
        if (err)

And this chip ID matches Christian's card.

What do you think?

Christian, can you try to either cherry-pick this commit into the backport tree (git-cherry-pick 9788ba7500c3a6268ceb63296a0f37f34d450beb) or revert it from the v2.6.26 tree (git-revert 9788ba7500c3a6268ceb63296a0f37f34d450beb)? If this doesn't work, I can rebuild the backport tree to include ssb changes.

Thanks.


Vegard

(In reply to comment #18)
> I did not include SSB patches in my backport. I've noticed now that SSB also
> gives an error in the log, s

I'm still not sure which SSB message you are talking about the whole time. I don't see a SSB error. I only see an error message from the core PCI subsystem. (Or IOMAP, not sure).

> 
> This patch looks interesting:
> 
> commit 9788ba7500c3a6268ceb63296a0f37f34d450beb
> Author: Michael Buesch <mb@bu3sch.de>
> Date:   Fri Mar 28 10:34:55 2008 +0100
> 
>     ssb-pcmcia: IRQ and DMA related fixes

No it doesn't. This is completely unrelated, as it's only for PCMCIA devices.
That's not the case here.

> It has this change:
> -       /* Init IRQ routing */
> -       if (bus->chip_id == 0x4306)
> -               offset = SSB_PCMCIA_CORECTL;
> -       else
> -               offset = SSB_PCMCIA_CORECTL2;
> -       err = ssb_pcmcia_cfg_read(bus, offset, &val);
> +       /* Init the COR register. */
> +       err = ssb_pcmcia_cor_setup(bus, CISREG_COR);
>         if (err)
> 
> And this chip ID matches Christian's card.

But the card is not a PCMCIA card.

> What do you think?
> 
> Christian, can you try to either cherry-pick this commit into the backport
> tree
> (git-cherry-pick 9788ba7500c3a6268ceb63296a0f37f34d450beb) or revert it from
> the v2.6.26 tree (git-revert 9788ba7500c3a6268ceb63296a0f37f34d450beb)? If
> this
> doesn't work, I can rebuild the backport tree to include ssb changes.

It would be so trivial to simply bisect the tree instead of cherrypicking some patches that can't cause this, for sure. ;)

I've just tried without the kernel debugging hacks & options, it still doesn't work.
The errors in the PCI subsystem are also present in the 2.6.25.4 kernel dmesg btw, I've just noticed, so it doesn't seem to be what breaks here.

@mb : nevertheless, I can't bisect it even if I wanted. The 2.6.26 kernel never worked for me. Maybe I could find the first commit that make the kernel boot on my machine (it's between 2.6.26-rc1 and 2.6.26-rc2), and hope my b43 works then to bisect between that revision and -rc2, but I doubt it would work, -rc1 certainly will also fail I think. It would then require first to backport fixes from -rc2 to -rc1. I cannot bisect between a non bootable kernel and a kernel that boots but doesn't work!
I'm not competent enough for such a high level diagnostic/analysys/backporting. Sorry, I'm not a linux kernel programmer :-( And I' a too bad programmer to debug my own code, as we say, so I do test mine :-). All I can do (and I indeed do) is checking "nearly stable" -rcs and reports bugs on my 2 PCs. I've got 3 for this revision, I think that's enough for me... and fearing not being able to pursue since rc5 doesn't even boot on one of my box (IDE completly broken). My skills ends up un adding printk, patching, and testing...

Btw, I'll certainly have to dive into the code one more time with prinks to see what the hell breaks everything (also very time consuming, especially since I do not know the code and the subsystems). But to be fair, I would prefer fix the IDE layer on my main box now!

(In reply to comment #20)
> to bisect between that revision and -rc2, but I doubt it would work, -rc1
> certainly will also fail I think. It would then require first to backport
> fixes

Well, git bisect skip was mentioned. You can skip revisions that don't build or run with this.

> My skills ends up un adding printk, patching, and testing...

Yeah, printk is fine. I just don't know where to put one :P

> Btw, I'll certainly have to dive into the code one more time with prinks to
> see
> what the hell breaks everything (also very time consuming, especially since I
> do not know the code and the subsystems). But to be fair, I would prefer fix
> the IDE layer on my main box now!

In the b43 DMA code you'll most likely see that you get DMA addresses from the kernel that are wider than 30bit. But your device can only handle 30bit.

(In reply to comment #19)

Thanks, I realized the PCMCIA/PCI blunder too :-(

And yes, I agree that bisect is probably easier than reverting/cherry-picking random commits, that was not a very useful suggestion on my part. But I do think the idea of backporting b43 was a good one, and it did confirm your theory that it was outside b43 code.

(In reply to comment #21)
> In the b43 DMA code you'll most likely see that you get DMA addresses from
> the
> kernel that are wider than 30bit. But your device can only handle 30bit.
> 

Hm, if that is the case, why don't you have the appropriate WARN_ON()?

That would be useful for catching errors and tell us exactly what's going on with very little hassle.

But this sort of suggestion is very useful. Do you have any other ideas like this?

(In reply to comment #22)
> (In reply to comment #19)
> 
> Thanks, I realized the PCMCIA/PCI blunder too :-(
> 
> And yes, I agree that bisect is probably easier than reverting/cherry-picking
> random commits, that was not a very useful suggestion on my part. But I do
> think the idea of backporting b43 was a good one, and it did confirm your
> theory that it was outside b43 code.

yes, thanks a lot for doing this.

> (In reply to comment #21)
> > In the b43 DMA code you'll most likely see that you get DMA addresses from
> the
> > kernel that are wider than 30bit. But your device can only handle 30bit.
> > 
> 
> Hm, if that is the case, why don't you have the appropriate WARN_ON()?
> 
> That would be useful for catching errors and tell us exactly what's going on
> with very little hassle.

I don't think so.
First thing is: If DMA fails, there's really not much that can go wrong. It's really only the address that can be too long.
Second thing is: We know that the address is too long. Well, that gains us exactly nothing. :)

There's a bug somewhere in the kernel, but not in b43. That's what I was saying from the start of this bug thread.

Christian/Vegard, there's a DMA related patch posted by Michael:

http://lkml.org/lkml/2008/6/12/227

Does it fix the problem you're seeing?

Yes it works :-)
I applied the patch on -rc6, and now b43 is happy.
Good job Michael, I suspected SSB for wrong reasons, but I certainly would not been able to go further. Of course I will keep testing each -rc, and you prove one more time that the Linux community has the best support I've ever seen as an IT pro.
Thanks again and congrats!
PS: I mark the bug as resolved, but I do not know who's in charge to close it.

(In reply to comment #25)
> Yes it works :-)
> I applied the patch on -rc6, and now b43 is happy.
> Good job Michael, I suspected SSB for wrong reasons, but I certainly would
> not
> been able to go further. Of course I will keep testing each -rc, and you
> prove
> one more time that the Linux community has the best support I've ever seen as
> an IT pro.
> Thanks again and congrats!
> PS: I mark the bug as resolved, but I do not know who's in charge to close
> it.

That's great. Cheers to Michael for fixing and to Christian for testing. I'll mark it closed when the patch hits mainline.

Well, the real credits go to "Kirill A. Shutemov" for finding and reporting this bug in the first place. I only did the patch.
Thread was "[PATCH] ssb: Partial revert of 4ac5846 to fix coherent DMA mask setting" on lkml. The patch was wrong, but the bug was the right one. :)

Handled-By      : Michael Buesch <mb@bu3sch.de>
Patch           : http://lkml.org/lkml/2008/6/14/179

fixed by commit e6340361f9c70e84312caed98c6e058ac6234e9b