[prev in list] [next in list] [prev in thread] [next in thread]
List: linux-pci
Subject: Re: [PATCH 1/2] pci: remove pci_mmap_page_range wrapper
From: Bjorn Helgaas <helgaas () kernel ! org>
Date: 2022-07-29 17:12:15
Message-ID: 20220729171215.GA464747 () bhelgaas
[Download RAW message or body]
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
>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic