| Message ID | 20220715153617.3393420-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] pci: remove pci_mmap_page_range wrapper | expand |
On Fri, Jul 15, 2022 at 05:36:16PM +0200, Arnd Bergmann wrote: > From: Arnd Bergmann <arnd@arndb.de> > > The ARCH_GENERIC_PCI_MMAP_RESOURCE symbol came up in a recent discussion, > and I noticed that this was left behind by an unfinished cleanup from > 2017. > > The only architecture that still relies on providing its > own pci_mmap_page_range() helper instead of using the generic > pci_mmap_resource_range() is sparc. Presumably the reasons for this have > not changed, but at least this can be simplified by converting sparc to > use the same interface as the others. > > The only difference between the two is the device specific offset that > gets added to or subtracted from vma->vm_pgoff. > > Change the only caller of pci_mmap_page_range() in common code to subtract > this offset and call the modern interface, while adding it back in the > sparc implementation to preserve the existing behavior. > > This removes the complexities of the dual interfaces from the common code, > and keeps it all specific to the sparc architecture code. According to > David Miller, the sparc code lets user space poke into the VGA I/O port > registers by mmapping the I/O space of the parent bridge device, which is > something that the generic pci_mmap_resource_range() code apparently > does not. > > Cc: David Woodhouse <dwmw2@infradead.org> > Cc: David S. Miller <davem@davemloft.net> > Cc: Stafford Horne <shorne@gmail.com> > Link: https://lore.kernel.org/lkml/1519887203.622.3.camel@infradead.org/t/ > Link: https://lore.kernel.org/lkml/20220714214657.2402250-3-shorne@gmail.com/ > Signed-off-by: Arnd Bergmann <arnd@arndb.de> I applied both these patches on a pci/resource branch for v5.20, thanks! Sparc folks, let me know if you object or prefer to take these via your tree. > --- > Documentation/PCI/sysfs-pci.rst | 2 +- > arch/sparc/kernel/pci.c | 13 +++++++--- > drivers/pci/mmap.c | 44 --------------------------------- > drivers/pci/proc.c | 7 +++++- > include/linux/pci.h | 12 +-------- > 5 files changed, 17 insertions(+), 61 deletions(-) > > diff --git a/Documentation/PCI/sysfs-pci.rst b/Documentation/PCI/sysfs-pci.rst > index 742fbd21dc1f..f495185aa88a 100644 > --- a/Documentation/PCI/sysfs-pci.rst > +++ b/Documentation/PCI/sysfs-pci.rst > @@ -125,7 +125,7 @@ implementation of that functionality. To support the historical interface of > mmap() through files in /proc/bus/pci, platforms may also set HAVE_PCI_MMAP. > > Alternatively, platforms which set HAVE_PCI_MMAP may provide their own > -implementation of pci_mmap_page_range() instead of defining > +implementation of pci_mmap_resource_range() instead of defining > ARCH_GENERIC_PCI_MMAP_RESOURCE. > > Platforms which support write-combining maps of PCI resources must define > diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c > index 31b0c1983286..f580db840bf7 100644 > --- a/arch/sparc/kernel/pci.c > +++ b/arch/sparc/kernel/pci.c > @@ -876,17 +876,22 @@ static void __pci_mmap_set_pgprot(struct pci_dev *dev, struct vm_area_struct *vm > > /* Perform the actual remap of the pages for a PCI device mapping, as appropriate > * for this architecture. The region in the process to map is described by vm_start > - * and vm_end members of VMA, the base physical address is found in vm_pgoff. > + * and vm_end members of VMA, the BAR relative address is found in vm_pgoff. > * The pci device structure is provided so that architectures may make mapping > * decisions on a per-device or per-bus basis. > * > * Returns a negative error code on failure, zero on success. > */ > -int pci_mmap_page_range(struct pci_dev *dev, int bar, > - struct vm_area_struct *vma, > - enum pci_mmap_state mmap_state, int write_combine) > +int pci_mmap_resource_range(struct pci_dev *dev, int bar, > + struct vm_area_struct *vma, > + enum pci_mmap_state mmap_state, int write_combine) > { > int ret; > + resource_size_t start, end; > + > + /* convert per-BAR address to PCI bus address */ > + pci_resource_to_user(dev, bar, &dev->resource[bar], &start, &end); > + vma->vm_pgoff += start >> PAGE_SHIFT; > > ret = __pci_mmap_make_offset(dev, vma, mmap_state); > if (ret < 0) > diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c > index b8c9011987f4..4504039056d1 100644 > --- a/drivers/pci/mmap.c > +++ b/drivers/pci/mmap.c > @@ -13,27 +13,6 @@ > > #ifdef ARCH_GENERIC_PCI_MMAP_RESOURCE > > -/* > - * Modern setup: generic pci_mmap_resource_range(), and implement the legacy > - * pci_mmap_page_range() (if needed) as a wrapper round it. > - */ > - > -#ifdef HAVE_PCI_MMAP > -int pci_mmap_page_range(struct pci_dev *pdev, int bar, > - struct vm_area_struct *vma, > - enum pci_mmap_state mmap_state, int write_combine) > -{ > - resource_size_t start, end; > - > - pci_resource_to_user(pdev, bar, &pdev->resource[bar], &start, &end); > - > - /* Adjust vm_pgoff to be the offset within the resource */ > - vma->vm_pgoff -= start >> PAGE_SHIFT; > - return pci_mmap_resource_range(pdev, bar, vma, mmap_state, > - write_combine); > -} > -#endif > - > static const struct vm_operations_struct pci_phys_vm_ops = { > #ifdef CONFIG_HAVE_IOREMAP_PROT > .access = generic_access_phys, > @@ -70,27 +49,4 @@ int pci_mmap_resource_range(struct pci_dev *pdev, int bar, > vma->vm_page_prot); > } > > -#elif defined(HAVE_PCI_MMAP) /* && !ARCH_GENERIC_PCI_MMAP_RESOURCE */ > - > -/* > - * Legacy setup: Implement pci_mmap_resource_range() as a wrapper around > - * the architecture's pci_mmap_page_range(), converting to "user visible" > - * addresses as necessary. > - */ > - > -int pci_mmap_resource_range(struct pci_dev *pdev, int bar, > - struct vm_area_struct *vma, > - enum pci_mmap_state mmap_state, int write_combine) > -{ > - resource_size_t start, end; > - > - /* > - * pci_mmap_page_range() expects the same kind of entry as coming > - * from /proc/bus/pci/ which is a "user visible" value. If this is > - * different from the resource itself, arch will do necessary fixup. > - */ > - pci_resource_to_user(pdev, bar, &pdev->resource[bar], &start, &end); > - vma->vm_pgoff += start >> PAGE_SHIFT; > - return pci_mmap_page_range(pdev, bar, vma, mmap_state, write_combine); > -} > #endif > diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c > index 31b26d8ea6cc..f967709082d6 100644 > --- a/drivers/pci/proc.c > +++ b/drivers/pci/proc.c > @@ -244,6 +244,7 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma) > { > struct pci_dev *dev = pde_data(file_inode(file)); > struct pci_filp_private *fpriv = file->private_data; > + resource_size_t start, end; > int i, ret, write_combine = 0, res_bit = IORESOURCE_MEM; > > if (!capable(CAP_SYS_RAWIO) || > @@ -278,7 +279,11 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma) > iomem_is_exclusive(dev->resource[i].start)) > return -EINVAL; > > - ret = pci_mmap_page_range(dev, i, vma, > + pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); > + > + /* Adjust vm_pgoff to be the offset within the resource */ > + vma->vm_pgoff -= start >> PAGE_SHIFT; > + ret = pci_mmap_resource_range(dev, i, vma, > fpriv->mmap_state, write_combine); > if (ret < 0) > return ret; > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 81a57b498f22..060af91bafcd 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -1909,24 +1909,14 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, > > #include <asm/pci.h> > > -/* These two functions provide almost identical functionality. Depending > - * on the architecture, one will be implemented as a wrapper around the > - * other (in drivers/pci/mmap.c). > - * > +/* > * pci_mmap_resource_range() maps a specific BAR, and vm->vm_pgoff > * is expected to be an offset within that region. > * > - * pci_mmap_page_range() is the legacy architecture-specific interface, > - * which accepts a "user visible" resource address converted by > - * pci_resource_to_user(), as used in the legacy mmap() interface in > - * /proc/bus/pci/. > */ > int pci_mmap_resource_range(struct pci_dev *dev, int bar, > struct vm_area_struct *vma, > enum pci_mmap_state mmap_state, int write_combine); > -int pci_mmap_page_range(struct pci_dev *pdev, int bar, > - struct vm_area_struct *vma, > - enum pci_mmap_state mmap_state, int write_combine); > > #ifndef arch_can_pci_mmap_wc > #define arch_can_pci_mmap_wc() 0 > -- > 2.29.2 >
diff --git a/Documentation/PCI/sysfs-pci.rst b/Documentation/PCI/sysfs-pci.rst index 742fbd21dc1f..f495185aa88a 100644 --- a/Documentation/PCI/sysfs-pci.rst +++ b/Documentation/PCI/sysfs-pci.rst @@ -125,7 +125,7 @@ implementation of that functionality. To support the historical interface of mmap() through files in /proc/bus/pci, platforms may also set HAVE_PCI_MMAP. Alternatively, platforms which set HAVE_PCI_MMAP may provide their own -implementation of pci_mmap_page_range() instead of defining +implementation of pci_mmap_resource_range() instead of defining ARCH_GENERIC_PCI_MMAP_RESOURCE. Platforms which support write-combining maps of PCI resources must define diff --git a/arch/sparc/kernel/pci.c b/arch/sparc/kernel/pci.c index 31b0c1983286..f580db840bf7 100644 --- a/arch/sparc/kernel/pci.c +++ b/arch/sparc/kernel/pci.c @@ -876,17 +876,22 @@ static void __pci_mmap_set_pgprot(struct pci_dev *dev, struct vm_area_struct *vm /* Perform the actual remap of the pages for a PCI device mapping, as appropriate * for this architecture. The region in the process to map is described by vm_start - * and vm_end members of VMA, the base physical address is found in vm_pgoff. + * and vm_end members of VMA, the BAR relative address is found in vm_pgoff. * The pci device structure is provided so that architectures may make mapping * decisions on a per-device or per-bus basis. * * Returns a negative error code on failure, zero on success. */ -int pci_mmap_page_range(struct pci_dev *dev, int bar, - struct vm_area_struct *vma, - enum pci_mmap_state mmap_state, int write_combine) +int pci_mmap_resource_range(struct pci_dev *dev, int bar, + struct vm_area_struct *vma, + enum pci_mmap_state mmap_state, int write_combine) { int ret; + resource_size_t start, end; + + /* convert per-BAR address to PCI bus address */ + pci_resource_to_user(dev, bar, &dev->resource[bar], &start, &end); + vma->vm_pgoff += start >> PAGE_SHIFT; ret = __pci_mmap_make_offset(dev, vma, mmap_state); if (ret < 0) diff --git a/drivers/pci/mmap.c b/drivers/pci/mmap.c index b8c9011987f4..4504039056d1 100644 --- a/drivers/pci/mmap.c +++ b/drivers/pci/mmap.c @@ -13,27 +13,6 @@ #ifdef ARCH_GENERIC_PCI_MMAP_RESOURCE -/* - * Modern setup: generic pci_mmap_resource_range(), and implement the legacy - * pci_mmap_page_range() (if needed) as a wrapper round it. - */ - -#ifdef HAVE_PCI_MMAP -int pci_mmap_page_range(struct pci_dev *pdev, int bar, - struct vm_area_struct *vma, - enum pci_mmap_state mmap_state, int write_combine) -{ - resource_size_t start, end; - - pci_resource_to_user(pdev, bar, &pdev->resource[bar], &start, &end); - - /* Adjust vm_pgoff to be the offset within the resource */ - vma->vm_pgoff -= start >> PAGE_SHIFT; - return pci_mmap_resource_range(pdev, bar, vma, mmap_state, - write_combine); -} -#endif - static const struct vm_operations_struct pci_phys_vm_ops = { #ifdef CONFIG_HAVE_IOREMAP_PROT .access = generic_access_phys, @@ -70,27 +49,4 @@ int pci_mmap_resource_range(struct pci_dev *pdev, int bar, vma->vm_page_prot); } -#elif defined(HAVE_PCI_MMAP) /* && !ARCH_GENERIC_PCI_MMAP_RESOURCE */ - -/* - * Legacy setup: Implement pci_mmap_resource_range() as a wrapper around - * the architecture's pci_mmap_page_range(), converting to "user visible" - * addresses as necessary. - */ - -int pci_mmap_resource_range(struct pci_dev *pdev, int bar, - struct vm_area_struct *vma, - enum pci_mmap_state mmap_state, int write_combine) -{ - resource_size_t start, end; - - /* - * pci_mmap_page_range() expects the same kind of entry as coming - * from /proc/bus/pci/ which is a "user visible" value. If this is - * different from the resource itself, arch will do necessary fixup. - */ - pci_resource_to_user(pdev, bar, &pdev->resource[bar], &start, &end); - vma->vm_pgoff += start >> PAGE_SHIFT; - return pci_mmap_page_range(pdev, bar, vma, mmap_state, write_combine); -} #endif diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c index 31b26d8ea6cc..f967709082d6 100644 --- a/drivers/pci/proc.c +++ b/drivers/pci/proc.c @@ -244,6 +244,7 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma) { struct pci_dev *dev = pde_data(file_inode(file)); struct pci_filp_private *fpriv = file->private_data; + resource_size_t start, end; int i, ret, write_combine = 0, res_bit = IORESOURCE_MEM; if (!capable(CAP_SYS_RAWIO) || @@ -278,7 +279,11 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma) iomem_is_exclusive(dev->resource[i].start)) return -EINVAL; - ret = pci_mmap_page_range(dev, i, vma, + pci_resource_to_user(dev, i, &dev->resource[i], &start, &end); + + /* Adjust vm_pgoff to be the offset within the resource */ + vma->vm_pgoff -= start >> PAGE_SHIFT; + ret = pci_mmap_resource_range(dev, i, vma, fpriv->mmap_state, write_combine); if (ret < 0) return ret; diff --git a/include/linux/pci.h b/include/linux/pci.h index 81a57b498f22..060af91bafcd 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1909,24 +1909,14 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs, #include <asm/pci.h> -/* These two functions provide almost identical functionality. Depending - * on the architecture, one will be implemented as a wrapper around the - * other (in drivers/pci/mmap.c). - * +/* * pci_mmap_resource_range() maps a specific BAR, and vm->vm_pgoff * is expected to be an offset within that region. * - * pci_mmap_page_range() is the legacy architecture-specific interface, - * which accepts a "user visible" resource address converted by - * pci_resource_to_user(), as used in the legacy mmap() interface in - * /proc/bus/pci/. */ int pci_mmap_resource_range(struct pci_dev *dev, int bar, struct vm_area_struct *vma, enum pci_mmap_state mmap_state, int write_combine); -int pci_mmap_page_range(struct pci_dev *pdev, int bar, - struct vm_area_struct *vma, - enum pci_mmap_state mmap_state, int write_combine); #ifndef arch_can_pci_mmap_wc #define arch_can_pci_mmap_wc() 0