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.10-bk13-bjking1/drivers/pci/access.c | 104 +++++++++++++++++++++++++
linux-2.6.10-bk13-bjking1/include/linux/pci.h | 37 ++------
2 files changed, 115 insertions(+), 26 deletions(-)
diff -puN include/linux/pci.h~pci_block_config_io_during_bist include/linux/pci.h
--- linux-2.6.10-bk13/include/linux/pci.h~pci_block_config_io_during_bist 2005-01-10 08:43:25.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/include/linux/pci.h 2005-01-10 08:43:25.000000000 -0600
@@ -535,7 +535,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_cfg_access:1; /* config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -750,31 +751,12 @@ int pci_bus_read_config_dword (struct pc
int pci_bus_write_config_byte (struct pci_bus *bus, unsigned int devfn, int where, u8 val);
int pci_bus_write_config_word (struct pci_bus *bus, unsigned int devfn, int where, u16 val);
int pci_bus_write_config_dword (struct pci_bus *bus, unsigned int devfn, int where, u32 val);
-
-static inline int pci_read_config_byte(struct pci_dev *dev, int where, u8 *val)
-{
- return pci_bus_read_config_byte (dev->bus, dev->devfn, where, val);
-}
-static inline int pci_read_config_word(struct pci_dev *dev, int where, u16 *val)
-{
- return pci_bus_read_config_word (dev->bus, dev->devfn, where, val);
-}
-static inline int pci_read_config_dword(struct pci_dev *dev, int where, u32 *val)
-{
- return pci_bus_read_config_dword (dev->bus, dev->devfn, where, val);
-}
-static inline int pci_write_config_byte(struct pci_dev *dev, int where, u8 val)
-{
- return pci_bus_write_config_byte (dev->bus, dev->devfn, where, val);
-}
-static inline int pci_write_config_word(struct pci_dev *dev, int where, u16 val)
-{
- return pci_bus_write_config_word (dev->bus, dev->devfn, where, val);
-}
-static inline int pci_write_config_dword(struct pci_dev *dev, int where, u32 val)
-{
- return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
-}
+int pci_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_write_config_dword(struct pci_dev *dev, int where, u32 val);
int pci_enable_device(struct pci_dev *dev);
int pci_enable_device_bars(struct pci_dev *dev, int mask);
@@ -870,6 +852,9 @@ extern void pci_disable_msix(struct pci_
extern void msi_remove_pci_irq_vectors(struct pci_dev *dev);
#endif
+extern int pci_start_bist(struct pci_dev *dev);
+extern void pci_block_config_access(struct pci_dev *dev);
+extern void pci_unblock_config_access(struct pci_dev *dev);
#endif /* CONFIG_PCI */
/* Include architecture-dependent settings and functions */
diff -puN drivers/pci/access.c~pci_block_config_io_during_bist drivers/pci/access.c
--- linux-2.6.10-bk13/drivers/pci/access.c~pci_block_config_io_during_bist 2005-01-10 08:43:25.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/drivers/pci/access.c 2005-01-10 08:43:25.000000000 -0600
@@ -60,3 +60,107 @@ 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_READ_CONFIG(size,type) \
+int pci_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_cfg_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_WRITE_CONFIG(size,type) \
+int pci_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_cfg_access)) \
+ ret = dev->bus->ops->write(dev->bus, dev->devfn, pos, sizeof(type), val); \
+ spin_unlock_irqrestore(&pci_lock, flags); \
+ return ret; \
+}
+
+PCI_READ_CONFIG(byte, u8)
+PCI_READ_CONFIG(word, u16)
+PCI_READ_CONFIG(dword, u32)
+PCI_WRITE_CONFIG(byte, u8)
+PCI_WRITE_CONFIG(word, u16)
+PCI_WRITE_CONFIG(dword, u32)
+
+/**
+ * pci_block_config_access - Block PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function blocks any PCI config accesses from occurring.
+ * When blocked, any writes will be humored and reads will return
+ * the data saved using pci_save_state for the first 64 bytes
+ * of config space and return ff's for all other config reads.
+ *
+ * Return value:
+ * nothing
+ **/
+void pci_block_config_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ pci_save_state(dev);
+ spin_lock_irqsave(&pci_lock, flags);
+ dev->block_cfg_access = 1;
+ spin_unlock_irqrestore(&pci_lock, flags);
+}
+
+/**
+ * pci_unblock_config_access - Unblock PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows PCI config accesses to resume.
+ *
+ * Return value:
+ * nothing
+ **/
+void pci_unblock_config_access(struct pci_dev *dev)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&pci_lock, flags);
+ dev->block_cfg_access = 0;
+ spin_unlock_irqrestore(&pci_lock, flags);
+}
+
+/**
+ * pci_start_bist - Start BIST on a PCI device
+ * @dev: pci device struct
+ *
+ * This function allows a device driver to start BIST
+ * when PCI config accesses are disabled.
+ *
+ * Return value:
+ * nothing
+ **/
+int pci_start_bist(struct pci_dev *dev)
+{
+ return pci_bus_write_config_byte(dev->bus, dev->devfn, PCI_BIST, PCI_BIST_START);
+}
+
+EXPORT_SYMBOL(pci_read_config_byte);
+EXPORT_SYMBOL(pci_read_config_word);
+EXPORT_SYMBOL(pci_read_config_dword);
+EXPORT_SYMBOL(pci_write_config_byte);
+EXPORT_SYMBOL(pci_write_config_word);
+EXPORT_SYMBOL(pci_write_config_dword);
+EXPORT_SYMBOL(pci_start_bist);
+EXPORT_SYMBOL(pci_block_config_access);
+EXPORT_SYMBOL(pci_unblock_config_access);
_
[email protected] writes:
> 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.
I think it would be better to do this check higher level in the driver.
IMHO pci_*_config should stay lean and fast low level functions without
such baggage.
-Andi
Andi Kleen wrote:
> [email protected] writes:
>
>
>>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.
>
>
> I think it would be better to do this check higher level in the driver.
> IMHO pci_*_config should stay lean and fast low level functions without
> such baggage.
The problem I am trying to solve is the userspace PCI access methods
accessing my config space when the adapter is not able to handle such an
access. Today these accesses bypass the device driver altogether and
there is no way to stop them. An alternative to this patch would be to
only do these checks for the PCI config accesses initiated from the
various userspace mechanisms, but I'm not sure the patch would then be
usable for what benh wanted it for. Ben?
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
> The problem I am trying to solve is the userspace PCI access methods
> accessing my config space when the adapter is not able to handle such an
> access. Today these accesses bypass the device driver altogether and
> there is no way to stop them. An alternative to this patch would be to
For this I would add a semaphore or a lock bit to pci_dev.
Probably a simple flag is good enough that is checked by sysfs/proc
and return EBUSY when set.
This assumes that the driver controls the device during BIST time.
-Andi
On Llu, 2005-01-10 at 16:10, Andi Kleen wrote:
> [email protected] writes:
> I think it would be better to do this check higher level in the driver.
> IMHO pci_*_config should stay lean and fast low level functions without
> such baggage.
Userspace pci interfaces can also trigger this withour protection.
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.10-bk13-bjking1/drivers/pci/access.c | 83 ++++++++++++++++++++++
linux-2.6.10-bk13-bjking1/drivers/pci/pci-sysfs.c | 10 +-
linux-2.6.10-bk13-bjking1/drivers/pci/proc.c | 28 +++----
linux-2.6.10-bk13-bjking1/drivers/pci/syscall.c | 12 +--
linux-2.6.10-bk13-bjking1/include/linux/pci.h | 12 ++-
5 files changed, 119 insertions(+), 26 deletions(-)
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist drivers/pci/pci-sysfs.c
--- linux-2.6.10-bk13/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist 2005-01-10 11:14:49.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/drivers/pci/pci-sysfs.c 2005-01-10 12:48:43.000000000 -0600
@@ -109,7 +109,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -118,7 +118,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -129,7 +129,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -153,7 +153,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -170,7 +170,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/access.c~pci_block_user_config_io_during_bist drivers/pci/access.c
--- linux-2.6.10-bk13/drivers/pci/access.c~pci_block_user_config_io_during_bist 2005-01-10 11:15:46.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/drivers/pci/access.c 2005-01-10 14:29:57.000000000 -0600
@@ -60,3 +60,86 @@ 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])]; \
+ else \
+ ret = -EBUSY; \
+ 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 = -EBUSY; \
+ 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;
+
+ 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);
+
+/**
+ * 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
+ **/
+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);
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist include/linux/pci.h
--- linux-2.6.10-bk13/include/linux/pci.h~pci_block_user_config_io_during_bist 2005-01-10 11:23:03.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/include/linux/pci.h 2005-01-10 13:17:09.000000000 -0600
@@ -535,7 +535,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -776,6 +777,13 @@ static inline int pci_write_config_dword
return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
}
+int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
int pci_enable_device(struct pci_dev *dev);
int pci_enable_device_bars(struct pci_dev *dev, int mask);
void pci_disable_device(struct pci_dev *dev);
@@ -870,6 +878,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 */
/* Include architecture-dependent settings and functions */
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist drivers/pci/syscall.c
--- linux-2.6.10-bk13/drivers/pci/syscall.c~pci_block_user_config_io_during_bist 2005-01-10 12:49:16.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/drivers/pci/syscall.c 2005-01-10 12:49:50.000000000 -0600
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist drivers/pci/proc.c
--- linux-2.6.10-bk13/drivers/pci/proc.c~pci_block_user_config_io_during_bist 2005-01-10 12:50:05.000000000 -0600
+++ linux-2.6.10-bk13-bjking1/drivers/pci/proc.c 2005-01-10 12:51:04.000000000 -0600
@@ -79,7 +79,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +88,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +97,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +106,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +115,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +150,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +159,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +168,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +177,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +186,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +474,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
_
On Llu, 2005-01-10 at 22:57, Brian King wrote:
> > For this I would add a semaphore or a lock bit to pci_dev.
> > Probably a simple flag is good enough that is checked by sysfs/proc
> > and return EBUSY when set.
>
> How about something like this... (only compile tested at this point)
User space does not expect to get dumped with -EBUSY randomly on PCI
accesses. Not a viable option in that form _but_ making them sleep would
work - even with a simple global wait queue
for the pci_unblock_.. path
ie add the following (oh and uninlined probably for compatcness)
static int pci_user_wait_access(struct pci_dev *pdev) {
wait_event(&pci_ucfg_wait, dev->block_ucfg_access == 0);
}
On Tue, Jan 11, 2005 at 02:37:40PM +0000, Alan Cox wrote:
> On Llu, 2005-01-10 at 22:57, Brian King wrote:
> > > For this I would add a semaphore or a lock bit to pci_dev.
> > > Probably a simple flag is good enough that is checked by sysfs/proc
> > > and return EBUSY when set.
> >
> > How about something like this... (only compile tested at this point)
>
>
> User space does not expect to get dumped with -EBUSY randomly on PCI
I think it's a reasonable thing to do. If you prefer you could fake a
0xffffffff read, that would look like busy or non existing hardware.
But the errno would seem to be cleaner to me.
There may be other reasons to have error codes here in the future
too - e.g. with the upcomming support for real PCI error handling
it would make a lot of sense to return EIO in some cases. User space
will just have to cope with that.
> accesses. Not a viable option in that form _but_ making them sleep would
> work - even with a simple global wait queue
> for the pci_unblock_.. path
>
> ie add the following (oh and uninlined probably for compatcness)
>
> static int pci_user_wait_access(struct pci_dev *pdev) {
> wait_event(&pci_ucfg_wait, dev->block_ucfg_access == 0);
> }
I don't like this very much. What happens when the device
doesn't get out of BIST for some reason?
I think it's better to keep this simple, and an error is fine for that.
-Andi
Andi Kleen wrote:
> On Tue, Jan 11, 2005 at 02:37:40PM +0000, Alan Cox wrote:
>
>>On Llu, 2005-01-10 at 22:57, Brian King wrote:
>>
>>>>For this I would add a semaphore or a lock bit to pci_dev.
>>>>Probably a simple flag is good enough that is checked by sysfs/proc
>>>>and return EBUSY when set.
>>>
>>>How about something like this... (only compile tested at this point)
>>
>>
>>User space does not expect to get dumped with -EBUSY randomly on PCI
>
>
> I think it's a reasonable thing to do. If you prefer you could fake a
> 0xffffffff read, that would look like busy or non existing hardware.
> But the errno would seem to be cleaner to me.
We can certainly go either way. I decided to go the way I did simply
because that was what was suggested.
> There may be other reasons to have error codes here in the future
> too - e.g. with the upcomming support for real PCI error handling
> it would make a lot of sense to return EIO in some cases. User space
> will just have to cope with that.
>
>
>>accesses. Not a viable option in that form _but_ making them sleep would
>>work - even with a simple global wait queue
>>for the pci_unblock_.. path
>>
>>ie add the following (oh and uninlined probably for compatcness)
>>
>>static int pci_user_wait_access(struct pci_dev *pdev) {
>> wait_event(&pci_ucfg_wait, dev->block_ucfg_access == 0);
>>}
>
>
> I don't like this very much. What happens when the device
> doesn't get out of BIST for some reason?
>
> I think it's better to keep this simple, and an error is fine for that.
Also, Ben Herrenschmidt expressed a desire to use this interface in
dealing with some power management issues on G5's. The issue is that
when some devices are powered off on G5 they will not respond to config
cycles and may even deadlock the system. This usage would require the
ability to block userspace for an indefinite period of time and also
make use of the config space caching code that is in my patch.
-Brian
--
Brian King
eServer Storage I/O
IBM Linux Technology Center
On Maw, 2005-01-11 at 22:17, Brian King wrote:
> Andi Kleen wrote:
> We can certainly go either way. I decided to go the way I did simply
> because that was what was suggested.
The error case breaks X11. The cached approach of Ben's would probably
hide that nicely although it might cause some random crashes during
powerdown. Its hard to fix in user space because X has no idea how to
tell what is going on portably in the 'it broke' case other than spin
trying to for a while. The kernel knows what is up and can make an
intelligent decision - if the device doesn't come back from bist or we
need to mark devices as "very gone away" then maybe a second flag so we
have two user checked flags
In BIST - wait
Dead - error
> cycles and may even deadlock the system. This usage would require the
> ability to block userspace for an indefinite period of time and also
> make use of the config space caching code that is in my patch.
What if the user space you block holds a resource that is preventing
power down completing ? I can see how the kernel side stuff needs to be
more robust (bad news btw.. over 99% of pci config calls in the kernel
don't check the return according to a quick grep count. Good news is
they are almost all reads so caching ought to work for the main config
space stuff at least)
Caching doesn't however work for the cases like IRQ handlers but that
seems not to be problematic as the only "other device" stuff people
should be sticking their noses in except for bridge fixup stuff is
things like the BAR registers.
Alan
On Thu, Jan 13, 2005 at 03:35:59PM +0000, Alan Cox wrote:
> On Maw, 2005-01-11 at 17:33, Andi Kleen wrote:
> > > User space does not expect to get dumped with -EBUSY randomly on PCI
> >
> > I think it's a reasonable thing to do. If you prefer you could fake a
> > 0xffffffff read, that would look like busy or non existing hardware.
> > But the errno would seem to be cleaner to me.
>
> Either will break X.
You are saying that X during its own private broken PCI scan
stops scanning when it sees an errno?
That sounds incredibly broken if true. I'm not sure how much
effort the kernel should really take to work around such
user breakage. I suppose an ffffffff return would work.
>
> > > static int pci_user_wait_access(struct pci_dev *pdev) {
> > > wait_event(&pci_ucfg_wait, dev->block_ucfg_access == 0);
> > > }
> >
> > I don't like this very much. What happens when the device
> > doesn't get out of BIST for some reason?
>
> Then you need to switch to wait_event_timeout(). Its not terribly hard
> 8)
Just complicating something that should be very simple.
-Andi
On Iau, 2005-01-13 at 18:03, Andi Kleen wrote:
> You are saying that X during its own private broken PCI scan
> stops scanning when it sees an errno?
>
> That sounds incredibly broken if true. I'm not sure how much
> effort the kernel should really take to work around such
> user breakage. I suppose an ffffffff return would work.
X needs to be able to find the device layout in order to build its PCI
mappings. Cached data is probably quite sufficient for this.
> > Then you need to switch to wait_event_timeout(). Its not terribly hard
> > 8)
>
> Just complicating something that should be very simple.
You are breaking an established user space API. Its not suprising this
will break applications is it.
On Thu, Jan 13, 2005 at 06:46:33PM +0000, Alan Cox wrote:
> On Iau, 2005-01-13 at 18:03, Andi Kleen wrote:
> > You are saying that X during its own private broken PCI scan
> > stops scanning when it sees an errno?
> >
> > That sounds incredibly broken if true. I'm not sure how much
> > effort the kernel should really take to work around such
> > user breakage. I suppose an ffffffff return would work.
>
> X needs to be able to find the device layout in order to build its PCI
> mappings. Cached data is probably quite sufficient for this.
I mean i would expect it to continue scanning other entries when it sees
an error on one. Is that not true?
The devices we're talking about here that do BIST are SCSI controllers etc.
that are normally of no interest to X.
>
> > > Then you need to switch to wait_event_timeout(). Its not terribly hard
> > > 8)
> >
> > Just complicating something that should be very simple.
>
> You are breaking an established user space API. Its not suprising this
> will break applications is it.
Are you sure these devices even return something useful during BIST?
As Brian said the device he was working with would just not answer,
leading to a bus abort. This would get ffffffff on a PC.
You could simulate this if you want, although I think a EBUSY or EIO
is better.
-Andi
On Iau, 2005-01-13 at 20:23, Andi Kleen wrote:
> > X needs to be able to find the device layout in order to build its PCI
> > mappings. Cached data is probably quite sufficient for this.
>
> I mean i would expect it to continue scanning other entries when it sees
> an error on one. Is that not true?
X needs to be able to find the device layout in order to build its PCI
mappings. If there are things mysteriously vanishing now and then its
not going to have valid mappings
On Thu, Jan 13, 2005 at 07:44:52PM +0000, Alan Cox wrote:
> On Iau, 2005-01-13 at 20:23, Andi Kleen wrote:
> > > X needs to be able to find the device layout in order to build its PCI
> > > mappings. Cached data is probably quite sufficient for this.
> >
> > I mean i would expect it to continue scanning other entries when it sees
> > an error on one. Is that not true?
>
> X needs to be able to find the device layout in order to build its PCI
> mappings. If there are things mysteriously vanishing now and then its
> not going to have valid mappings
I could see it as a problem when it happens on a bridge, but why
should it be a problem when some arbitary, nothing to do with X
leaf is temporarily not available?
-Andi
On Maw, 2005-01-11 at 17:33, Andi Kleen wrote:
> > User space does not expect to get dumped with -EBUSY randomly on PCI
>
> I think it's a reasonable thing to do. If you prefer you could fake a
> 0xffffffff read, that would look like busy or non existing hardware.
> But the errno would seem to be cleaner to me.
Either will break X.
> > static int pci_user_wait_access(struct pci_dev *pdev) {
> > wait_event(&pci_ucfg_wait, dev->block_ucfg_access == 0);
> > }
>
> I don't like this very much. What happens when the device
> doesn't get out of BIST for some reason?
Then you need to switch to wait_event_timeout(). Its not terribly hard
8)
On Iau, 2005-01-13 at 21:50, Andi Kleen wrote:
> I could see it as a problem when it happens on a bridge, but why
> should it be a problem when some arbitary, nothing to do with X
> leaf is temporarily not available?
Because X will believe that PCI address range is free and right now in
some circumstances older X may want to play with PCI layout itself.
Alan
On Sat, Jan 15, 2005 at 12:33:08AM +0000, Alan Cox wrote:
> On Iau, 2005-01-13 at 21:50, Andi Kleen wrote:
> > I could see it as a problem when it happens on a bridge, but why
> > should it be a problem when some arbitary, nothing to do with X
> > leaf is temporarily not available?
>
> Because X will believe that PCI address range is free and right now in
> some circumstances older X may want to play with PCI layout itself.
Yuck. sounds as broken as it can be. Hopefully nobody uses
such X servers anymore.
Then it won't work with this BIST hardware anyways - if it tries
to read config space of a device that is currently in BIST
it will just get a bus abort and no useful information.
The only point of this whole patch exercise is to avoid the bus abort
to satisfy the more strict hardware error checking on PPC64. On PCs
it really won't make any difference.
-Andi
On Sad, 2005-01-15 at 01:44, Andi Kleen wrote:
> Then it won't work with this BIST hardware anyways - if it tries
> to read config space of a device that is currently in BIST
> it will just get a bus abort and no useful information.
So it should wait to preseve a sane API at least for a short while and
if the user hasn't specified O_NDELAY. Its a compatibility consideration
> The only point of this whole patch exercise is to avoid the bus abort
> to satisfy the more strict hardware error checking on PPC64. On PCs
> it really won't make any difference.
I thought Ben wanted to do this for other PPC stuff ?
On Sat, 2005-01-15 at 01:01 +0000, Alan Cox wrote:
> On Sad, 2005-01-15 at 01:44, Andi Kleen wrote:
> > Then it won't work with this BIST hardware anyways - if it tries
> > to read config space of a device that is currently in BIST
> > it will just get a bus abort and no useful information.
>
> So it should wait to preseve a sane API at least for a short while and
> if the user hasn't specified O_NDELAY. Its a compatibility consideration
>
> > The only point of this whole patch exercise is to avoid the bus abort
> > to satisfy the more strict hardware error checking on PPC64. On PCs
> > it really won't make any difference.
>
> I thought Ben wanted to do this for other PPC stuff ?
Yes. On various models, I can turn off some ASIC cells, some of them
being PCI devices. I do that dynamically for power management typically.
Depending on the chipset, tapping one of those "disabled" cells with
config space accesses results in either all 1's or a lockup. On the
former, I currently do nothing special (but that cause problems with
various distro HW detection/configuration tools or the possible problem
with X you mentioned, among others), on the later, I have a special
filter in my pmac low level config space access routines to block access
to those sleeping devices (and currently to return all 1's).
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.
Ben.
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.
On Sun, 2005-01-16 at 00:58 +0000, 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.
> 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
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.
> 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
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).
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.
Andi Kleen wrote:
> As Brian said the device he was working with would just not answer,
> leading to a bus abort. This would get ffffffff 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
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 ffffffff 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
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.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13 15:57:54.000000000 -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;
+
+ 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);
+
+/**
+ * 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
+ **/
+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);
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again drivers/pci/pci-sysfs.c
--- linux-2.6.11-rc1/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c 2005-01-13 15:57:15.000000000 -0600
@@ -110,7 +110,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -119,7 +119,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -130,7 +130,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -154,7 +154,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -171,7 +171,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist_again drivers/pci/proc.c
--- linux-2.6.11-rc1/drivers/pci/proc.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/proc.c 2005-01-13 15:57:15.000000000 -0600
@@ -79,7 +79,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +88,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +97,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +106,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +115,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +150,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +159,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +168,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +177,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +186,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +474,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again drivers/pci/syscall.c
--- linux-2.6.11-rc1/drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c 2005-01-13 15:57:15.000000000 -0600
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist_again include/linux/pci.h
--- linux-2.6.11-rc1/include/linux/pci.h~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/include/linux/pci.h 2005-01-13 15:57:15.000000000 -0600
@@ -557,7 +557,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -801,6 +802,13 @@ static inline int pci_write_config_dword
return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
}
+int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
int pci_enable_device(struct pci_dev *dev);
int pci_enable_device_bars(struct pci_dev *dev, int mask);
void pci_disable_device(struct pci_dev *dev);
@@ -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 */
/* Include architecture-dependent settings and functions */
_
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.
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.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13 15:57:54.000000000 -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;
+
+ 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);
+
+/**
+ * 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
+ **/
+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);
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again drivers/pci/pci-sysfs.c
--- linux-2.6.11-rc1/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c 2005-01-13 15:57:15.000000000 -0600
@@ -110,7 +110,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -119,7 +119,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -130,7 +130,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -154,7 +154,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -171,7 +171,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist_again drivers/pci/proc.c
--- linux-2.6.11-rc1/drivers/pci/proc.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/proc.c 2005-01-13 15:57:15.000000000 -0600
@@ -79,7 +79,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +88,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +97,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +106,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +115,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +150,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +159,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +168,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +177,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +186,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +474,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again drivers/pci/syscall.c
--- linux-2.6.11-rc1/drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c 2005-01-13 15:57:15.000000000 -0600
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist_again include/linux/pci.h
--- linux-2.6.11-rc1/include/linux/pci.h~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/include/linux/pci.h 2005-01-13 15:57:15.000000000 -0600
@@ -557,7 +557,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -801,6 +802,13 @@ static inline int pci_write_config_dword
return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
}
+int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
int pci_enable_device(struct pci_dev *dev);
int pci_enable_device_bars(struct pci_dev *dev, int mask);
void pci_disable_device(struct pci_dev *dev);
@@ -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 */
/* Include architecture-dependent settings and functions */
_
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.
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
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
On Thu, 2005-01-27 at 15:53 +0000, 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.
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.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/access.c 2005-01-13 15:57:54.000000000 -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;
+
+ 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);
+
+/**
+ * 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
+ **/
+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);
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again drivers/pci/pci-sysfs.c
--- linux-2.6.11-rc1/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/pci-sysfs.c 2005-01-13 15:57:15.000000000 -0600
@@ -110,7 +110,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -119,7 +119,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -130,7 +130,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -154,7 +154,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -171,7 +171,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist_again drivers/pci/proc.c
--- linux-2.6.11-rc1/drivers/pci/proc.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/proc.c 2005-01-13 15:57:15.000000000 -0600
@@ -79,7 +79,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +88,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +97,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +106,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +115,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +150,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +159,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +168,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +177,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +186,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +474,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again drivers/pci/syscall.c
--- linux-2.6.11-rc1/drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/drivers/pci/syscall.c 2005-01-13 15:57:15.000000000 -0600
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist_again include/linux/pci.h
--- linux-2.6.11-rc1/include/linux/pci.h~pci_block_user_config_io_during_bist_again 2005-01-13 15:57:15.000000000 -0600
+++ linux-2.6.11-rc1-bjking1/include/linux/pci.h 2005-01-13 15:57:15.000000000 -0600
@@ -557,7 +557,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -801,6 +802,13 @@ static inline int pci_write_config_dword
return pci_bus_write_config_dword (dev->bus, dev->devfn, where, val);
}
+int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
int pci_enable_device(struct pci_dev *dev);
int pci_enable_device_bars(struct pci_dev *dev, int mask);
void pci_disable_device(struct pci_dev *dev);
@@ -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 */
/* Include architecture-dependent settings and functions */
_
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
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.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01 08:39:57.000000000 -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; \
+ 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.
+ **/
+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_GPL(pci_block_user_cfg_access);
+
+/**
+ * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows userspace PCI config accesses to resume.
+ **/
+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_GPL(pci_unblock_user_cfg_access);
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again drivers/pci/pci-sysfs.c
--- linux-2.6.11-rc2-bk9/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again 2005-02-01 08:35:29.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c 2005-02-01 08:35:29.000000000 -0600
@@ -110,7 +110,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -119,7 +119,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -130,7 +130,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -154,7 +154,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -171,7 +171,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist_again drivers/pci/proc.c
--- linux-2.6.11-rc2-bk9/drivers/pci/proc.c~pci_block_user_config_io_during_bist_again 2005-02-01 08:35:29.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c 2005-02-01 08:35:29.000000000 -0600
@@ -79,7 +79,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +88,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +97,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +106,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +115,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +150,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +159,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +168,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +177,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +186,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +474,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again drivers/pci/syscall.c
--- linux-2.6.11-rc2-bk9/drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again 2005-02-01 08:35:29.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c 2005-02-01 08:35:29.000000000 -0600
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist_again include/linux/pci.h
--- linux-2.6.11-rc2-bk9/include/linux/pci.h~pci_block_user_config_io_during_bist_again 2005-02-01 08:35:29.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h 2005-02-01 08:41:23.000000000 -0600
@@ -557,7 +557,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -896,6 +897,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 */
/* Include architecture-dependent settings and functions */
@@ -947,6 +950,8 @@ static inline void pci_unregister_driver
static inline int pci_find_capability (struct pci_dev *dev, int cap) {return 0; }
static inline int pci_find_ext_capability (struct pci_dev *dev, int cap) {return 0; }
static inline const struct pci_device_id *pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev) { return NULL; }
+static inline void pci_block_user_cfg_access(struct pci_dev *dev) { }
+static inline void pci_unblock_user_cfg_access(struct pci_dev *dev) { }
/* Power management related routines */
static inline int pci_save_state(struct pci_dev *dev) { return 0; }
diff -puN drivers/pci/pci.h~pci_block_user_config_io_during_bist_again drivers/pci/pci.h
--- linux-2.6.11-rc2-bk9/drivers/pci/pci.h~pci_block_user_config_io_during_bist_again 2005-02-01 08:36:58.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h 2005-02-01 08:37:53.000000000 -0600
@@ -11,6 +11,13 @@ extern int pci_bus_alloc_resource(struct
void (*alignf)(void *, struct resource *,
unsigned long, unsigned long),
void *alignf_data);
+extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+extern int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+extern int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+extern int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
/* PCI /proc functions */
#ifdef CONFIG_PROC_FS
extern int pci_proc_attach_device(struct pci_dev *dev);
_
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.000000000 -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
Matthew Wilcox wrote:
>>+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-01
08:39:57.000000000 -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
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
On Tue, Feb 01, 2005 at 03:44:00PM +0000, 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
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
On Tue, 2005-02-01 at 17:47 +0000, 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.
On Tue, 2005-02-01 at 15:44 +0000, 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.
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.
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.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/access.c 2005-02-02 09:15:30.000000000 -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, \
+ 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 be bit bucketed and reads will return the
+ * data saved using pci_save_state for the first 64 bytes of config
+ * space and return 0xff for all other config reads.
+ **/
+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_GPL(pci_block_user_cfg_access);
+
+/**
+ * pci_unblock_user_cfg_access - Unblock userspace PCI config reads/writes
+ * @dev: pci device struct
+ *
+ * This function allows userspace PCI config accesses to resume.
+ **/
+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_GPL(pci_unblock_user_cfg_access);
diff -puN drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again drivers/pci/pci-sysfs.c
--- linux-2.6.11-rc2-bk9/drivers/pci/pci-sysfs.c~pci_block_user_config_io_during_bist_again 2005-02-02 08:58:53.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci-sysfs.c 2005-02-02 08:58:53.000000000 -0600
@@ -110,7 +110,7 @@ pci_read_config(struct kobject *kobj, ch
while (off & 3) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
if (--size == 0)
@@ -119,7 +119,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 3) {
unsigned int val;
- pci_read_config_dword(dev, off, &val);
+ pci_user_read_config_dword(dev, off, &val);
buf[off - init_off] = val & 0xff;
buf[off - init_off + 1] = (val >> 8) & 0xff;
buf[off - init_off + 2] = (val >> 16) & 0xff;
@@ -130,7 +130,7 @@ pci_read_config(struct kobject *kobj, ch
while (size > 0) {
unsigned char val;
- pci_read_config_byte(dev, off, &val);
+ pci_user_read_config_byte(dev, off, &val);
buf[off - init_off] = val;
off++;
--size;
@@ -154,7 +154,7 @@ pci_write_config(struct kobject *kobj, c
}
while (off & 3) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
if (--size == 0)
break;
@@ -171,7 +171,7 @@ pci_write_config(struct kobject *kobj, c
}
while (size > 0) {
- pci_write_config_byte(dev, off, buf[off - init_off]);
+ pci_user_write_config_byte(dev, off, buf[off - init_off]);
off++;
--size;
}
diff -puN drivers/pci/proc.c~pci_block_user_config_io_during_bist_again drivers/pci/proc.c
--- linux-2.6.11-rc2-bk9/drivers/pci/proc.c~pci_block_user_config_io_during_bist_again 2005-02-02 08:58:53.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/proc.c 2005-02-02 08:58:53.000000000 -0600
@@ -16,6 +16,8 @@
#include <asm/uaccess.h>
#include <asm/byteorder.h>
+#include "pci.h"
+
static int proc_initialized; /* = 0 */
static loff_t
@@ -79,7 +81,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 1) && cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -88,7 +90,7 @@ proc_bus_pci_read(struct file *file, cha
if ((pos & 3) && cnt > 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -97,7 +99,7 @@ proc_bus_pci_read(struct file *file, cha
while (cnt >= 4) {
unsigned int val;
- pci_read_config_dword(dev, pos, &val);
+ pci_user_read_config_dword(dev, pos, &val);
__put_user(cpu_to_le32(val), (unsigned int __user *) buf);
buf += 4;
pos += 4;
@@ -106,7 +108,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt >= 2) {
unsigned short val;
- pci_read_config_word(dev, pos, &val);
+ pci_user_read_config_word(dev, pos, &val);
__put_user(cpu_to_le16(val), (unsigned short __user *) buf);
buf += 2;
pos += 2;
@@ -115,7 +117,7 @@ proc_bus_pci_read(struct file *file, cha
if (cnt) {
unsigned char val;
- pci_read_config_byte(dev, pos, &val);
+ pci_user_read_config_byte(dev, pos, &val);
__put_user(val, buf);
buf++;
pos++;
@@ -150,7 +152,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 1) && cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -159,7 +161,7 @@ proc_bus_pci_write(struct file *file, co
if ((pos & 3) && cnt > 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -168,7 +170,7 @@ proc_bus_pci_write(struct file *file, co
while (cnt >= 4) {
unsigned int val;
__get_user(val, (unsigned int __user *) buf);
- pci_write_config_dword(dev, pos, le32_to_cpu(val));
+ pci_user_write_config_dword(dev, pos, le32_to_cpu(val));
buf += 4;
pos += 4;
cnt -= 4;
@@ -177,7 +179,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt >= 2) {
unsigned short val;
__get_user(val, (unsigned short __user *) buf);
- pci_write_config_word(dev, pos, le16_to_cpu(val));
+ pci_user_write_config_word(dev, pos, le16_to_cpu(val));
buf += 2;
pos += 2;
cnt -= 2;
@@ -186,7 +188,7 @@ proc_bus_pci_write(struct file *file, co
if (cnt) {
unsigned char val;
__get_user(val, buf);
- pci_write_config_byte(dev, pos, val);
+ pci_user_write_config_byte(dev, pos, val);
buf++;
pos++;
cnt--;
@@ -474,10 +476,10 @@ static int show_dev_config(struct seq_fi
drv = pci_dev_driver(dev);
- pci_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
- pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
- pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
- pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
+ pci_user_read_config_dword(dev, PCI_CLASS_REVISION, &class_rev);
+ pci_user_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
+ pci_user_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
+ pci_user_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
diff -puN drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again drivers/pci/syscall.c
--- linux-2.6.11-rc2-bk9/drivers/pci/syscall.c~pci_block_user_config_io_during_bist_again 2005-02-02 08:58:53.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/syscall.c 2005-02-02 08:58:53.000000000 -0600
@@ -13,7 +13,7 @@
#include <linux/smp_lock.h>
#include <linux/syscalls.h>
#include <asm/uaccess.h>
-
+#include "pci.h"
asmlinkage long
sys_pciconfig_read(unsigned long bus, unsigned long dfn,
@@ -38,13 +38,13 @@ sys_pciconfig_read(unsigned long bus, un
lock_kernel();
switch (len) {
case 1:
- cfg_ret = pci_read_config_byte(dev, off, &byte);
+ cfg_ret = pci_user_read_config_byte(dev, off, &byte);
break;
case 2:
- cfg_ret = pci_read_config_word(dev, off, &word);
+ cfg_ret = pci_user_read_config_word(dev, off, &word);
break;
case 4:
- cfg_ret = pci_read_config_dword(dev, off, &dword);
+ cfg_ret = pci_user_read_config_dword(dev, off, &dword);
break;
default:
err = -EINVAL;
@@ -112,7 +112,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(byte, (u8 __user *)buf);
if (err)
break;
- err = pci_write_config_byte(dev, off, byte);
+ err = pci_user_write_config_byte(dev, off, byte);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -121,7 +121,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(word, (u16 __user *)buf);
if (err)
break;
- err = pci_write_config_word(dev, off, word);
+ err = pci_user_write_config_word(dev, off, word);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
@@ -130,7 +130,7 @@ sys_pciconfig_write(unsigned long bus, u
err = get_user(dword, (u32 __user *)buf);
if (err)
break;
- err = pci_write_config_dword(dev, off, dword);
+ err = pci_user_write_config_dword(dev, off, dword);
if (err != PCIBIOS_SUCCESSFUL)
err = -EIO;
break;
diff -puN include/linux/pci.h~pci_block_user_config_io_during_bist_again include/linux/pci.h
--- linux-2.6.11-rc2-bk9/include/linux/pci.h~pci_block_user_config_io_during_bist_again 2005-02-02 08:58:53.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/include/linux/pci.h 2005-02-02 08:58:53.000000000 -0600
@@ -557,7 +557,8 @@ struct pci_dev {
/* keep track of device state */
unsigned int is_enabled:1; /* pci_enable_device has been called */
unsigned int is_busmaster:1; /* device is busmaster */
-
+ unsigned int block_ucfg_access:1; /* userspace config space access is blocked */
+
u32 saved_config_space[16]; /* config space saved at suspend time */
struct bin_attribute *rom_attr; /* attribute descriptor for sysfs ROM entry */
int rom_attr_enabled; /* has display of the rom attribute been enabled? */
@@ -896,6 +897,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 */
/* Include architecture-dependent settings and functions */
@@ -947,6 +950,8 @@ static inline void pci_unregister_driver
static inline int pci_find_capability (struct pci_dev *dev, int cap) {return 0; }
static inline int pci_find_ext_capability (struct pci_dev *dev, int cap) {return 0; }
static inline const struct pci_device_id *pci_match_device(const struct pci_device_id *ids, const struct pci_dev *dev) { return NULL; }
+static inline void pci_block_user_cfg_access(struct pci_dev *dev) { }
+static inline void pci_unblock_user_cfg_access(struct pci_dev *dev) { }
/* Power management related routines */
static inline int pci_save_state(struct pci_dev *dev) { return 0; }
diff -puN drivers/pci/pci.h~pci_block_user_config_io_during_bist_again drivers/pci/pci.h
--- linux-2.6.11-rc2-bk9/drivers/pci/pci.h~pci_block_user_config_io_during_bist_again 2005-02-02 08:58:53.000000000 -0600
+++ linux-2.6.11-rc2-bk9-bjking1/drivers/pci/pci.h 2005-02-02 08:58:53.000000000 -0600
@@ -11,6 +11,13 @@ extern int pci_bus_alloc_resource(struct
void (*alignf)(void *, struct resource *,
unsigned long, unsigned long),
void *alignf_data);
+extern int pci_user_read_config_byte(struct pci_dev *dev, int where, u8 *val);
+extern int pci_user_read_config_word(struct pci_dev *dev, int where, u16 *val);
+extern int pci_user_read_config_dword(struct pci_dev *dev, int where, u32 *val);
+extern int pci_user_write_config_byte(struct pci_dev *dev, int where, u8 val);
+extern int pci_user_write_config_word(struct pci_dev *dev, int where, u16 val);
+extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
+
/* PCI /proc functions */
#ifdef CONFIG_PROC_FS
extern int pci_proc_attach_device(struct pci_dev *dev);
_
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