Subject: Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

From: Michael Ellerman <[email-masked]>
Date: Tue, 23 Oct 2007 14:20:46 +1000

On Mon, 2007-10-22 at 13:13 -0500, Linas Vepstas wrote:
> On Mon, Oct 22, 2007 at 11:49:24AM +1000, Michael Ellerman wrote:
> > 
> > On pseries there's a chance it will work for PCI error recovery, but if
> > so it's just lucky that firmware has left everything configured the same
> > way. 
> 
> ? The papr is quite clear that i is up to the OS to restore the msi
> state after an eeh error.

Perhaps. I see R1-7.3.10.5.1-10, which says we should restore the config
space after a reset operation, but after an isolate/unisolate we must
recall RTAS. I thought EEH was doing the isolate/unisolate, but if it's
just a reset then just blatting the config space back should work.

> > Yes I think so. That way we can properly reconfigure via the firmware
> > interface. The other option would be to design some new arch hook to do
> > resume, but just doing a disable/enable seems simpler to me.
> 
> Err, If you read the code for suspend/resume, it never actually calls
> disable/enable (and thus doesn't go to the firmware); it calls 
> restore_msi_state() function!

I know. That was a proposed solution, to explicitly call disable/enable
instead of restore_msi_state().

> If suspend/resume needs to call firmware to restore the state, then,
> at the moment, suspend/resume is broken.  As I mentioned earlier,
> I presumed that no powerpc laptops currently use msi-enabled devices,
> as otherwise, this would have been flushed out.

On _pseries_ suspend/resume with MSI is broken. The other powerpc
platforms that implement MSI should be fine, although I don't think
anyone's tested them, because they're not laptops and so suspend/resume
is not that interesting.

cheers

-- 
Michael Ellerman
OzLabs, IBM Australia Development Lab

wwweb: http://michael.ellerman.id.au
phone: +61 2 6212 1183 (tie line 70 21183)

We do not inherit the earth from our ancestors,
we borrow it from our children. - S.M.A.R.T Person