Re: [PATCH 1/2] pci: Block config access during BIST

Brian King
2004-12-03T15:27:22Z

Greg - do you have any comments on this patch?

Thanks

-Brian

Brian King wrote:
> Benjamin Herrenschmidt wrote:
>
>>> I thought about that when coding this up and thought it would
>>> be better to simply have the function do what it advertises and no
>>> more. Seems strange that a function called pci_block_config_access
>>> would go and do a bunch of pci config accesses, but we can
>>> certainly add it if you like.
>>
>>
>>
>> Well, considering that when blocked, reads return data from the cache,
>> it makes sense to fill the cache when blocking ... or we'll end up
>> returning crap.
>
>
> Here is an updated patch with the added pci_save_state call.
>
>
>
> ------------------------------------------------------------------------
>
>
> 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 <brking@us.ibm.com>
> ---
>
>
>
> ---
>
> linux-2.6.10-rc2-bk5-bjking1/drivers/pci/access.c | 104 ++++++++++++++++++++++
> linux-2.6.10-rc2-bk5-bjking1/include/linux/pci.h | 37 ++-----
> 2 files changed, 115 insertions(+), 26 deletions(-)
>
> diff -puN drivers/pci/access.c~pci_block_config_io_during_bist drivers/pci/access.c
> --- linux-2.6.10-rc2-bk5/drivers/pci/access.c~pci_block_config_io_during_bist 2004-11-20 16:07:19.000000000 -0600
> +++ linux-2.6.10-rc2-bk5-bjking1/drivers/pci/access.c 2004-11-21 10:42:10.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);
> diff -puN include/linux/pci.h~pci_block_config_io_during_bist include/linux/pci.h
> --- linux-2.6.10-rc2-bk5/include/linux/pci.h~pci_block_config_io_during_bist 2004-11-20 16:07:19.000000000 -0600
> +++ linux-2.6.10-rc2-bk5-bjking1/include/linux/pci.h 2004-11-20 16:07:19.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 */
>
> _

--
Brian King
eServer Storage I/O
IBM Linux Technology Center