Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-22 Thread Linas Vepstas
On Fri, Oct 19, 2007 at 05:53:08PM -0700, David Miller wrote:
 From: [EMAIL PROTECTED] (Linas Vepstas)
 Date: Fri, 19 Oct 2007 19:46:10 -0500
 
  FWIW, it looks like not all that many arches do this; the output
  for grep -r address_hi * is pretty thin. Then, looking at
  i386/kernel/io_apic.c as an example, one can see that the 
  msi state save happens by accident if CONFIG_SMP is enabled;
  and so its surely broekn on uniprocesor machines.
 
 I don't see this, in all cases write_msi_msg() will transfer
 the given *msg to entry-msg by this assignment in
 drivers/pci/msi.c:
 
 void write_msi_msg(unsigned int irq, struct msi_msg *msg)
 {
  ...
   entry-msg = *msg;
 }
 
 So as long as write_msi_msg() is invoked, it will be saved
 properly.

As Michael Ellerman points out, the pseries msi setup is done
by firmware, and so this bit never happens. 

As discussed in the other thread, I'll try to set up a patch
for an arch callback for restoring msi state.

-linas
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2007-10-22 Thread Benjamin Herrenschmidt

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.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2007-10-22 Thread Linas Vepstas
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

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-22 Thread David Miller
From: [EMAIL PROTECTED] (Linas Vepstas)
Date: Mon, 22 Oct 2007 14:54:52 -0500

 As discussed in the other thread, I'll try to set up a patch
 for an arch callback for restoring msi state.

Thank you.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2007-10-22 Thread Benjamin Herrenschmidt

  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.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-22 Thread Benjamin Herrenschmidt

On Mon, 2007-10-22 at 17:23 -0700, David Miller wrote:
 From: [EMAIL PROTECTED] (Linas Vepstas)
 Date: Mon, 22 Oct 2007 14:54:52 -0500
 
  As discussed in the other thread, I'll try to set up a patch
  for an arch callback for restoring msi state.

From what it looks like at this stage, pSeries might need to
differenciate restoring MSI state after a device reset (PCI error
recovery) from restoring MSI state after suspend/resume (if we ever
implement that one).

The former apparently require manual saving  restoring of the config
space bits. (Linas, do you have a pointer to the bit of PAPR spec that
specifies that we need to save  restore the MSI message ourselves ?)

For the later (suspend/resume), that will definitely not work, or at
least, will not be enough, especially with something like suspend to
disk, where we'll need to have the firmware reconfigure the MSIs for us
(to make sure, among others, that the interrupt controllers are properly
configured for MSIs etc...).

Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


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

2007-10-22 Thread Michael Ellerman

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


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-21 Thread Benjamin Herrenschmidt

 That's a pity, but AFAIK it shouldn't be a problem because we don't
 enable CONFIG_PM on those machines anyway. If we ever want to we'll need
 to sort out with firmware how that will work WRT restoring MSI state.

I think the current generic code for pci_restore_msi_state() or whatever
it's called wilol directly call into write_msi_msg() etc... might be a
problem.

Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-20 Thread Michael Ellerman

On Fri, 2007-10-19 at 17:53 -0700, David Miller wrote:
 From: [EMAIL PROTECTED] (Linas Vepstas)
 Date: Fri, 19 Oct 2007 19:46:10 -0500
 
  FWIW, it looks like not all that many arches do this; the output
  for grep -r address_hi * is pretty thin. Then, looking at
  i386/kernel/io_apic.c as an example, one can see that the 
  msi state save happens by accident if CONFIG_SMP is enabled;
  and so its surely broekn on uniprocesor machines.
 
 I don't see this, in all cases write_msi_msg() will transfer
 the given *msg to entry-msg by this assignment in
 drivers/pci/msi.c:
 
 void write_msi_msg(unsigned int irq, struct msi_msg *msg)
 {
  ...
   entry-msg = *msg;
 }
 
 So as long as write_msi_msg() is invoked, it will be saved
 properly.
 
 Platforms need not do this explicitly.

I'm short on context here, and it's Saturday, so excuse me if I'm
missing the point somewhere.

On pseries machines we don't call write_msi_msg(), because we don't
control the contents of the message, firmware does. So entry-msg will
be bogus.

That's a pity, but AFAIK it shouldn't be a problem because we don't
enable CONFIG_PM on those machines anyway. If we ever want to we'll need
to sort out with firmware how that will work WRT restoring MSI state.

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


signature.asc
Description: This is a digitally signed message part
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-20 Thread Michael Chan
On Sat, 2007-10-20 at 16:43 +1000, Michael Ellerman wrote:
 On Fri, 2007-10-19 at 17:53 -0700, David Miller wrote:
  I don't see this, in all cases write_msi_msg() will transfer
  the given *msg to entry-msg by this assignment in
  drivers/pci/msi.c:
  
  void write_msi_msg(unsigned int irq, struct msi_msg *msg)
  {
   ...
  entry-msg = *msg;
  }
  
  So as long as write_msi_msg() is invoked, it will be saved
  properly.
  
  Platforms need not do this explicitly.
 
 I'm short on context here, and it's Saturday, so excuse me if I'm
 missing the point somewhere.
 
 On pseries machines we don't call write_msi_msg(), because we don't
 control the contents of the message, firmware does. So entry-msg will
 be bogus.
 
 That's a pity, but AFAIK it shouldn't be a problem because we don't
 enable CONFIG_PM on those machines anyway. If we ever want to we'll need
 to sort out with firmware how that will work WRT restoring MSI state.
 

The PCI error recovery that Linas is working on requires the MSI state
to be restored after we do PCI reset to recover from PCI errors.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-19 Thread David Miller
From: [EMAIL PROTECTED] (Linas Vepstas)
Date: Fri, 19 Oct 2007 19:46:10 -0500

 FWIW, it looks like not all that many arches do this; the output
 for grep -r address_hi * is pretty thin. Then, looking at
 i386/kernel/io_apic.c as an example, one can see that the 
 msi state save happens by accident if CONFIG_SMP is enabled;
 and so its surely broekn on uniprocesor machines.

I don't see this, in all cases write_msi_msg() will transfer
the given *msg to entry-msg by this assignment in
drivers/pci/msi.c:

void write_msi_msg(unsigned int irq, struct msi_msg *msg)
{
 ...
entry-msg = *msg;
}

So as long as write_msi_msg() is invoked, it will be saved
properly.

Platforms need not do this explicitly.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


[BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-19 Thread Linas Vepstas
Hi,

On Fri, Oct 19, 2007 at 05:27:06PM -0700, David Miller wrote:
 From: [EMAIL PROTECTED] (Linas Vepstas)
 Date: Fri, 19 Oct 2007 19:04:21 -0500
 
  I'm working in linux-2.6.23-rc8-mm1 at the moment, and I don't see
  that happening. viz. read_msi_msg() is not called anywhere, and I need
  to have valid msg-address_lo and msg-address_hi and msg-data
  in order to be able to restore.
 
 See the pci_restore_msi_state() call done from pci_restore_state()
 in drivers/pci/pci.c, that pci_restore_msi_state() code in
 drivers/pci/msi.c very much relies upon the entry-msg values
 being uptodate and valid.
 
 The MSI arch layer code is supposed to fill the entry-msg values in
 via arch_setup_msi_irq().  Perhaps the pseries code is forgetting to
 do that.

Yep.  Thank you for confirming the correct location for the fix.

FWIW, it looks like not all that many arches do this; the output
for grep -r address_hi * is pretty thin. Then, looking at
i386/kernel/io_apic.c as an example, one can see that the 
msi state save happens by accident if CONFIG_SMP is enabled;
and so its surely broekn on uniprocesor machines.

I'm cc'ing the powerpc mailing list to point this out: 
it looks like only cell/axon_msi.c and mpic_u3msi.c 
bother do do anything.  I guess that there aren't any old 
macintosh laptops that have msi on them? Because without
this, suspend and resume breaks.

Paul,
On the off chance your reading this, I'll send a pseries
patch on Monday, with luck (and some other patches too).
I'm not touching any of the other plaforms, you and benh 
would know those better.

--linas
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function

2007-10-19 Thread Benjamin Herrenschmidt

 I'm cc'ing the powerpc mailing list to point this out: 
 it looks like only cell/axon_msi.c and mpic_u3msi.c 
 bother do do anything.  I guess that there aren't any old 
 macintosh laptops that have msi on them? Because without
 this, suspend and resume breaks.

The only macs that can do any form of MSIs are the G5s using
mpic_u3msi.c

 Paul,
 On the off chance your reading this, I'll send a pseries
 patch on Monday, with luck (and some other patches too).
 I'm not touching any of the other plaforms, you and benh 
 would know those better.

Or rather Michael as he wrote the ppc MSI support. I'll check with him

Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev