Re: [PATCH] powerpc: disable MSI using new interface if possible

[Michael Ellerman <michael@ellerman.id.au>]

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

On Thu, 2011-03-03 at 17:41 -0800, Nishanth Aravamudan wrote:
> On 04.03.2011 [12:05:29 +1100], Michael Ellerman wrote:
> > On Thu, 2011-03-03 at 11:39 -0800, Nishanth Aravamudan wrote:
> > > On upcoming hardware, we have a PCI adapter with two functions, one of
> > > which uses MSI and the other uses MSI-X. This adapter, when MSI is
> > > disabled using the "old" firmware interface (RTAS_CHANGE_FN), still
> > > signals an MSI-X interrupt and triggers an EEH. We are working with the
> > > vendor to ensure that the hardware is not at fault, but if we use the
> > > "new" interface (RTAS_CHANGE_MSI_FN) to disable MSI, we also
> > > automatically disable MSI-X and the adapter does not appear to signal
> > > any stray MSI-X interrupt.
> > 
> > It seems this could also be a firmware bug, have we heard anything from
> > them? PAPR explicitly says that RTAS_CHANGE_FN (function=1) should
> > disable MSI _and_ MSI-X (R1???7.3.10.5.1???1).
> 
> We're tracking that down too. I think the fact that the interrupt is
> coming in is a hardware bug in this particular adapter.
> 
> I'm looking at PAPR again and I see what might be a contradiction:
> 
> 7.3.10.5.1: "To removing all MSIs, set the Requested Number of
> Interrupts to zero."
> 
> Table 71: "Function ... 1: Request to set to a new number of MSI or
> MSI-X (platform choice) interrupts (including set to 0)"
> 
> It seems like the Table claims that using RTAS_CHANGE_FN with 0, could
> change only MSI or MSI-X and still be not a bug?

Yeah I guess you could read it that way, though I think that would be a
bug.

The idea is that it chooses for you whether it uses MSI or MSI-X. So the
only sane semantic is that when deconfiguring it deconfigures either,
ie. both, kinds.

Looking closer at your patch, now I don't understand :)

+       /*
+        * disabling MSI with the explicit interface also disables MSI-X
+        */
+       if (rtas_change_msi(pdn, RTAS_CHANGE_MSI_FN, 0) != 0) {


So we first disable using function 3, which should:

        3: Request to set to a new number of MSI interrupts (including set to 0)

Which does not mention MSI-X at all, implying it has no effect on them.
Which contradicts what you see, and the comment in the code?

So I think I'm not sure what's going on here :)

cheers

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]