* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
[not found] ` <20071021.162131.43417026.davem@davemloft.net>
@ 2007-10-22 1:49 ` Michael Ellerman
2007-10-22 18:13 ` Linas Vepstas
0 siblings, 1 reply; 6+ messages in thread
From: Michael Ellerman @ 2007-10-22 1:49 UTC (permalink / raw)
To: David Miller; +Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci
[-- Attachment #1: Type: text/plain, Size: 1345 bytes --]
On Sun, 2007-10-21 at 16:21 -0700, David Miller wrote:
> From: "Matt Carlson" <mcarlson@broadcom.com>
> Date: Fri, 19 Oct 2007 14:36:56 -0700
>
> > This patch exports the pci_restore_msi_state() function. This function
> > is needed to restore the MSI state during PCI error recovery.
> >
> > Signed-off-by: Linas Vepstas <linas@austin.ibm.com>
> > Signed-off-by: Matt Carlson <mcarlson@broadcom.com>
> > Signed-off-by: Michael Chan <mchan@broadcom.com>
>
> I'm not so sure about this.
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. For actual suspend/resume it will never work, we need to ask
firmware to configure things.
> Perhaps, instead, you should do a pci_msi_disable() and
> pci_msi_enable() in the error detection and recovery sequence.
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.
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
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
2007-10-22 1:49 ` [PATCH 5/7] pci: Export the pci_restore_msi_state() function Michael Ellerman
@ 2007-10-22 18:13 ` Linas Vepstas
2007-10-22 21:24 ` Benjamin Herrenschmidt
2007-10-23 4:20 ` Michael Ellerman
0 siblings, 2 replies; 6+ messages in thread
From: Linas Vepstas @ 2007-10-22 18:13 UTC (permalink / raw)
To: Michael Ellerman
Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
David Miller
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.
> 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!
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.
--linas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
2007-10-22 18:13 ` Linas Vepstas
@ 2007-10-22 21:24 ` Benjamin Herrenschmidt
2007-10-23 0:13 ` Linas Vepstas
2007-10-23 4:20 ` Michael Ellerman
1 sibling, 1 reply; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-22 21:24 UTC (permalink / raw)
To: Linas Vepstas
Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
David Miller
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.
Via direct config space access or via firmware change-msi calls ?
> > 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!
>
> 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.
I don't know why you keep talking about powerpc laptops here ...
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
2007-10-22 21:24 ` Benjamin Herrenschmidt
@ 2007-10-23 0:13 ` Linas Vepstas
2007-10-23 0:29 ` Benjamin Herrenschmidt
0 siblings, 1 reply; 6+ messages in thread
From: Linas Vepstas @ 2007-10-23 0:13 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
David Miller
On Tue, Oct 23, 2007 at 07:24:27AM +1000, Benjamin Herrenschmidt wrote:
>
> 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.
>
> Via direct config space access or via firmware change-msi calls ?
Direct config space access. It says that the OS is supposed to read the
MSI config (after its been set up), save it, and restore it, (via direct
config space writes) if the device is ever reset.
> I don't know why you keep talking about powerpc laptops here ...
Well, there are Apple laptops, right? Aren't those the "powermac"
platform? Now, I don't know if they support MSI, but if they do,
I get the impression that they might not restore msi state correctly,
after being put into hardware suspend. But perhaps I'm mistaken;
I was simply grepping for various msi-related functions in various
arch subdirectories, comparing x86 to other arches, and noticed
that code that would restore msi state seems to be missing for
most arches and most powerpc platforms.
--linas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
2007-10-23 0:13 ` Linas Vepstas
@ 2007-10-23 0:29 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-23 0:29 UTC (permalink / raw)
To: Linas Vepstas
Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
David Miller
> > I don't know why you keep talking about powerpc laptops here ...
>
> Well, there are Apple laptops, right? Aren't those the "powermac"
> platform? Now, I don't know if they support MSI, but if they do,
> I get the impression that they might not restore msi state correctly,
> after being put into hardware suspend. But perhaps I'm mistaken;
> I was simply grepping for various msi-related functions in various
> arch subdirectories, comparing x86 to other arches, and noticed
> that code that would restore msi state seems to be missing for
> most arches and most powerpc platforms.
Ah ok, i see. Well, platforms that use write_msi_msg() shouldn't need
anything special right ? So only pSeries is an issue here....
PowerBooks don't indeed have MSI support, though G5's do and some people
have been toying around with suspend/resume on them (hibernation only at
that stage) but it doesn't matter at this stage. We are specifically
talking about pSeries which is the "special" case here.
Ben.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
2007-10-22 18:13 ` Linas Vepstas
2007-10-22 21:24 ` Benjamin Herrenschmidt
@ 2007-10-23 4:20 ` Michael Ellerman
1 sibling, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2007-10-23 4:20 UTC (permalink / raw)
To: Linas Vepstas
Cc: netdev, mcarlson, linuxppc-dev list, mchan, linux-pci,
David Miller
[-- Attachment #1: Type: text/plain, Size: 1962 bytes --]
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
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2007-10-23 4:20 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1192829817.22064.559.camel@teletran1>
[not found] ` <20071021.162131.43417026.davem@davemloft.net>
2007-10-22 1:49 ` [PATCH 5/7] pci: Export the pci_restore_msi_state() function Michael Ellerman
2007-10-22 18:13 ` Linas Vepstas
2007-10-22 21:24 ` Benjamin Herrenschmidt
2007-10-23 0:13 ` Linas Vepstas
2007-10-23 0:29 ` Benjamin Herrenschmidt
2007-10-23 4:20 ` Michael Ellerman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).