Re: [BUG] powerpc does not save msi state [was Re: [PATCH 5/7] pci: Export the pci_restore_msi_state() function
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
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
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
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
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
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
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
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
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
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
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
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
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