Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 01:46:20PM -0400, John W. Linville wrote: > On Sat, Jul 02, 2005 at 01:29:54AM -0600, Grant Grundler wrote: > > On Thu, Jun 30, 2005 at 10:26:37PM -0400, John W. Linville wrote: > > > > + /* Some devices lose PCI config header data during D3hot->D0 > > > > Can you name some of those devices here? > > I just want to know what sort of devices need to be tested > > if this code changes in the future. > > I don't really have a list. The devices that brought this issue to > my attention are a 3c905B and a 3c556B, both covered by the 3c59x > driver. John, apologies for the late reply - been offline the past two weeks on holiday. Just listing the two devices in a comment would be sufficient. > According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT INTERFACE > SPECIFICATION, REV. 1.2", a device transitioning from D3hot to D0 > _may_ perform an internal reset, thereby going to "D0 Uninitialized" > rather than "D0 Initialized". Including the above paragraph in a comment would be a good thing. I don't know if this spec is publicly available. But even if it is, typically only a handful of people will be familiar enough with it to know where to look in it. > Since this behaviour is ratified by > the spec, I think we need to accomodate it. Yes - sounds reasonable to me too. > A bit in the PMCSR register indicates how a device will behave in > this regard. We could have a test to only execute the BAR restoration > for those devices that seem to need it. I left that out because it > seemed to add needless complexity. In the meantime the patch has > gotten bigger and more complex, so maybe that code doesn't make it > any worse. Do you want me to add that? I think I'd keep it simpler until someone proves we need it. I've read the rest of the thread and don't recall any such proof. > > > > > > +transition. Since some firmware leaves devices in D3hot > > > +state at boot, this information needs to be restored. > > > > Again, which firmware? > > Examples are good since it makes it possible to track down > > the offending devices for testing. > > The Thinkpad T21 does this. I don't know of any others specifically, > but it seems like something laptop BIOSes would be likely to do. That's fine - just listing the Thinkpad T21 in a comment is helpful. If you happen to know the firmware version too, that would be even better. > > The following chunk looks like it will have issues with 64-bit BARs: > > As RMK pointed-out, this code is inspired by setup-res.c; specifically > that in pci_update_resource. I'd prefer not to blaze any new trails > regarding 64-bit BAR support ATM... :-) After thinking about this more, I'm convinced it's broken if a 64-bit BAR is present on the PCI device. It doesn't matter if the MMIO value is greater than 4GB or not. The problem is pci_dev->resource[i] does NOT map 1:1 with PCI_BASE_ADDRESS_0+(i*4). > So, is the current patch acceptable? I don't think so. 64-bit BARs are just too common today. One solution is to use a seperate variable to track the offset into PCI config space. ie use "i" to walk through pci_dev->resource[] and add "unsigned int pcibar_offset" to keep track of 32 vs 64-bit BARs. > Or shall I add the check for the "no soft reset" bit in the PMCSR register? I don't see why that's necessary. thanks, grant - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 01:46:20PM -0400, John W. Linville wrote: On Sat, Jul 02, 2005 at 01:29:54AM -0600, Grant Grundler wrote: On Thu, Jun 30, 2005 at 10:26:37PM -0400, John W. Linville wrote: + /* Some devices lose PCI config header data during D3hot-D0 Can you name some of those devices here? I just want to know what sort of devices need to be tested if this code changes in the future. I don't really have a list. The devices that brought this issue to my attention are a 3c905B and a 3c556B, both covered by the 3c59x driver. John, apologies for the late reply - been offline the past two weeks on holiday. Just listing the two devices in a comment would be sufficient. According to section 5.4.1 of the PCI BUS POWER MANAGEMENT INTERFACE SPECIFICATION, REV. 1.2, a device transitioning from D3hot to D0 _may_ perform an internal reset, thereby going to D0 Uninitialized rather than D0 Initialized. Including the above paragraph in a comment would be a good thing. I don't know if this spec is publicly available. But even if it is, typically only a handful of people will be familiar enough with it to know where to look in it. Since this behaviour is ratified by the spec, I think we need to accomodate it. Yes - sounds reasonable to me too. A bit in the PMCSR register indicates how a device will behave in this regard. We could have a test to only execute the BAR restoration for those devices that seem to need it. I left that out because it seemed to add needless complexity. In the meantime the patch has gotten bigger and more complex, so maybe that code doesn't make it any worse. Do you want me to add that? I think I'd keep it simpler until someone proves we need it. I've read the rest of the thread and don't recall any such proof. +transition. Since some firmware leaves devices in D3hot +state at boot, this information needs to be restored. Again, which firmware? Examples are good since it makes it possible to track down the offending devices for testing. The Thinkpad T21 does this. I don't know of any others specifically, but it seems like something laptop BIOSes would be likely to do. That's fine - just listing the Thinkpad T21 in a comment is helpful. If you happen to know the firmware version too, that would be even better. The following chunk looks like it will have issues with 64-bit BARs: As RMK pointed-out, this code is inspired by setup-res.c; specifically that in pci_update_resource. I'd prefer not to blaze any new trails regarding 64-bit BAR support ATM... :-) After thinking about this more, I'm convinced it's broken if a 64-bit BAR is present on the PCI device. It doesn't matter if the MMIO value is greater than 4GB or not. The problem is pci_dev-resource[i] does NOT map 1:1 with PCI_BASE_ADDRESS_0+(i*4). So, is the current patch acceptable? I don't think so. 64-bit BARs are just too common today. One solution is to use a seperate variable to track the offset into PCI config space. ie use i to walk through pci_dev-resource[] and add unsigned int pcibar_offset to keep track of 32 vs 64-bit BARs. Or shall I add the check for the no soft reset bit in the PMCSR register? I don't see why that's necessary. thanks, grant - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Fri, Jul 08, 2005 at 12:33:33AM -0700, David S. Miller wrote: > Do PCI devices ever come out of reset in a PM state, and thus > would execute John's new code as a side effect? PCI spec requires that all devices must enter D0 state from power on reset, so this code shouldn't be executed unless you have some really buggy PM firmware (which is unlikely :-). Ivan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: Ivan Kokshaysky <[EMAIL PROTECTED]> Date: Fri, 8 Jul 2005 11:03:58 +0400 > On Thu, Jul 07, 2005 at 11:35:30PM -0700, David S. Miller wrote: > > That's fine, what would be the most minimal implementation? > > #define pci_update_resource(dev, res, n) BUG() > > No, I'm serious - I don't believe that generic implementation of > pcibios_resource_to_bus() in the proposed patch does the right thing > on sparc64 anyway. We really don't do power management on sparc64 at all currently, besides cpufreq, so I guess this would be OK. Do PCI devices ever come out of reset in a PM state, and thus would execute John's new code as a side effect? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Thu, Jul 07, 2005 at 11:35:30PM -0700, David S. Miller wrote: > That's fine, what would be the most minimal implementation? #define pci_update_resource(dev, res, n)BUG() No, I'm serious - I don't believe that generic implementation of pcibios_resource_to_bus() in the proposed patch does the right thing on sparc64 anyway. Ivan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: Ivan Kokshaysky <[EMAIL PROTECTED]> Date: Fri, 8 Jul 2005 09:51:04 +0400 > Why not just implement sparc64 version of pci_update_resource elsewhere > (perhaps a dummy one, if you don't need PCI PM), rather than force the > rest of the world to duplicate the code? That's fine, what would be the most minimal implementation? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: Ivan Kokshaysky [EMAIL PROTECTED] Date: Fri, 8 Jul 2005 09:51:04 +0400 Why not just implement sparc64 version of pci_update_resource elsewhere (perhaps a dummy one, if you don't need PCI PM), rather than force the rest of the world to duplicate the code? That's fine, what would be the most minimal implementation? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Thu, Jul 07, 2005 at 11:35:30PM -0700, David S. Miller wrote: That's fine, what would be the most minimal implementation? #define pci_update_resource(dev, res, n)BUG() No, I'm serious - I don't believe that generic implementation of pcibios_resource_to_bus() in the proposed patch does the right thing on sparc64 anyway. Ivan. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: Ivan Kokshaysky [EMAIL PROTECTED] Date: Fri, 8 Jul 2005 11:03:58 +0400 On Thu, Jul 07, 2005 at 11:35:30PM -0700, David S. Miller wrote: That's fine, what would be the most minimal implementation? #define pci_update_resource(dev, res, n) BUG() No, I'm serious - I don't believe that generic implementation of pcibios_resource_to_bus() in the proposed patch does the right thing on sparc64 anyway. We really don't do power management on sparc64 at all currently, besides cpufreq, so I guess this would be OK. Do PCI devices ever come out of reset in a PM state, and thus would execute John's new code as a side effect? - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Fri, Jul 08, 2005 at 12:33:33AM -0700, David S. Miller wrote: Do PCI devices ever come out of reset in a PM state, and thus would execute John's new code as a side effect? PCI spec requires that all devices must enter D0 state from power on reset, so this code shouldn't be executed unless you have some really buggy PM firmware (which is unlikely :-). Ivan. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Thu, Jul 07, 2005 at 08:11:03PM -0700, David S. Miller wrote: > > Problem: pci_update_resource doesn't exist for sparc64. > > Yes, the drivers/pci/setup-res.c code isn't compiled in on > sparc64 because it assumes a totally different model of > PCI bus probing than we use on sparc64. Why not just implement sparc64 version of pci_update_resource elsewhere (perhaps a dummy one, if you don't need PCI PM), rather than force the rest of the world to duplicate the code? Ivan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: "John W. Linville" <[EMAIL PROTECTED]> Date: Thu, 7 Jul 2005 20:57:04 -0400 > Problem: pci_update_resource doesn't exist for sparc64. Yes, the drivers/pci/setup-res.c code isn't compiled in on sparc64 because it assumes a totally different model of PCI bus probing than we use on sparc64. On sparc64, we check out if the boot firmware has assigned the PCI resources for the device, and if so we just leave things alone. We also make sure that the firmware device tree properties mostly match what the PCI config space registers say and we spit out a warning message if not. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Wed, Jul 06, 2005 at 03:34:54AM +0400, Ivan Kokshaysky wrote: > On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: > > Rather than reimplementing the internals of pci_update_resource() it > > may be worth splitting the common stuff out so it gets fixed for both > > pci_update_resource() and pci_enable_device(). > > Just use pci_update_resource(). Problem: pci_update_resource doesn't exist for sparc64. > John, I'd also suggest following changes to the patch: > - move the code to pci_set_power_state(), where it belongs to; > - explicitly check for D3hot->D0 transition *and* for the > No_Soft_Reset bit, to avoid unnecessary config space accesses; > - add a quote from PCI spec (as a comment) explaining why is it needed. I have reformulated the patch to account for these comments, but I am not currently using pci_update_resource for the reason stated above. I'll go ahead and post the new patch for comment. If we can resolve the pci_update_resource issue, I'll post another (either alternative or additional) patch to cover that. Patch to follow... Thanks! John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Wed, Jul 06, 2005 at 03:34:54AM +0400, Ivan Kokshaysky wrote: On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: Rather than reimplementing the internals of pci_update_resource() it may be worth splitting the common stuff out so it gets fixed for both pci_update_resource() and pci_enable_device(). Just use pci_update_resource(). Problem: pci_update_resource doesn't exist for sparc64. John, I'd also suggest following changes to the patch: - move the code to pci_set_power_state(), where it belongs to; - explicitly check for D3hot-D0 transition *and* for the No_Soft_Reset bit, to avoid unnecessary config space accesses; - add a quote from PCI spec (as a comment) explaining why is it needed. I have reformulated the patch to account for these comments, but I am not currently using pci_update_resource for the reason stated above. I'll go ahead and post the new patch for comment. If we can resolve the pci_update_resource issue, I'll post another (either alternative or additional) patch to cover that. Patch to follow... Thanks! John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
From: John W. Linville [EMAIL PROTECTED] Date: Thu, 7 Jul 2005 20:57:04 -0400 Problem: pci_update_resource doesn't exist for sparc64. Yes, the drivers/pci/setup-res.c code isn't compiled in on sparc64 because it assumes a totally different model of PCI bus probing than we use on sparc64. On sparc64, we check out if the boot firmware has assigned the PCI resources for the device, and if so we just leave things alone. We also make sure that the firmware device tree properties mostly match what the PCI config space registers say and we spit out a warning message if not. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Thu, Jul 07, 2005 at 08:11:03PM -0700, David S. Miller wrote: Problem: pci_update_resource doesn't exist for sparc64. Yes, the drivers/pci/setup-res.c code isn't compiled in on sparc64 because it assumes a totally different model of PCI bus probing than we use on sparc64. Why not just implement sparc64 version of pci_update_resource elsewhere (perhaps a dummy one, if you don't need PCI PM), rather than force the rest of the world to duplicate the code? Ivan. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Wed, Jul 06, 2005 at 03:34:54AM +0400, Ivan Kokshaysky wrote: > On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: > > On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: > > > 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we > > > ought to support 64-bit BARs on 32-bit machines. > > > > This only occurs because the problematical functions (eg, > > pci_update_resource) probably aren't called on 64-bit machines - if > > they were, they'd zero the upper 32-bits. Maybe 64-bit machines are > > happy with that anyway? > > Why problematical? It's just the way how linux has always dealt with > 64-bit BARs - put everything below 4G in the bus address space, on *any* > architecture. I'd be quite surprised if some firmware doesn't do the same > thing - so far I haven't heard any complaints. If this is so, Grant's concern about programming the top half of 64-bit resources with zero isn't appropriate. However, he did raise this as an issue... -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Wed, Jul 06, 2005 at 03:34:54AM +0400, Ivan Kokshaysky wrote: On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we ought to support 64-bit BARs on 32-bit machines. This only occurs because the problematical functions (eg, pci_update_resource) probably aren't called on 64-bit machines - if they were, they'd zero the upper 32-bits. Maybe 64-bit machines are happy with that anyway? Why problematical? It's just the way how linux has always dealt with 64-bit BARs - put everything below 4G in the bus address space, on *any* architecture. I'd be quite surprised if some firmware doesn't do the same thing - so far I haven't heard any complaints. If this is so, Grant's concern about programming the top half of 64-bit resources with zero isn't appropriate. However, he did raise this as an issue... -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: > On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: > > 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we > > ought to support 64-bit BARs on 32-bit machines. > > This only occurs because the problematical functions (eg, > pci_update_resource) probably aren't called on 64-bit machines - if > they were, they'd zero the upper 32-bits. Maybe 64-bit machines are > happy with that anyway? Why problematical? It's just the way how linux has always dealt with 64-bit BARs - put everything below 4G in the bus address space, on *any* architecture. I'd be quite surprised if some firmware doesn't do the same thing - so far I haven't heard any complaints. Eventually we'll have to support MMIO above 4G, but I suspect this won't be necessary at least in a next couple of years. Anyway, there are no fundamental changes required for the generic PCI layer to handle that, just some tweaks here and there - almost all non-trivial stuff will be arch-specific. > Rather than reimplementing the internals of pci_update_resource() it > may be worth splitting the common stuff out so it gets fixed for both > pci_update_resource() and pci_enable_device(). Just use pci_update_resource(). John, I'd also suggest following changes to the patch: - move the code to pci_set_power_state(), where it belongs to; - explicitly check for D3hot->D0 transition *and* for the No_Soft_Reset bit, to avoid unnecessary config space accesses; - add a quote from PCI spec (as a comment) explaining why is it needed. Ivan. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: > On Sat, Jul 02, 2005 at 09:09:13AM +0100, Russell King wrote: > > The PCI subsystem is incomplete for 64-bit BAR support. What it does > > do though is ensure that 64-bit BARs will work correctly in a 32-bit > > system. Therefore, I think that folk who want 64-bit BAR support to > > work need to do some code reviews on the PCI subsystem to resolve the > > remaining issues. > > 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we > ought to support 64-bit BARs on 32-bit machines. This only occurs because the problematical functions (eg, pci_update_resource) probably aren't called on 64-bit machines - if they were, they'd zero the upper 32-bits. Maybe 64-bit machines are happy with that anyway? Rather than reimplementing the internals of pci_update_resource() it may be worth splitting the common stuff out so it gets fixed for both pci_update_resource() and pci_enable_device(). -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Sat, Jul 02, 2005 at 09:09:13AM +0100, Russell King wrote: > The PCI subsystem is incomplete for 64-bit BAR support. What it does > do though is ensure that 64-bit BARs will work correctly in a 32-bit > system. Therefore, I think that folk who want 64-bit BAR support to > work need to do some code reviews on the PCI subsystem to resolve the > remaining issues. 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we ought to support 64-bit BARs on 32-bit machines. -- "Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception." -- Mark Twain - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Sat, Jul 02, 2005 at 01:29:54AM -0600, Grant Grundler wrote: > On Thu, Jun 30, 2005 at 10:26:37PM -0400, John W. Linville wrote: > > + /* Some devices lose PCI config header data during D3hot->D0 > > Can you name some of those devices here? > I just want to know what sort of devices need to be tested > if this code changes in the future. I don't really have a list. The devices that brought this issue to my attention are a 3c905B and a 3c556B, both covered by the 3c59x driver. According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT INTERFACE SPECIFICATION, REV. 1.2", a device transitioning from D3hot to D0 _may_ perform an internal reset, thereby going to "D0 Uninitialized" rather than "D0 Initialized". Since this behaviour is ratified by the spec, I think we need to accomodate it. A bit in the PMCSR register indicates how a device will behave in this regard. We could have a test to only execute the BAR restoration for those devices that seem to need it. I left that out because it seemed to add needless complexity. In the meantime the patch has gotten bigger and more complex, so maybe that code doesn't make it any worse. Do you want me to add that? > > > + transition. Since some firmware leaves devices in D3hot > > + state at boot, this information needs to be restored. > > Again, which firmware? > Examples are good since it makes it possible to track down > the offending devices for testing. The Thinkpad T21 does this. I don't know of any others specifically, but it seems like something laptop BIOSes would be likely to do. > The following chunk looks like it will have issues with 64-bit BARs: As RMK pointed-out, this code is inspired by setup-res.c; specifically that in pci_update_resource. I'd prefer not to blaze any new trails regarding 64-bit BAR support ATM... :-) So, is the current patch acceptable? Or shall I add the check for the "no soft reset" bit in the PMCSR register? Thanks, John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Sat, Jul 02, 2005 at 01:29:54AM -0600, Grant Grundler wrote: On Thu, Jun 30, 2005 at 10:26:37PM -0400, John W. Linville wrote: + /* Some devices lose PCI config header data during D3hot-D0 Can you name some of those devices here? I just want to know what sort of devices need to be tested if this code changes in the future. I don't really have a list. The devices that brought this issue to my attention are a 3c905B and a 3c556B, both covered by the 3c59x driver. According to section 5.4.1 of the PCI BUS POWER MANAGEMENT INTERFACE SPECIFICATION, REV. 1.2, a device transitioning from D3hot to D0 _may_ perform an internal reset, thereby going to D0 Uninitialized rather than D0 Initialized. Since this behaviour is ratified by the spec, I think we need to accomodate it. A bit in the PMCSR register indicates how a device will behave in this regard. We could have a test to only execute the BAR restoration for those devices that seem to need it. I left that out because it seemed to add needless complexity. In the meantime the patch has gotten bigger and more complex, so maybe that code doesn't make it any worse. Do you want me to add that? + transition. Since some firmware leaves devices in D3hot + state at boot, this information needs to be restored. Again, which firmware? Examples are good since it makes it possible to track down the offending devices for testing. The Thinkpad T21 does this. I don't know of any others specifically, but it seems like something laptop BIOSes would be likely to do. The following chunk looks like it will have issues with 64-bit BARs: As RMK pointed-out, this code is inspired by setup-res.c; specifically that in pci_update_resource. I'd prefer not to blaze any new trails regarding 64-bit BAR support ATM... :-) So, is the current patch acceptable? Or shall I add the check for the no soft reset bit in the PMCSR register? Thanks, John -- John W. Linville [EMAIL PROTECTED] - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Sat, Jul 02, 2005 at 09:09:13AM +0100, Russell King wrote: The PCI subsystem is incomplete for 64-bit BAR support. What it does do though is ensure that 64-bit BARs will work correctly in a 32-bit system. Therefore, I think that folk who want 64-bit BAR support to work need to do some code reviews on the PCI subsystem to resolve the remaining issues. 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we ought to support 64-bit BARs on 32-bit machines. -- Next the statesmen will invent cheap lies, putting the blame upon the nation that is attacked, and every man will be glad of those conscience-soothing falsities, and will diligently study them, and refuse to examine any refutations of them; and thus he will by and by convince himself that the war is just, and will thank God for the better sleep he enjoys after this process of grotesque self-deception. -- Mark Twain - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: On Sat, Jul 02, 2005 at 09:09:13AM +0100, Russell King wrote: The PCI subsystem is incomplete for 64-bit BAR support. What it does do though is ensure that 64-bit BARs will work correctly in a 32-bit system. Therefore, I think that folk who want 64-bit BAR support to work need to do some code reviews on the PCI subsystem to resolve the remaining issues. 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we ought to support 64-bit BARs on 32-bit machines. This only occurs because the problematical functions (eg, pci_update_resource) probably aren't called on 64-bit machines - if they were, they'd zero the upper 32-bits. Maybe 64-bit machines are happy with that anyway? Rather than reimplementing the internals of pci_update_resource() it may be worth splitting the common stuff out so it gets fixed for both pci_update_resource() and pci_enable_device(). -- Russell King Linux kernel2.6 ARM Linux - http://www.arm.linux.org.uk/ maintainer of: 2.6 Serial core - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [patch 2.6.12 (repost w/ corrected subject)] pci: restore BAR values in pci_enable_device_bars
On Tue, Jul 05, 2005 at 10:46:20PM +0100, Russell King wrote: On Tue, Jul 05, 2005 at 09:05:55PM +0100, Matthew Wilcox wrote: 64-bit BARs work fine on 64-bit machines. I'm ambivalent whether we ought to support 64-bit BARs on 32-bit machines. This only occurs because the problematical functions (eg, pci_update_resource) probably aren't called on 64-bit machines - if they were, they'd zero the upper 32-bits. Maybe 64-bit machines are happy with that anyway? Why problematical? It's just the way how linux has always dealt with 64-bit BARs - put everything below 4G in the bus address space, on *any* architecture. I'd be quite surprised if some firmware doesn't do the same thing - so far I haven't heard any complaints. Eventually we'll have to support MMIO above 4G, but I suspect this won't be necessary at least in a next couple of years. Anyway, there are no fundamental changes required for the generic PCI layer to handle that, just some tweaks here and there - almost all non-trivial stuff will be arch-specific. Rather than reimplementing the internals of pci_update_resource() it may be worth splitting the common stuff out so it gets fixed for both pci_update_resource() and pci_enable_device(). Just use pci_update_resource(). John, I'd also suggest following changes to the patch: - move the code to pci_set_power_state(), where it belongs to; - explicitly check for D3hot-D0 transition *and* for the No_Soft_Reset bit, to avoid unnecessary config space accesses; - add a quote from PCI spec (as a comment) explaining why is it needed. Ivan. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
