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

[Matthew Wilcox]

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
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/