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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to