Re: [PATCH 1/1] pci: Block config access during BIST (resend)

[Brian King <brking@us.ibm.com>]

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