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

[Nishanth Aravamudan <nacc@us.ibm.com>]

On 04.03.2011 [14:01:24 +1100], Michael Ellerman wrote:
> 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.

I agree with you that is how it should be :) I'm asking the firmware
folks to make sure I'm not misunderstanding the underlying issue.

> 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?

Thanks for the thorough review!

Per PAPR 2.4 from Power.org, look at the page before that table, page
169:

"Specifying Function 3 (MSI) also disables MSI-X for the specified IOA
function, and likewise specifying Function 4 (MSI-X) disables MSI for
the IOA function....Specifying the Requested Number of Interrupts to
zero for either Function 3 or 4 removes all MSI & MSI-X interrupts from
the IOA function."

So I'm relying on this aspect of PAPR being enforced by the firmware,
which I think it is in my testing.

Thanks,
Nish