Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Wed, Feb 02, 2005 at 09:33:06AM -0600, Brian King wrote: > Matthew Wilcox wrote: > >On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: > > > >>>If we've done a write to config space while the adapter was blocked, > >>>shouldn't we replay those accesses at this point? > >> > >>I did not think that was necessary. > > > > > >We have to do *something*. We can't just throw away writes. > > > >I see a few options: > > > > - Log all pending writes to config space and replay the log when the > > device is unblocked. > > - Fail writes to config space while the device is blocked. > > - Write to the saved config space and then blat the saved config space > > back to the device upon unblocking. > > Here is an updated patch which will now fail writes to config space > while the device is blocked. I have also fixed up the caching to return > the correct data and tested it on both little endian and big endian > machines. Applied, thanks. greg k-h - 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 1/1] pci: Block config access during BIST (resend)
On Wed, Feb 02, 2005 at 09:33:06AM -0600, Brian King wrote: Matthew Wilcox wrote: On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? I did not think that was necessary. We have to do *something*. We can't just throw away writes. I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. - Fail writes to config space while the device is blocked. - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Here is an updated patch which will now fail writes to config space while the device is blocked. I have also fixed up the caching to return the correct data and tested it on both little endian and big endian machines. Applied, thanks. greg k-h - 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 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote:
On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote:
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
I did not think that was necessary.
We have to do *something*. We can't just throw away writes.
I see a few options:
- Log all pending writes to config space and replay the log when the
device is unblocked.
- Fail writes to config space while the device is blocked.
- Write to the saved config space and then blat the saved config space
back to the device upon unblocking.
Here is an updated patch which will now fail writes to config space
while the device is blocked. I have also fixed up the caching to return
the correct data and tested it on both little endian and big endian
machines.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c| 86 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h |7 +
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c | 30 +++---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c | 14 +--
linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h |7 +
6 files changed, 127 insertions(+), 27 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc2-bk9/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-02-02 08:58:53.0 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-02
09:15:30.0 -0600
@@ -60,3 +60,89 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+static u32 pci_user_cached_config(struct pci_dev *dev, int pos)
+{
+ u32 data;
+
+ data = dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])];
+ data >>= (pos % sizeof(dev->saved_config_space[0])) * 8;
+ return data;
+}
+
+#define PCI_USER_READ_CONFIG(size,type)
\
+int pci_user_read_config_##size
\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))\
+ ret = dev->bus->ops->read(dev->bus, dev->devfn, \
+ pos, sizeof(type), ); \
+ else if (pos < sizeof(dev->saved_config_space)) \
+ data = pci_user_cached_config(dev, pos);\
+ spin_unlock_irqrestore(_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = -EIO; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))\
+ ret = dev->bus->ops->write(dev->bus, dev->devfn,\
+
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote:
On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote:
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
I did not think that was necessary.
We have to do *something*. We can't just throw away writes.
I see a few options:
- Log all pending writes to config space and replay the log when the
device is unblocked.
- Fail writes to config space while the device is blocked.
- Write to the saved config space and then blat the saved config space
back to the device upon unblocking.
Here is an updated patch which will now fail writes to config space
while the device is blocked. I have also fixed up the caching to return
the correct data and tested it on both little endian and big endian
machines.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c| 86 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h |7 +
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c | 30 +++---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c | 14 +--
linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h |7 +
6 files changed, 127 insertions(+), 27 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc2-bk9/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-02-02 08:58:53.0 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-02
09:15:30.0 -0600
@@ -60,3 +60,89 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+static u32 pci_user_cached_config(struct pci_dev *dev, int pos)
+{
+ u32 data;
+
+ data = dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])];
+ data = (pos % sizeof(dev-saved_config_space[0])) * 8;
+ return data;
+}
+
+#define PCI_USER_READ_CONFIG(size,type)
\
+int pci_user_read_config_##size
\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, \
+ pos, sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data = pci_user_cached_config(dev, pos);\
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = -EIO; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-write(dev-bus, dev-devfn,\
+
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 10:58 -0800, Greg KH wrote: > > If we've done a write to config space while the adapter was blocked, > > shouldn't we replay those accesses at this point? > > This has been discussed a lot already. I think we might as well let the > thing fail in random and odd ways, as it's some pretty broken hardware > anyway that wants this functionality :) I don't agree that it's broken hardware, especially in the case where we use these for power managed devices (on pmac, or eventually embedded, where we can have PM be as drastic as completely removing power/clock from a device or that sort of thing). But as I wrote earlier, I consider that in those cases, userland has no business writing to config space. If it does, then it has to be aware of the device beeing potentially offlined or that sort of thing and have proper interface to the kernel driver etc... The reads coming from the cache, on the other hand, have a more legitimate use which are to let - distro HW probing tool to actually see devices even when they are power managed or BIST by their kernel driver - crap like XFree86 to have a proper idea of what address ranges are actually used on the bus, including devices that are power managed or BIST (*). Ben. (*) An unrelated note: It's not always legal to allocate thigns in PCI space or move devices around anyway. It's definitely not on logically partitionned PPC machines where HV won't let a partition access other IO ranges than the ones where the "allowed" devices for this partition were intially put by the firmware... Again, X should really be changed to just stop doing anything but "probing" on the bus at least with Linux. The only problem is the VGA enable story, but as we discussed earlier, that too should be dealt with in the kernel. I think we could integrate that cleanly in sysfs for PCI devices in the generic code btw. Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 17:47 +, Matthew Wilcox wrote: > On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: > > >If we've done a write to config space while the adapter was blocked, > > >shouldn't we replay those accesses at this point? > > > > I did not think that was necessary. > > We have to do *something*. We can't just throw away writes. I think we can in fact. Again, nobody outside of the driver has legitimacy to write to the config space of a device, especially if the device is "unreachable" (either doing a BIST or power managed). > I see a few options: > > - Log all pending writes to config space and replay the log when the >device is unblocked. > - Fail writes to config space while the device is blocked. I agree that returning an error in this case would be a good idea. > - Write to the saved config space and then blat the saved config space >back to the device upon unblocking. > > Any other ideas? > > BTW, you know things like XFree86 go completely around the kernel's PCI > accessors and poke at config space directly? Not anymore afaik. They use /proc/bus/pci Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 15:44 +, Matthew Wilcox wrote:
>
> > +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(_lock, flags);
> > + dev->block_ucfg_access = 0;
> > + spin_unlock_irqrestore(_lock, flags);
> > +}
>
> If we've done a write to config space while the adapter was blocked,
> shouldn't we replay those accesses at this point?
I think no. In fact, I would be ok returning errors on writes from
userland. Need to do config space writes from userland is rare, must
more than reads, and if whatever does it can't properly arbitrate with
what's going on in the kernel, then it's broken.
Ben.
-
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 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote: On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? I did not think that was necessary. We have to do *something*. We can't just throw away writes. I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. This would need to be dynamic in size, as a device could be blocked for a long time. In the scenario that this is used for power management and the device could be blocked for a long time, its unclear that we would still want all the writes accumulated to be written out when the device becomes unblocked. - Fail writes to config space while the device is blocked. This would be nice and simple. I know Alan had some issue with returning failures on reads when blocked. Alan - do you have the same issue on writes? - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Would also be pretty simple to do and seems a little safer than potentially assaulting the recently unblocked device with who knows what values. The only problem I see with this is that we could end up returning strange values on cached reads if the writes update the cache. If userspace wrote to a read only register, we would end up returning that value on the read, which may not be the right thing to do. Any other ideas? We could go back to Alan's idea of putting userspace reads/writes to sleep when the device is blocked, although this has additional complications as well... BTW, you know things like XFree86 go completely around the kernel's PCI accessors and poke at config space directly? The purpose of this API is to provide a way for the kernel to stop userspace from accessing PCI devices when the results of doing so would be catastrophic, such as a PCI bus error. The only users of this API so far are device drivers running BIST on an adapter (which shouldn't happen on a video card AFAIK) and for PPC power management (Ben - will this be an issue for you?) -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 03:44:00PM +, Matthew Wilcox wrote:
> On Tue, Feb 01, 2005 at 09:12:56AM -0600, Brian King wrote:
> > Greg KH wrote:
> > >On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
> > >>+void pci_block_user_cfg_access(struct pci_dev *dev)
> > >>+{
> > >>+ unsigned long flags;
> > >>+
> > >>+ pci_save_state(dev);
> > >>+ spin_lock_irqsave(_lock, flags);
> > >>+ dev->block_ucfg_access = 1;
> > >>+ spin_unlock_irqrestore(_lock, flags);
> > >>+}
> > >>+EXPORT_SYMBOL(pci_block_user_cfg_access);
> > >
> > >
> > >EXPORT_SYMBOL_GPL() please?
> >
> > Ok.
>
> I'm not entirely convinced these should be GPL-only exports. Basically,
> this is saying that any driver for a device that has this problem must be
> GPLd. I think that's a firmer stance than Linus had in mind originally.
"originally"? These are new functions added to the PCI core. I think
that any driver that wants to use this functionality had better be
released under the GPL as what they are wanting to do is a "new" thing.
That's why I prefer all new exports to be marked _GPL.
thanks,
greg k-h
> > +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> > +{
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(_lock, flags);
> > + dev->block_ucfg_access = 0;
> > + spin_unlock_irqrestore(_lock, flags);
> > +}
>
> If we've done a write to config space while the adapter was blocked,
> shouldn't we replay those accesses at this point?
This has been discussed a lot already. I think we might as well let the
thing fail in random and odd ways, as it's some pretty broken hardware
anyway that wants this functionality :)
thanks,
greg k-h
-
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 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: > >If we've done a write to config space while the adapter was blocked, > >shouldn't we replay those accesses at this point? > > I did not think that was necessary. We have to do *something*. We can't just throw away writes. I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. - Fail writes to config space while the device is blocked. - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Any other ideas? BTW, you know things like XFree86 go completely around the kernel's PCI accessors and poke at config space directly? -- "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 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote:
>>+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.0 -0600
@@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
{ \
unsigned long flags;\
Could you line up the \ so they're uniform like the above PCI_OP_READ?
Ok.
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access)) \
+ ret = dev->bus->ops->read(dev->bus, dev->devfn, pos, sizeof(type),
); \
+ else if (pos < sizeof(dev->saved_config_space)) \
+ data =
dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
+ spin_unlock_irqrestore(_lock, flags); \
+ *val = (type)data; \
Does this actually work? Surely if you're reading byte 14, you get the
byte that was at address 12 or 15 in the config space, depending whether
you're on a big or little endian machine?
It actually only works for 4 byte accesses. I am fixing this and will be
posting a patch later.
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(_lock, flags);
+ dev->block_ucfg_access = 0;
+ spin_unlock_irqrestore(_lock, flags);
+}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
I did not think that was necessary. It will certainly make the patch
more complicated. To implement it would really require we make the
userspace writers wait, which gets ugly since the wait is based on a
flag that can be updated at interrupt level so you end up with some fun
spinlocking. Not a big deal, it just starts getting ugly. Keep in mind,
one of the potential uses of this is for power management on PPC where a
given device could have its config space blocked for a long time.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
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 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 09:12:56AM -0600, Brian King wrote:
> Greg KH wrote:
> >On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
> >>+void pci_block_user_cfg_access(struct pci_dev *dev)
> >>+{
> >>+ unsigned long flags;
> >>+
> >>+ pci_save_state(dev);
> >>+ spin_lock_irqsave(_lock, flags);
> >>+ dev->block_ucfg_access = 1;
> >>+ spin_unlock_irqrestore(_lock, flags);
> >>+}
> >>+EXPORT_SYMBOL(pci_block_user_cfg_access);
> >
> >
> >EXPORT_SYMBOL_GPL() please?
>
> Ok.
I'm not entirely convinced these should be GPL-only exports. Basically,
this is saying that any driver for a device that has this problem must be
GPLd. I think that's a firmer stance than Linus had in mind originally.
> +++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
> 08:39:57.0 -0600
> @@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
> EXPORT_SYMBOL(pci_bus_write_config_byte);
> EXPORT_SYMBOL(pci_bus_write_config_word);
> EXPORT_SYMBOL(pci_bus_write_config_dword);
> +
> +#define PCI_USER_READ_CONFIG(size,type) \
> +int pci_user_read_config_##size \
> + (struct pci_dev *dev, int pos, type *val) \
{ \
unsigned long flags;\
Could you line up the \ so they're uniform like the above PCI_OP_READ?
> + int ret = 0;\
> + u32 data = -1; \
> + if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> + spin_lock_irqsave(_lock, flags);\
> + if (likely(!dev->block_ucfg_access))\
> + ret = dev->bus->ops->read(dev->bus, dev->devfn, pos,
> sizeof(type), ); \
> + else if (pos < sizeof(dev->saved_config_space)) \
> + data =
> dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
> + spin_unlock_irqrestore(_lock, flags); \
> + *val = (type)data; \
Does this actually work? Surely if you're reading byte 14, you get the
byte that was at address 12 or 15 in the config space, depending whether
you're on a big or little endian machine?
> +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(_lock, flags);
> + dev->block_ucfg_access = 0;
> + spin_unlock_irqrestore(_lock, flags);
> +}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
--
"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 1/1] pci: Block config access during BIST (resend)
Thanks for looking at this, Greg.
Greg KH wrote:
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
Global but not exported? If so, they are local to the pci core, right?
And if so, please put them in the drivers/pci/pci.h file and not the
include/linux/pci.h file.
Ok. They are now local to pci core.
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all other config reads.
+ *
+ * Return value:
+ * nothing
We know the return value is not needed by the way the function is
defined, these two lines are unneeded.
Deleted.
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(_lock, flags);
+ dev->block_ucfg_access = 1;
+ spin_unlock_irqrestore(_lock, flags);
+}
+EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
Ok.
+/**
+ * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows userspace PCI config accesses to resume.
+ *
+ * Return value:
+ * nothing
Same here as above.
Done.
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(_lock, flags);
+ dev->block_ucfg_access = 0;
+ spin_unlock_irqrestore(_lock, flags);
+}
+EXPORT_SYMBOL(pci_unblock_user_cfg_access);
Same as above.
Done.
@@ -896,6 +904,8 @@ extern void pci_disable_msix(struct pci_
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
#endif
+extern void pci_block_user_cfg_access(struct pci_dev *dev);
+extern void pci_unblock_user_cfg_access(struct pci_dev *dev);
#endif /* CONFIG_PCI */
Don't need empty functions for these if CONFIG_PCI is not enabled? Who
would be calling these functions, drivers? If so, please create the
empty functions.
Yes, device drivers would be one of the potential users of these
functions. Added the empty functions.
Also, please CC the linux-pci mailing list for pci specific patches like
this.
Done.
Here is an updated patch...
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c| 75 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h |7 +
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h |7 +
6 files changed, 113 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc2-bk9/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-02-01 08:35:29.0 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.0 -0600
@@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER;
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
Thanks for looking at this, Greg.
Greg KH wrote:
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
Global but not exported? If so, they are local to the pci core, right?
And if so, please put them in the drivers/pci/pci.h file and not the
include/linux/pci.h file.
Ok. They are now local to pci core.
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all other config reads.
+ *
+ * Return value:
+ * nothing
We know the return value is not needed by the way the function is
defined, these two lines are unneeded.
Deleted.
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 1;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
Ok.
+/**
+ * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows userspace PCI config accesses to resume.
+ *
+ * Return value:
+ * nothing
Same here as above.
Done.
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_unblock_user_cfg_access);
Same as above.
Done.
@@ -896,6 +904,8 @@ extern void pci_disable_msix(struct pci_
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
#endif
+extern void pci_block_user_cfg_access(struct pci_dev *dev);
+extern void pci_unblock_user_cfg_access(struct pci_dev *dev);
#endif /* CONFIG_PCI */
Don't need empty functions for these if CONFIG_PCI is not enabled? Who
would be calling these functions, drivers? If so, please create the
empty functions.
Yes, device drivers would be one of the potential users of these
functions. Added the empty functions.
Also, please CC the linux-pci mailing list for pci specific patches like
this.
Done.
Here is an updated patch...
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c| 75 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h |7 +
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h |7 +
6 files changed, 113 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc2-bk9/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-02-01 08:35:29.0 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.0 -0600
@@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 09:12:56AM -0600, Brian King wrote:
Greg KH wrote:
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 1;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
Ok.
I'm not entirely convinced these should be GPL-only exports. Basically,
this is saying that any driver for a device that has this problem must be
GPLd. I think that's a firmer stance than Linus had in mind originally.
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.0 -0600
@@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type) \
+int pci_user_read_config_##size \
+ (struct pci_dev *dev, int pos, type *val) \
{ \
unsigned long flags;\
Could you line up the \ so they're uniform like the above PCI_OP_READ?
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos,
sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
Does this actually work? Surely if you're reading byte 14, you get the
byte that was at address 12 or 15 in the config space, depending whether
you're on a big or little endian machine?
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
--
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 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote:
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.0 -0600
@@ -60,3 +60,78 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
{ \
unsigned long flags;\
Could you line up the \ so they're uniform like the above PCI_OP_READ?
Ok.
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access)) \
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos, sizeof(type),
data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
Does this actually work? Surely if you're reading byte 14, you get the
byte that was at address 12 or 15 in the config space, depending whether
you're on a big or little endian machine?
It actually only works for 4 byte accesses. I am fixing this and will be
posting a patch later.
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
I did not think that was necessary. It will certainly make the patch
more complicated. To implement it would really require we make the
userspace writers wait, which gets ugly since the wait is based on a
flag that can be updated at interrupt level so you end up with some fun
spinlocking. Not a big deal, it just starts getting ugly. Keep in mind,
one of the potential uses of this is for power management on PPC where a
given device could have its config space blocked for a long time.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
-
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 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? I did not think that was necessary. We have to do *something*. We can't just throw away writes. I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. - Fail writes to config space while the device is blocked. - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Any other ideas? BTW, you know things like XFree86 go completely around the kernel's PCI accessors and poke at config space directly? -- 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 1/1] pci: Block config access during BIST (resend)
On Tue, Feb 01, 2005 at 03:44:00PM +, Matthew Wilcox wrote:
On Tue, Feb 01, 2005 at 09:12:56AM -0600, Brian King wrote:
Greg KH wrote:
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 1;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
Ok.
I'm not entirely convinced these should be GPL-only exports. Basically,
this is saying that any driver for a device that has this problem must be
GPLd. I think that's a firmer stance than Linus had in mind originally.
originally? These are new functions added to the PCI core. I think
that any driver that wants to use this functionality had better be
released under the GPL as what they are wanting to do is a new thing.
That's why I prefer all new exports to be marked _GPL.
thanks,
greg k-h
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
This has been discussed a lot already. I think we might as well let the
thing fail in random and odd ways, as it's some pretty broken hardware
anyway that wants this functionality :)
thanks,
greg k-h
-
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 1/1] pci: Block config access during BIST (resend)
Matthew Wilcox wrote: On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? I did not think that was necessary. We have to do *something*. We can't just throw away writes. I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. This would need to be dynamic in size, as a device could be blocked for a long time. In the scenario that this is used for power management and the device could be blocked for a long time, its unclear that we would still want all the writes accumulated to be written out when the device becomes unblocked. - Fail writes to config space while the device is blocked. This would be nice and simple. I know Alan had some issue with returning failures on reads when blocked. Alan - do you have the same issue on writes? - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Would also be pretty simple to do and seems a little safer than potentially assaulting the recently unblocked device with who knows what values. The only problem I see with this is that we could end up returning strange values on cached reads if the writes update the cache. If userspace wrote to a read only register, we would end up returning that value on the read, which may not be the right thing to do. Any other ideas? We could go back to Alan's idea of putting userspace reads/writes to sleep when the device is blocked, although this has additional complications as well... BTW, you know things like XFree86 go completely around the kernel's PCI accessors and poke at config space directly? The purpose of this API is to provide a way for the kernel to stop userspace from accessing PCI devices when the results of doing so would be catastrophic, such as a PCI bus error. The only users of this API so far are device drivers running BIST on an adapter (which shouldn't happen on a video card AFAIK) and for PPC power management (Ben - will this be an issue for you?) -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 15:44 +, Matthew Wilcox wrote:
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
If we've done a write to config space while the adapter was blocked,
shouldn't we replay those accesses at this point?
I think no. In fact, I would be ok returning errors on writes from
userland. Need to do config space writes from userland is rare, must
more than reads, and if whatever does it can't properly arbitrate with
what's going on in the kernel, then it's broken.
Ben.
-
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 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 17:47 +, Matthew Wilcox wrote: On Tue, Feb 01, 2005 at 11:35:05AM -0600, Brian King wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? I did not think that was necessary. We have to do *something*. We can't just throw away writes. I think we can in fact. Again, nobody outside of the driver has legitimacy to write to the config space of a device, especially if the device is unreachable (either doing a BIST or power managed). I see a few options: - Log all pending writes to config space and replay the log when the device is unblocked. - Fail writes to config space while the device is blocked. I agree that returning an error in this case would be a good idea. - Write to the saved config space and then blat the saved config space back to the device upon unblocking. Any other ideas? BTW, you know things like XFree86 go completely around the kernel's PCI accessors and poke at config space directly? Not anymore afaik. They use /proc/bus/pci Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Tue, 2005-02-01 at 10:58 -0800, Greg KH wrote: If we've done a write to config space while the adapter was blocked, shouldn't we replay those accesses at this point? This has been discussed a lot already. I think we might as well let the thing fail in random and odd ways, as it's some pretty broken hardware anyway that wants this functionality :) I don't agree that it's broken hardware, especially in the case where we use these for power managed devices (on pmac, or eventually embedded, where we can have PM be as drastic as completely removing power/clock from a device or that sort of thing). But as I wrote earlier, I consider that in those cases, userland has no business writing to config space. If it does, then it has to be aware of the device beeing potentially offlined or that sort of thing and have proper interface to the kernel driver etc... The reads coming from the cache, on the other hand, have a more legitimate use which are to let - distro HW probing tool to actually see devices even when they are power managed or BIST by their kernel driver - crap like XFree86 to have a proper idea of what address ranges are actually used on the bus, including devices that are power managed or BIST (*). Ben. (*) An unrelated note: It's not always legal to allocate thigns in PCI space or move devices around anyway. It's definitely not on logically partitionned PPC machines where HV won't let a partition access other IO ranges than the ones where the allowed devices for this partition were intially put by the firmware... Again, X should really be changed to just stop doing anything but probing on the bus at least with Linux. The only problem is the VGA enable story, but as we discussed earlier, that too should be dealt with in the kernel. I think we could integrate that cleanly in sysfs for PCI devices in the generic code btw. Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
> +#define PCI_USER_READ_CONFIG(size,type) \
> +int pci_user_read_config_##size \
> + (struct pci_dev *dev, int pos, type *val) \
> +{\
> + unsigned long flags;\
> + int ret = 0;\
> + u32 data = -1; \
> + if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> + spin_lock_irqsave(_lock, flags);\
> + if (likely(!dev->block_ucfg_access))\
> + ret = dev->bus->ops->read(dev->bus, dev->devfn, pos,
> sizeof(type), ); \
> + else if (pos < sizeof(dev->saved_config_space)) \
> + data =
> dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
> + spin_unlock_irqrestore(_lock, flags); \
> + *val = (type)data; \
> + return ret; \
> +}
> +
> +#define PCI_USER_WRITE_CONFIG(size,type) \
> +int pci_user_write_config_##size \
> + (struct pci_dev *dev, int pos, type val)\
> +{\
> + unsigned long flags;\
> + int ret = 0;\
> + if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
> + spin_lock_irqsave(_lock, flags);\
> + if (likely(!dev->block_ucfg_access))
> \
> + ret = dev->bus->ops->write(dev->bus, dev->devfn, pos,
> sizeof(type), val); \
> + spin_unlock_irqrestore(_lock, flags); \
> + return ret; \
> +}
> +
> +PCI_USER_READ_CONFIG(byte, u8)
> +PCI_USER_READ_CONFIG(word, u16)
> +PCI_USER_READ_CONFIG(dword, u32)
> +PCI_USER_WRITE_CONFIG(byte, u8)
> +PCI_USER_WRITE_CONFIG(word, u16)
> +PCI_USER_WRITE_CONFIG(dword, u32)
Global but not exported? If so, they are local to the pci core, right?
And if so, please put them in the drivers/pci/pci.h file and not the
include/linux/pci.h file.
> +/**
> + * pci_block_user_cfg_access - Block userspace PCI config reads/writes
> + * @dev: pci device struct
> + *
> + * This function blocks any userspace PCI config accesses from occurring.
> + * When blocked, any writes will return -EBUSY and reads will return the
> + * data saved using pci_save_state for the first 64 bytes of config
> + * space and return -EBUSY for all other config reads.
> + *
> + * Return value:
> + * nothing
We know the return value is not needed by the way the function is
defined, these two lines are unneeded.
> +void pci_block_user_cfg_access(struct pci_dev *dev)
> +{
> + unsigned long flags;
> +
> + pci_save_state(dev);
> + spin_lock_irqsave(_lock, flags);
> + dev->block_ucfg_access = 1;
> + spin_unlock_irqrestore(_lock, flags);
> +}
> +EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
> +/**
> + * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
> + * @dev: pci device struct
> + *
> + * This function allows userspace PCI config accesses to resume.
> + *
> + * Return value:
> + * nothing
Same here as above.
> +void pci_unblock_user_cfg_access(struct pci_dev *dev)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(_lock, flags);
> + dev->block_ucfg_access = 0;
> + spin_unlock_irqrestore(_lock, flags);
> +}
> +EXPORT_SYMBOL(pci_unblock_user_cfg_access);
Same as above.
> @@ -896,6 +904,8 @@ extern void pci_disable_msix(struct pci_
> extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
> #endif
>
> +extern void pci_block_user_cfg_access(struct pci_dev *dev);
> +extern void pci_unblock_user_cfg_access(struct pci_dev *dev);
> #endif /* CONFIG_PCI */
Don't need empty functions for these if CONFIG_PCI is not enabled? Who
would be calling these functions, drivers? If so, please create the
empty functions.
Also, please CC the linux-pci mailing list for pci specific patches like
this.
thanks,
greg k-h
-
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 1/1] pci: Block config access during BIST (resend)
On Fri, Jan 28, 2005 at 08:35:46AM -0600, Brian King wrote:
+#define PCI_USER_READ_CONFIG(size,type) \
+int pci_user_read_config_##size \
+ (struct pci_dev *dev, int pos, type *val) \
+{\
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos,
sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{\
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))
\
+ ret = dev-bus-ops-write(dev-bus, dev-devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
Global but not exported? If so, they are local to the pci core, right?
And if so, please put them in the drivers/pci/pci.h file and not the
include/linux/pci.h file.
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all other config reads.
+ *
+ * Return value:
+ * nothing
We know the return value is not needed by the way the function is
defined, these two lines are unneeded.
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 1;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_block_user_cfg_access);
EXPORT_SYMBOL_GPL() please?
+/**
+ * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows userspace PCI config accesses to resume.
+ *
+ * Return value:
+ * nothing
Same here as above.
+void pci_unblock_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(pci_lock, flags);
+ dev-block_ucfg_access = 0;
+ spin_unlock_irqrestore(pci_lock, flags);
+}
+EXPORT_SYMBOL(pci_unblock_user_cfg_access);
Same as above.
@@ -896,6 +904,8 @@ extern void pci_disable_msix(struct pci_
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
#endif
+extern void pci_block_user_cfg_access(struct pci_dev *dev);
+extern void pci_unblock_user_cfg_access(struct pci_dev *dev);
#endif /* CONFIG_PCI */
Don't need empty functions for these if CONFIG_PCI is not enabled? Who
would be calling these functions, drivers? If so, please create the
empty functions.
Also, please CC the linux-pci mailing list for pci specific patches like
this.
thanks,
greg k-h
-
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 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote:
On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote:
On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote:
Well, I honestly think that this is unnecessary burden. I think that
just dropping writes & returning data from the cache on reads is enough,
blocking userspace isn't necessary, but then, I may be wrong ;)
Providing the BARs, cmd register and bridge VGA_EN are cached then I
think you
are right.
Everyone seems satisfied with the patch now. Greg, can you please apply?
Thanks
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))\
+ ret = dev->bus->ops->read(dev->bus, dev->devfn, pos,
sizeof(type), ); \
+ else if (pos < sizeof(dev->saved_config_space)) \
+ data =
dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
+ spin_unlock_irqrestore(_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))
\
+ ret = dev->bus->ops->write(dev->bus, dev->devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all other config reads.
+ *
+ * Return value:
+ * nothing
+ **/
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote:
On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote:
On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote:
Well, I honestly think that this is unnecessary burden. I think that
just dropping writes returning data from the cache on reads is enough,
blocking userspace isn't necessary, but then, I may be wrong ;)
Providing the BARs, cmd register and bridge VGA_EN are cached then I
think you
are right.
Everyone seems satisfied with the patch now. Greg, can you please apply?
Thanks
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos,
sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))
\
+ ret = dev-bus-ops-write(dev-bus, dev-devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all other config reads.
+ *
+ * Return value:
+ * nothing
+ **/
+void pci_block_user_cfg_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Thu, 2005-01-27 at 15:53 +, Alan Cox wrote: > On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: > > On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: > > Well, I honestly think that this is unnecessary burden. I think that > > just dropping writes & returning data from the cache on reads is enough, > > blocking userspace isn't necessary, but then, I may be wrong ;) > > Providing the BARs, cmd register and bridge VGA_EN are cached then I > think you > are right. There might be one problem with dropping of writes tho, which has to do with userland like X doing VGA flip-flip (VGA_EN on bridges and CMD_IO on devices). But I think that's a non-issues because: - For now, nobody is using this interface to hide a VGA device or a bridge, and I don't see that happening in the near future - Eventually, the control of who owns VGA is to be moved to a kernel driver doing proper arbitration as we discussed previously on several lists. (BTW. Alan, I've been a bit out of touch with that and Jon is too busy lately, do you know if there have been progress or at least prototype code one could take over from for that ?) Ben. - 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 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote: On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: Well, I honestly think that this is unnecessary burden. I think that just dropping writes & returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Providing the BARs, cmd register and bridge VGA_EN are cached then I think you are right. The first 64 bytes of config space are cached, so this should handle the registers you mention above. -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: > On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: > Well, I honestly think that this is unnecessary burden. I think that > just dropping writes & returning data from the cache on reads is enough, > blocking userspace isn't necessary, but then, I may be wrong ;) Providing the BARs, cmd register and bridge VGA_EN are cached then I think you are right. Alan - 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 1/1] pci: Block config access during BIST (resend)
On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: Well, I honestly think that this is unnecessary burden. I think that just dropping writes returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Providing the BARs, cmd register and bridge VGA_EN are cached then I think you are right. Alan - 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 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote: On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: Well, I honestly think that this is unnecessary burden. I think that just dropping writes returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Providing the BARs, cmd register and bridge VGA_EN are cached then I think you are right. The first 64 bytes of config space are cached, so this should handle the registers you mention above. -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
On Thu, 2005-01-27 at 15:53 +, Alan Cox wrote: On Mer, 2005-01-26 at 22:10, Benjamin Herrenschmidt wrote: On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: Well, I honestly think that this is unnecessary burden. I think that just dropping writes returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Providing the BARs, cmd register and bridge VGA_EN are cached then I think you are right. There might be one problem with dropping of writes tho, which has to do with userland like X doing VGA flip-flip (VGA_EN on bridges and CMD_IO on devices). But I think that's a non-issues because: - For now, nobody is using this interface to hide a VGA device or a bridge, and I don't see that happening in the near future - Eventually, the control of who owns VGA is to be moved to a kernel driver doing proper arbitration as we discussed previously on several lists. (BTW. Alan, I've been a bit out of touch with that and Jon is too busy lately, do you know if there have been progress or at least prototype code one could take over from for that ?) Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: > Here is the last one. I've looked at making userspace sleep until BIST > is finished. The downside I see to this is that is complicates the patch > due to the following reasons: > > 1. In order to also make this work for Ben's PPC power management usage > would require an additional flag and additional APIs to set and clear > the flag. > 2. Since BIST can be run at interrupt context, the interfaces to block > and unblock userspace accesses across BIST must be callable from > interrupt context. This prevents the usage of semaphores or simple > wait_event macros and requires new macros that carefully check the new > pci device flag and manage the spinlock. > Well, I honestly think that this is unnecessary burden. I think that just dropping writes & returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Ben. - 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 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote:
On Maw, 2005-01-18 at 15:14, Brian King wrote:
Alan - are you satisfied with the most recent patch, or would you prefer
the patch not returning failure return codes and just bit bucketing
writes and returning all ff's on reads? Either way works for me.
Which was the last one. For userspace we need to block while we finish
the BIST.
Here is the last one. I've looked at making userspace sleep until BIST
is finished. The downside I see to this is that is complicates the patch
due to the following reasons:
1. In order to also make this work for Ben's PPC power management usage
would require an additional flag and additional APIs to set and clear
the flag.
2. Since BIST can be run at interrupt context, the interfaces to block
and unblock userspace accesses across BIST must be callable from
interrupt context. This prevents the usage of semaphores or simple
wait_event macros and requires new macros that carefully check the new
pci device flag and manage the spinlock.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))\
+ ret = dev->bus->ops->read(dev->bus, dev->devfn, pos,
sizeof(type), ); \
+ else if (pos < sizeof(dev->saved_config_space)) \
+ data =
dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
+ spin_unlock_irqrestore(_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))
\
+ ret = dev->bus->ops->write(dev->bus, dev->devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access -
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
Alan Cox wrote:
On Maw, 2005-01-18 at 15:14, Brian King wrote:
Alan - are you satisfied with the most recent patch, or would you prefer
the patch not returning failure return codes and just bit bucketing
writes and returning all ff's on reads? Either way works for me.
Which was the last one. For userspace we need to block while we finish
the BIST.
Here is the last one. I've looked at making userspace sleep until BIST
is finished. The downside I see to this is that is complicates the patch
due to the following reasons:
1. In order to also make this work for Ben's PPC power management usage
would require an additional flag and additional APIs to set and clear
the flag.
2. Since BIST can be run at interrupt context, the interfaces to block
and unblock userspace accesses across BIST must be callable from
interrupt context. This prevents the usage of semaphores or simple
wait_event macros and requires new macros that carefully check the new
pci device flag and manage the spinlock.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos,
sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))
\
+ ret = dev-bus-ops-write(dev-bus, dev-devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access - Block
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Wed, 2005-01-26 at 10:34 -0600, Brian King wrote: Here is the last one. I've looked at making userspace sleep until BIST is finished. The downside I see to this is that is complicates the patch due to the following reasons: 1. In order to also make this work for Ben's PPC power management usage would require an additional flag and additional APIs to set and clear the flag. 2. Since BIST can be run at interrupt context, the interfaces to block and unblock userspace accesses across BIST must be callable from interrupt context. This prevents the usage of semaphores or simple wait_event macros and requires new macros that carefully check the new pci device flag and manage the spinlock. Well, I honestly think that this is unnecessary burden. I think that just dropping writes returning data from the cache on reads is enough, blocking userspace isn't necessary, but then, I may be wrong ;) Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Maw, 2005-01-18 at 15:14, Brian King wrote: > Alan - are you satisfied with the most recent patch, or would you prefer > the patch not returning failure return codes and just bit bucketing > writes and returning all ff's on reads? Either way works for me. Which was the last one. For userspace we need to block while we finish the BIST. - 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 1/1] pci: Block config access during BIST (resend)
On Maw, 2005-01-18 at 15:14, Brian King wrote: Alan - are you satisfied with the most recent patch, or would you prefer the patch not returning failure return codes and just bit bucketing writes and returning all ff's on reads? Either way works for me. Which was the last one. For userspace we need to block while we finish the BIST. - 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 1/1] pci: Block config access during BIST (resend)
Andi Kleen wrote:
On Tue, Jan 18, 2005 at 09:14:21AM -0600, Brian King wrote:
Andi Kleen wrote:
As Brian said the device he was working with would just not answer,
leading to a bus abort. This would get on a PC.
You could simulate this if you want, although I think a EBUSY or EIO
is better.
Alan - are you satisfied with the most recent patch, or would you prefer
the patch not returning failure return codes and just bit bucketing
writes and returning all ff's on reads? Either way works for me.
Hmm, I think i haven't seen a recent patch. But as long as it doesn't
block in pci_config_* and is light weight there it's fine for me.
-Andi
Here is my latest patch.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
Signed-off-by: Brian King <[EMAIL PROTECTED]>
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))\
+ ret = dev->bus->ops->read(dev->bus, dev->devfn, pos,
sizeof(type), ); \
+ else if (pos < sizeof(dev->saved_config_space)) \
+ data =
dev->saved_config_space[pos/sizeof(dev->saved_config_space[0])]; \
+ spin_unlock_irqrestore(_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(_lock, flags);\
+ if (likely(!dev->block_ucfg_access))
\
+ ret = dev->bus->ops->write(dev->bus, dev->devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Tue, Jan 18, 2005 at 09:14:21AM -0600, Brian King wrote: > Andi Kleen wrote: > >As Brian said the device he was working with would just not answer, > >leading to a bus abort. This would get on a PC. > >You could simulate this if you want, although I think a EBUSY or EIO > >is better. > > Alan - are you satisfied with the most recent patch, or would you prefer > the patch not returning failure return codes and just bit bucketing > writes and returning all ff's on reads? Either way works for me. Hmm, I think i haven't seen a recent patch. But as long as it doesn't block in pci_config_* and is light weight there it's fine for me. -Andi - 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 1/1] pci: Block config access during BIST (resend)
Andi Kleen wrote: As Brian said the device he was working with would just not answer, leading to a bus abort. This would get on a PC. You could simulate this if you want, although I think a EBUSY or EIO is better. Alan - are you satisfied with the most recent patch, or would you prefer the patch not returning failure return codes and just bit bucketing writes and returning all ff's on reads? Either way works for me. -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
Andi Kleen wrote: As Brian said the device he was working with would just not answer, leading to a bus abort. This would get on a PC. You could simulate this if you want, although I think a EBUSY or EIO is better. Alan - are you satisfied with the most recent patch, or would you prefer the patch not returning failure return codes and just bit bucketing writes and returning all ff's on reads? Either way works for me. -- Brian King eServer Storage I/O IBM Linux Technology Center - 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 1/1] pci: Block config access during BIST (resend)
On Tue, Jan 18, 2005 at 09:14:21AM -0600, Brian King wrote: Andi Kleen wrote: As Brian said the device he was working with would just not answer, leading to a bus abort. This would get on a PC. You could simulate this if you want, although I think a EBUSY or EIO is better. Alan - are you satisfied with the most recent patch, or would you prefer the patch not returning failure return codes and just bit bucketing writes and returning all ff's on reads? Either way works for me. Hmm, I think i haven't seen a recent patch. But as long as it doesn't block in pci_config_* and is light weight there it's fine for me. -Andi - 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 1/1] pci: Block config access during BIST (resend)
Andi Kleen wrote:
On Tue, Jan 18, 2005 at 09:14:21AM -0600, Brian King wrote:
Andi Kleen wrote:
As Brian said the device he was working with would just not answer,
leading to a bus abort. This would get on a PC.
You could simulate this if you want, although I think a EBUSY or EIO
is better.
Alan - are you satisfied with the most recent patch, or would you prefer
the patch not returning failure return codes and just bit bucketing
writes and returning all ff's on reads? Either way works for me.
Hmm, I think i haven't seen a recent patch. But as long as it doesn't
block in pci_config_* and is light weight there it's fine for me.
-Andi
Here is my latest patch.
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
Some PCI adapters (eg. ipr scsi adapters) have an exposure today in that
they issue BIST to the adapter to reset the card. If, during the time
it takes to complete BIST, userspace attempts to access PCI config space,
the host bus bridge will master abort the access since the ipr adapter
does not respond on the PCI bus for a brief period of time when running BIST.
On PPC64 hardware, this master abort results in the host PCI bridge
isolating that PCI device from the rest of the system, making the device
unusable until Linux is rebooted. This patch is an attempt to close that
exposure by introducing some blocking code in the PCI code. When blocked,
writes will be humored and reads will return the cached value. Ben
Herrenschmidt has also mentioned that he plans to use this in PPC power
management.
Signed-off-by: Brian King [EMAIL PROTECTED]
---
Signed-off-by: Brian King [EMAIL PROTECTED]
---
linux-2.6.11-rc1-bjking1/drivers/pci/access.c| 81 +++
linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.11-rc1-bjking1/drivers/pci/proc.c | 28 +++
linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.11-rc1-bjking1/include/linux/pci.h | 12 +++
5 files changed, 117 insertions(+), 26 deletions(-)
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist_again
drivers/pci/access.c
---
linux-2.6.11-rc1/drivers/pci/access.c~pci_block_user_config_io_during_bist_again
2005-01-13 15:57:15.0 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13
15:57:54.0 -0600
@@ -60,3 +60,84 @@ EXPORT_SYMBOL(pci_bus_read_config_dword)
EXPORT_SYMBOL(pci_bus_write_config_byte);
EXPORT_SYMBOL(pci_bus_write_config_word);
EXPORT_SYMBOL(pci_bus_write_config_dword);
+
+#define PCI_USER_READ_CONFIG(size,type)\
+int pci_user_read_config_##size\
+ (struct pci_dev *dev, int pos, type *val) \
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ u32 data = -1; \
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))\
+ ret = dev-bus-ops-read(dev-bus, dev-devfn, pos,
sizeof(type), data); \
+ else if (pos sizeof(dev-saved_config_space)) \
+ data =
dev-saved_config_space[pos/sizeof(dev-saved_config_space[0])]; \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ *val = (type)data; \
+ return ret; \
+}
+
+#define PCI_USER_WRITE_CONFIG(size,type) \
+int pci_user_write_config_##size \
+ (struct pci_dev *dev, int pos, type val)\
+{ \
+ unsigned long flags;\
+ int ret = 0;\
+ if (PCI_##size##_BAD) return PCIBIOS_BAD_REGISTER_NUMBER; \
+ spin_lock_irqsave(pci_lock, flags);\
+ if (likely(!dev-block_ucfg_access))
\
+ ret = dev-bus-ops-write(dev-bus, dev-devfn, pos,
sizeof(type), val); \
+ spin_unlock_irqrestore(pci_lock, flags); \
+ return ret; \
+}
+
+PCI_USER_READ_CONFIG(byte, u8)
+PCI_USER_READ_CONFIG(word, u16)
+PCI_USER_READ_CONFIG(dword, u32)
+PCI_USER_WRITE_CONFIG(byte, u8)
+PCI_USER_WRITE_CONFIG(word, u16)
+PCI_USER_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_user_cfg_access - Block userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any userspace PCI config accesses from occurring.
+ * When blocked, any writes will return -EBUSY and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return -EBUSY for all
Re: [PATCH 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 23:07 +0100, Andi Kleen wrote: > > What is complex in there ? I agree it's not convenient to do this from > > the very low level ones that don't take the pci_dev * as an argument, > > but from the higher level ones that does, the overhead is basically to > > test a flag in the pci_dev, I doubt it will be significant in any way > > performance wise, especially compared to the cost of a config space > > access... > > For once you cannot block in them. There are even setups that > need to (have to) do config space accesses in interrupt handlers. > The operations done there should be rather light weight. I don't think we ever want to block in that sense. I think all we need is the "filter" mecanism, that is drop writes and return cached data on reads when the device is "offlined"... Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Sul, 2005-01-16 at 04:48, Andi Kleen wrote: > I just request that this shouldn't be done in the low level pci_config_read_* > functions. Please keep them simple and lean. If you want such complex > semantics for user space do it in a separate layer. It seems reasonable not to implement the wait (if not essential) in the pci_config_* cases just in the pci user ones (as Brian was doing in the later patches). - 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 1/1] pci: Block config access during BIST (resend)
> What is complex in there ? I agree it's not convenient to do this from > the very low level ones that don't take the pci_dev * as an argument, > but from the higher level ones that does, the overhead is basically to > test a flag in the pci_dev, I doubt it will be significant in any way > performance wise, especially compared to the cost of a config space > access... For once you cannot block in them. There are even setups that need to (have to) do config space accesses in interrupt handlers. The operations done there should be rather light weight. -Andi - 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 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 05:48 +0100, Andi Kleen wrote: > > Right. Though I think the "will be back soon" and "is invisible" are > > pretty much the same thing. That is, in both our cases (BIST and pmac > > PM), we want the device to still be visible to userland, as it actually > > exist, should be properly detected by userland config tools etc..., but > > may only be actually enabled when the interface is opened/used for PM > > reasons. > > I just request that this shouldn't be done in the low level pci_config_read_* > functions. Please keep them simple and lean. If you want such complex > semantics for user space do it in a separate layer. What is complex in there ? I agree it's not convenient to do this from the very low level ones that don't take the pci_dev * as an argument, but from the higher level ones that does, the overhead is basically to test a flag in the pci_dev, I doubt it will be significant in any way performance wise, especially compared to the cost of a config space access... Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 05:48 +0100, Andi Kleen wrote: Right. Though I think the will be back soon and is invisible are pretty much the same thing. That is, in both our cases (BIST and pmac PM), we want the device to still be visible to userland, as it actually exist, should be properly detected by userland config tools etc..., but may only be actually enabled when the interface is opened/used for PM reasons. I just request that this shouldn't be done in the low level pci_config_read_* functions. Please keep them simple and lean. If you want such complex semantics for user space do it in a separate layer. What is complex in there ? I agree it's not convenient to do this from the very low level ones that don't take the pci_dev * as an argument, but from the higher level ones that does, the overhead is basically to test a flag in the pci_dev, I doubt it will be significant in any way performance wise, especially compared to the cost of a config space access... Ben. - 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 1/1] pci: Block config access during BIST (resend)
What is complex in there ? I agree it's not convenient to do this from the very low level ones that don't take the pci_dev * as an argument, but from the higher level ones that does, the overhead is basically to test a flag in the pci_dev, I doubt it will be significant in any way performance wise, especially compared to the cost of a config space access... For once you cannot block in them. There are even setups that need to (have to) do config space accesses in interrupt handlers. The operations done there should be rather light weight. -Andi - 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 1/1] pci: Block config access during BIST (resend)
On Sul, 2005-01-16 at 04:48, Andi Kleen wrote: I just request that this shouldn't be done in the low level pci_config_read_* functions. Please keep them simple and lean. If you want such complex semantics for user space do it in a separate layer. It seems reasonable not to implement the wait (if not essential) in the pci_config_* cases just in the pci user ones (as Brian was doing in the later patches). - 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 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 23:07 +0100, Andi Kleen wrote: What is complex in there ? I agree it's not convenient to do this from the very low level ones that don't take the pci_dev * as an argument, but from the higher level ones that does, the overhead is basically to test a flag in the pci_dev, I doubt it will be significant in any way performance wise, especially compared to the cost of a config space access... For once you cannot block in them. There are even setups that need to (have to) do config space accesses in interrupt handlers. The operations done there should be rather light weight. I don't think we ever want to block in that sense. I think all we need is the filter mecanism, that is drop writes and return cached data on reads when the device is offlined... Ben. - 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 1/1] pci: Block config access during BIST (resend)
> Right. Though I think the "will be back soon" and "is invisible" are > pretty much the same thing. That is, in both our cases (BIST and pmac > PM), we want the device to still be visible to userland, as it actually > exist, should be properly detected by userland config tools etc..., but > may only be actually enabled when the interface is opened/used for PM > reasons. I just request that this shouldn't be done in the low level pci_config_read_* functions. Please keep them simple and lean. If you want such complex semantics for user space do it in a separate layer. -Andi - 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 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 00:58 +, Alan Cox wrote: > On Sad, 2005-01-15 at 06:20, Benjamin Herrenschmidt wrote: > > I'm pretty sure similar situations can happen on other archs when > > pushing a bit on power management, especially things like handhelds > > (though not much of them are PCI based for now). > > > > That's why a "generic" mecanism to hide such devices while providing > > cached data on config space read's would be useful to me as well. > > That makes a lot of sense. So we need both a "blocked, will be back > soon" and "this PCI device is invisible" flags. A device going into > blocked and not coming back would presumably transition into > "invisible". I'm assuming we can't just delete the PCI device because > the kernel needs to know that cell is there for future use/abuse. Right. Though I think the "will be back soon" and "is invisible" are pretty much the same thing. That is, in both our cases (BIST and pmac PM), we want the device to still be visible to userland, as it actually exist, should be properly detected by userland config tools etc..., but may only be actually enabled when the interface is opened/used for PM reasons. Ben. - 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 1/1] pci: Block config access during BIST (resend)
On Sad, 2005-01-15 at 06:20, Benjamin Herrenschmidt wrote: > I'm pretty sure similar situations can happen on other archs when > pushing a bit on power management, especially things like handhelds > (though not much of them are PCI based for now). > > That's why a "generic" mecanism to hide such devices while providing > cached data on config space read's would be useful to me as well. That makes a lot of sense. So we need both a "blocked, will be back soon" and "this PCI device is invisible" flags. A device going into blocked and not coming back would presumably transition into "invisible". I'm assuming we can't just delete the PCI device because the kernel needs to know that cell is there for future use/abuse. - 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 1/1] pci: Block config access during BIST (resend)
On Sad, 2005-01-15 at 06:20, Benjamin Herrenschmidt wrote: I'm pretty sure similar situations can happen on other archs when pushing a bit on power management, especially things like handhelds (though not much of them are PCI based for now). That's why a generic mecanism to hide such devices while providing cached data on config space read's would be useful to me as well. That makes a lot of sense. So we need both a blocked, will be back soon and this PCI device is invisible flags. A device going into blocked and not coming back would presumably transition into invisible. I'm assuming we can't just delete the PCI device because the kernel needs to know that cell is there for future use/abuse. - 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 1/1] pci: Block config access during BIST (resend)
On Sun, 2005-01-16 at 00:58 +, Alan Cox wrote: On Sad, 2005-01-15 at 06:20, Benjamin Herrenschmidt wrote: I'm pretty sure similar situations can happen on other archs when pushing a bit on power management, especially things like handhelds (though not much of them are PCI based for now). That's why a generic mecanism to hide such devices while providing cached data on config space read's would be useful to me as well. That makes a lot of sense. So we need both a blocked, will be back soon and this PCI device is invisible flags. A device going into blocked and not coming back would presumably transition into invisible. I'm assuming we can't just delete the PCI device because the kernel needs to know that cell is there for future use/abuse. Right. Though I think the will be back soon and is invisible are pretty much the same thing. That is, in both our cases (BIST and pmac PM), we want the device to still be visible to userland, as it actually exist, should be properly detected by userland config tools etc..., but may only be actually enabled when the interface is opened/used for PM reasons. Ben. - 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 1/1] pci: Block config access during BIST (resend)
Right. Though I think the will be back soon and is invisible are pretty much the same thing. That is, in both our cases (BIST and pmac PM), we want the device to still be visible to userland, as it actually exist, should be properly detected by userland config tools etc..., but may only be actually enabled when the interface is opened/used for PM reasons. I just request that this shouldn't be done in the low level pci_config_read_* functions. Please keep them simple and lean. If you want such complex semantics for user space do it in a separate layer. -Andi - 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/
