| Message ID | 20180604154803.14185-1-mr.nuke.me@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Mon, Jun 04, 2018 at 10:48:02AM -0500, Alexandru Gagniuc wrote: > +++ b/drivers/pci/access.c > @@ -223,16 +223,9 @@ int pci_user_read_config_##size \ > (struct pci_dev *dev, int pos, type *val) \ > { \ > int ret = PCIBIOS_SUCCESSFUL; \ > - u32 data = -1; \ > if (PCI_##size##_BAD) \ > return -EINVAL; \ > - raw_spin_lock_irq(&pci_lock); \ > - if (unlikely(dev->block_cfg_access)) \ > - pci_wait_cfg(dev); \ > - ret = dev->bus->ops->read(dev->bus, dev->devfn, \ > - pos, sizeof(type), &data); \ > - raw_spin_unlock_irq(&pci_lock); \ > - *val = (type)data; \ > + ret = pci_read_config_##size(dev, pos, val); \ > return pcibios_err_to_errno(ret); \ > } \ If it wasn't for the block_cfg_access check for user access, this would have been a nice code reuse cleanup.
On 6/4/2018 11:09 AM, Keith Busch wrote: > On Mon, Jun 04, 2018 at 10:48:02AM -0500, Alexandru Gagniuc wrote: >> +++ b/drivers/pci/access.c >> @@ -223,16 +223,9 @@ int pci_user_read_config_##size \ >> (struct pci_dev *dev, int pos, type *val) \ >> { \ >> int ret = PCIBIOS_SUCCESSFUL; \ >> - u32 data = -1; \ >> if (PCI_##size##_BAD) \ >> return -EINVAL; \ >> - raw_spin_lock_irq(&pci_lock); \ >> - if (unlikely(dev->block_cfg_access)) \ >> - pci_wait_cfg(dev); \ >> - ret = dev->bus->ops->read(dev->bus, dev->devfn, \ >> - pos, sizeof(type), &data); \ >> - raw_spin_unlock_irq(&pci_lock); \ >> - *val = (type)data; \ >> + ret = pci_read_config_##size(dev, pos, val); \ >> return pcibios_err_to_errno(ret); \ >> } \ > > If it wasn't for the block_cfg_access check for user access, this would > have been a nice code reuse cleanup. Great fresh catch! I'll get that fixed in v2. Alex
diff --git a/drivers/pci/access.c b/drivers/pci/access.c index a3ad2fe185b9..6db2a8713c85 100644 --- a/drivers/pci/access.c +++ b/drivers/pci/access.c @@ -223,16 +223,9 @@ int pci_user_read_config_##size \ (struct pci_dev *dev, int pos, type *val) \ { \ int ret = PCIBIOS_SUCCESSFUL; \ - u32 data = -1; \ if (PCI_##size##_BAD) \ return -EINVAL; \ - raw_spin_lock_irq(&pci_lock); \ - if (unlikely(dev->block_cfg_access)) \ - pci_wait_cfg(dev); \ - ret = dev->bus->ops->read(dev->bus, dev->devfn, \ - pos, sizeof(type), &data); \ - raw_spin_unlock_irq(&pci_lock); \ - *val = (type)data; \ + ret = pci_read_config_##size(dev, pos, val); \ return pcibios_err_to_errno(ret); \ } \ EXPORT_SYMBOL_GPL(pci_user_read_config_##size); @@ -245,12 +238,7 @@ int pci_user_write_config_##size \ int ret = PCIBIOS_SUCCESSFUL; \ if (PCI_##size##_BAD) \ return -EINVAL; \ - raw_spin_lock_irq(&pci_lock); \ - if (unlikely(dev->block_cfg_access)) \ - pci_wait_cfg(dev); \ - ret = dev->bus->ops->write(dev->bus, dev->devfn, \ - pos, sizeof(type), val); \ - raw_spin_unlock_irq(&pci_lock); \ + ret = pci_write_config_##size(dev, pos, val); \ return pcibios_err_to_errno(ret); \ } \ EXPORT_SYMBOL_GPL(pci_user_write_config_##size);
pci_read/write_config*() functions have several safeguards to prevent stallling accesses when a device is removed. However, their "_user_" counterparts use a different code path. To make sure that safeguards are used for userspace PCI config accesses, piggyback the "_user_" functions on the in-kernel pci_read/write_config*(). Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com> --- drivers/pci/access.c | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-)