[RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Thibaut VARENE]

[-- Attachment #1: Type: text/plain, Size: 3103 bytes --]

Hi

I've been debugging some IDE issues on my ppc box lately, and ran into the
following problem:

my machine has a "ghost" ide controller: a controller that has no physical
port and thus nothing hooked to it. The pmac ide driver first claims it,
then *tries* to release it. Unfortunately, it doesn't do that well enough,
because when one adds another (pci) controller to the box, the following
happens:

relevant snippets of dmesg output:

ide2: Found Apple KeyLargo ATA-3 controller, bus ID 1, irq 21
Probing IDE interface ide2...
ide2: Bus empty, interface released.
[...]
AEC6280R: IDE controller at PCI slot 0001:11:03.0
AEC6280R: chipset revision 7
AEC6280R: ROM enabled at 0x800a0000
AEC6280R: 100% native mode on irq 53
ide2: AEC6280R Bus-Master DMA disabled (BIOS)
    ide3: BM-DMA at 0x1008-0x100f, BIOS settings: hdg:pio, hdh:pio
kernel BUG in ide_setup_dma at drivers/ide/ide-dma.c:954!
Oops: Exception in kernel mode, sig: 5 [#1]
SMP NR_CPUS=2
NIP: C01F1FD4 LR: C01F1FC8 SP: EE475CB0 REGS: ee475c00 TRAP: 0700    Not
tainted
MSR: 00029032 EE: 1 PR: 0 FP: 0 ME: 1 IR/DR: 11
TASK = efc89270[1562] 'insmod' THREAD: ee474000
Last syscall: 128  CPU: 1
GPR00: 00000000 EE475CB0 EFC89270 00000001 00009032 FFFFFFFF C0480000
C0489FAC
GPR08: 00008000 C03CC0B0 000023D1 C03D0000 C0480000 100196C8 100D0000
100FB468
GPR16: EE475D1A C04A8180 C04A5480 00000035 EE475DB8 00000002 C04A5480
EFFD3438
GPR24: F107A7B5 00000001 C03D0000 C03CC2CC 00000008 C04A6174 EE523000
C04A5C00
NIP [c01f1fd4] ide_setup_dma+0x284/0x5f0
LR [c01f1fc8] ide_setup_dma+0x278/0x5f0
Call trace:
 [f107a320] init_dma_aec62xx+0xa0/0x140 [aec62xx]
 [c01f06c0] ide_pci_setup_ports+0x520/0x700


Trying to debug that oops, it appears that during the initialisation
process, the generic code accesses the hwif which still contains
pmac-related (invalid) pointers, totally irrelevant to the PCI driver.
Thus, the hwif is clobbered and the first time a generic function tries to
access these pointers, the kernel oopses.

The raw "fix" to this is to not release the hwif in ide/ppc/pmac.c:1401.
Indeed, with the attached patch, the card initialization goes (almost)
fine:

AEC6280R: IDE controller at PCI slot 0001:11:03.0
AEC6280R: chipset revision 7
AEC6280R: ROM enabled at 0x800a0000
AEC6280R: 100% native mode on irq 53
    ide3: BM-DMA at 0x1000-0x1007, BIOS settings: hdg:pio, hdh:pio
    ide4: BM-DMA at 0x1008-0x100f, BIOS settings: hdi:pio, hdj:pio

(the "almost" will be detailed in a subsequent mail, it's irrelevant to
the present thread).

I've been investigating the IDE code a bit, and it seems that it generally
assumes that an interface cannot be released once claimed. Cleaning up the
hwif without knowing its previous state in pmac.c is a big and potentially
dangerous job, and there seems to be no generic code interface that would
provide either a way to properly release an hwif or to clean it up.

The rationale behind this is related to the value of MAX_HWIFS, which is
usually pretty low (usually between 2 and 10).

So the question is "what should be done (and how)"?

HTH

T-Bone

PS: please CC me in answers.

[-- Attachment #2: pmac.c.diff --]
[-- Type: text/plain, Size: 582 bytes --]

--- linux-2.6.14-rc5.old/drivers/ide/ppc/pmac.c	2005-10-29 02:32:27.000000000 +0200
+++ linux-2.6.14-rc5.new/drivers/ide/ppc/pmac.c	2005-10-29 03:22:48.000000000 +0200
@@ -1401,6 +1401,7 @@
 	/* The code IDE code will have set hwif->present if we have devices attached,
 	 * if we don't, the discard the interface except if we are on a media bay slot
 	 */
+#if 0
 	if (!hwif->present && !pmif->mediabay) {
 		printk(KERN_INFO "ide%d: Bus empty, interface released.\n",
 			hwif->index);
@@ -1411,7 +1412,7 @@
 		hwif->noprobe = 1;
 		return -ENODEV;
 	}
-
+#endif
 	return 0;
 }
 

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Benjamin Herrenschmidt]

On Sat, 2005-10-29 at 04:48 +0200, Thibaut VARENE wrote:

> I've been investigating the IDE code a bit, and it seems that it generally
> assumes that an interface cannot be released once claimed. Cleaning up the
> hwif without knowing its previous state in pmac.c is a big and potentially
> dangerous job, and there seems to be no generic code interface that would
> provide either a way to properly release an hwif or to clean it up.
> 
> The rationale behind this is related to the value of MAX_HWIFS, which is
> usually pretty low (usually between 2 and 10).
> 
> So the question is "what should be done (and how)"?

Bart, the problem here is indeed the code in pmac IDE that tries to
"release" HWIFs to the core if they are probed empty. For some reason,
it used to work (I think ide-dma used to unconditionally override the
callbacks) but not any more.

Thus my question: Should I

 - Manually in ide-pmac clean up all callbacks & revert everything in
the HWIF when I want to hand it back

 - Call init_hwif_data() despite the nasty comment on top

 - Just don't release the interface...

Ben.

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Bartlomiej Zolnierkiewicz]

On 10/29/05, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> On Sat, 2005-10-29 at 04:48 +0200, Thibaut VARENE wrote:
>
> > I've been investigating the IDE code a bit, and it seems that it generally
> > assumes that an interface cannot be released once claimed. Cleaning up the
> > hwif without knowing its previous state in pmac.c is a big and potentially
> > dangerous job, and there seems to be no generic code interface that would
> > provide either a way to properly release an hwif or to clean it up.
> >
> > The rationale behind this is related to the value of MAX_HWIFS, which is
> > usually pretty low (usually between 2 and 10).
> >
> > So the question is "what should be done (and how)"?
>
> Bart, the problem here is indeed the code in pmac IDE that tries to
> "release" HWIFs to the core if they are probed empty. For some reason,
> it used to work (I think ide-dma used to unconditionally override the
> callbacks) but not any more.
>
> Thus my question: Should I
>
>  - Manually in ide-pmac clean up all callbacks & revert everything in
> the HWIF when I want to hand it back
>
>  - Call init_hwif_data() despite the nasty comment on top
>
>  - Just don't release the interface...

Just don't release it - it is the simplest solution and also
as the easiest to maintain in future (hotplug).

Bartlomiej

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Benjamin Herrenschmidt]

> > Thus my question: Should I
> >
> >  - Manually in ide-pmac clean up all callbacks & revert everything in
> > the HWIF when I want to hand it back
> >
> >  - Call init_hwif_data() despite the nasty comment on top
> >
> >  - Just don't release the interface...
> 
> Just don't release it - it is the simplest solution and also
> as the easiest to maintain in future (hotplug).

Well, not exactly :) If I start to add unplug of that driver, the
problem will be back, trying to release an interface or rmmod the module
will leave all sort of crap in the HWIF ... So I think you need to
exposrt something from the IDE core to totally cleanup a HWIF (return it
to pristine state) for use in those cases.

The exact same goes with ide-cd btw. It's not changing as many fields
as, afaik, it doesn't do DMA, but still. When the CS is removed, all
HWIF fields should be reverted to their initial state

Ben.

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Bartlomiej Zolnierkiewicz]

On 11/1/05, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > > Thus my question: Should I
> > >
> > >  - Manually in ide-pmac clean up all callbacks & revert everything in
> > > the HWIF when I want to hand it back
> > >
> > >  - Call init_hwif_data() despite the nasty comment on top
> > >
> > >  - Just don't release the interface...
> >
> > Just don't release it - it is the simplest solution and also
> > as the easiest to maintain in future (hotplug).
>
> Well, not exactly :) If I start to add unplug of that driver, the
> problem will be back, trying to release an interface or rmmod the module

Current behavior is such that pmac driver releases port which
doesn't have devices connected so new behavior will be step forward
(although small).

> will leave all sort of crap in the HWIF ... So I think you need to
> exposrt something from the IDE core to totally cleanup a HWIF (return it
> to pristine state) for use in those cases.
>
> The exact same goes with ide-cd btw. It's not changing as many fields
> as, afaik, it doesn't do DMA, but still. When the CS is removed, all
> HWIF fields should be reverted to their initial state.

"in future" we won't be doing brain-damaged restoring of HWIF
fields in ide_unregister() and the problem will go away.  If you
want to go ahead feel free to but remember to separate 2 states
(HWIF is unregistered by ioctl and HWIF is really going away)
which are currently both handled by ide_unregister().

Thanks,
Bartlomiej

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Benjamin Herrenschmidt]

> "in future" we won't be doing brain-damaged restoring of HWIF
> fields in ide_unregister() and the problem will go away.  If you
> want to go ahead feel free to but remember to separate 2 states
> (HWIF is unregistered by ioctl and HWIF is really going away)
> which are currently both handled by ide_unregister().

Yup... maybe I should do ide_unregister() :)

Ben.

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Bartlomiej Zolnierkiewicz]

Hi Ben,

Thibaut's change is fine with me so unless you've better one I would
like to merge it for 2.6.15.

Bartlomiej

On 11/2/05, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > "in future" we won't be doing brain-damaged restoring of HWIF
> > fields in ide_unregister() and the problem will go away.  If you
> > want to go ahead feel free to but remember to separate 2 states
> > (HWIF is unregistered by ioctl and HWIF is really going away)
> > which are currently both handled by ide_unregister().
>
> Yup... maybe I should do ide_unregister() :)
>
> Ben.

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Benjamin Herrenschmidt]

On Thu, 2005-11-10 at 11:56 +0100, Bartlomiej Zolnierkiewicz wrote:
> Hi Ben,
> 
> Thibaut's change is fine with me so unless you've better one I would
> like to merge it for 2.6.15.

Yup, go for it !

Thanks,
Ben.

Re: [RFC PATCH] pmac ide doesn't properly cleanup hwif when releasing controller

[Bartlomiej Zolnierkiewicz]

applied