* [PATCH v1 1/3] PCI: Ignore write combining when mapping I/O port space
2016-06-09 18:20 [PATCH v1 0/3] PCI: Clean up write combining usage for user mmap Bjorn Helgaas
@ 2016-06-09 18:20 ` Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 2/3] powerpc/pci: Remove __pci_mmap_set_pgprot() Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 3/3] microblaze/PCI: Remove useless __pci_mmap_set_pgprot() Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2016-06-09 18:20 UTC (permalink / raw)
To: Yinghai Lu, Benjamin Herrenschmidt, Michal Simek, Paul Mackerras,
Michael Ellerman
Cc: linux-pci, linuxppc-dev, linux-kernel
PCI exposes files like /proc/bus/pci/00/00.0 in procfs. These files
support operations like this:
ioctl(fd, PCIIOC_MMAP_IS_IO); # request I/O port space
ioctl(fd, PCIIOC_WRITE_COMBINE, 1); # request write-combining
mmap(fd, ...)
Write combining is useful on PCI memory space, but I don't think it makes
sense on PCI I/O port space.
We *could* change proc_bus_pci_ioctl() to make it impossible to set
mmap_state == pci_mmap_io and write_combine at the same time, but that
would break the following sequence, which is currently legal:
mmap(fd, ...) # default is I/O, non-combining
ioctl(fd, PCIIOC_WRITE_COMBINE, 1); # request write-combining
ioctl(fd, PCIIOC_MMAP_IS_MEM); # request memory space
mmap(fd, ...)
Ignore the write-combining flag when mapping I/O port space.
This patch should have no functional effect, based on this analysis of all
implementations of pci_mmap_page_range():
- ia64 mips parisc sh unicore32 x86 do not support mapping of I/O port
space at all.
- arm cris microblaze mn10300 sparc xtensa support mapping of I/O port
space, but ignore the write_combine argument to pci_mmap_page_range().
- powerpc supports mapping of I/O port space and uses write_combine, and
it disables write combining for I/O port space in
__pci_mmap_set_pgprot().
This patch makes it possible to remove __pci_mmap_set_pgprot() from
powerpc, which simplifies that path.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/pci/proc.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/proc.c b/drivers/pci/proc.c
index 3f155e7..2408abe 100644
--- a/drivers/pci/proc.c
+++ b/drivers/pci/proc.c
@@ -231,7 +231,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;
- int i, ret;
+ int i, ret, write_combine;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
@@ -245,9 +245,12 @@ static int proc_bus_pci_mmap(struct file *file, struct vm_area_struct *vma)
if (i >= PCI_ROM_RESOURCE)
return -ENODEV;
+ if (fpriv->mmap_state == pci_mmap_mem)
+ write_combine = fpriv->write_combine;
+ else
+ write_combine = 0;
ret = pci_mmap_page_range(dev, vma,
- fpriv->mmap_state,
- fpriv->write_combine);
+ fpriv->mmap_state, write_combine);
if (ret < 0)
return ret;
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v1 2/3] powerpc/pci: Remove __pci_mmap_set_pgprot()
2016-06-09 18:20 [PATCH v1 0/3] PCI: Clean up write combining usage for user mmap Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 1/3] PCI: Ignore write combining when mapping I/O port space Bjorn Helgaas
@ 2016-06-09 18:20 ` Bjorn Helgaas
2016-06-17 19:54 ` Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 3/3] microblaze/PCI: Remove useless __pci_mmap_set_pgprot() Bjorn Helgaas
2 siblings, 1 reply; 5+ messages in thread
From: Bjorn Helgaas @ 2016-06-09 18:20 UTC (permalink / raw)
To: Yinghai Lu, Benjamin Herrenschmidt, Michal Simek, Paul Mackerras,
Michael Ellerman
Cc: linux-pci, linuxppc-dev, linux-kernel
From: Yinghai Lu <yinghai@kernel.org>
The powerpc-specific __pci_mmap_set_pgprot() does two things:
1) Disables write combining for I/O port space mappings
This only affects procfs mappings. The pci_mmap_resource() sysfs path
only requests write combining for resources with IORESOURCE_PREFETCH
set, which doesn't include I/O resources.
The only way to request write combining for I/O port space mappings
was via the PCIIOC_WRITE_COMBINE ioctl and the proc_bus_pci_mmap()
path, and we recently changed that path to ignore write combining for
I/O, so this code in powerpc is no longer needed.
2) Automatically enables write combining for mappings of prefetchable
resources, even if not requested by the user
Both procfs (via PCIIOC_MMAP_IS_MEM and PCIIOC_WRITE_COMBINE ioctls)
and sysfs (via "resourceN_wc" files, which are created for resources
with IORESOURCE_PREFETCH) provide ways for the user to map PCI memory
space with write combining.
Users that desire write combining should use one of those ways instead
of relying on powerpc-specific behavior.
Remove the powerpc-specific __pci_mmap_set_pgprot().
The user-visible effect of this change is that users mapping prefetchable
PCI memory space via procfs without PCIIOC_WRITE_COMBINE or via sysfs
"resourceN" (not "resourceN_wc") will get regular uncacheable mappings
instead of the write combining mappings they used to get.
The new behavior matches the behavior on all other arches that support
write combining mapping.
[bhelgaas: changelog]
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
arch/powerpc/kernel/pci-common.c | 37 ++++---------------------------------
1 file changed, 4 insertions(+), 33 deletions(-)
diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index 0f7a60f..8c6beb0 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -356,36 +356,6 @@ static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
}
/*
- * Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
- * device mapping.
- */
-static pgprot_t __pci_mmap_set_pgprot(struct pci_dev *dev, struct resource *rp,
- pgprot_t protection,
- enum pci_mmap_state mmap_state,
- int write_combine)
-{
-
- /* Write combine is always 0 on non-memory space mappings. On
- * memory space, if the user didn't pass 1, we check for a
- * "prefetchable" resource. This is a bit hackish, but we use
- * this to workaround the inability of /sysfs to provide a write
- * combine bit
- */
- if (mmap_state != pci_mmap_mem)
- write_combine = 0;
- else if (write_combine == 0) {
- if (rp->flags & IORESOURCE_PREFETCH)
- write_combine = 1;
- }
-
- /* XXX would be nice to have a way to ask for write-through */
- if (write_combine)
- return pgprot_noncached_wc(protection);
- else
- return pgprot_noncached(protection);
-}
-
-/*
* This one is used by /dev/mem and fbdev who have no clue about the
* PCI device, it tries to find the PCI device first and calls the
* above routine
@@ -458,9 +428,10 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
return -EINVAL;
vma->vm_pgoff = offset >> PAGE_SHIFT;
- vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
- vma->vm_page_prot,
- mmap_state, write_combine);
+ if (write_combine)
+ vma->vm_page_prot = pgprot_noncached_wc(vma->vm_page_prot);
+ else
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
vma->vm_end - vma->vm_start, vma->vm_page_prot);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1 2/3] powerpc/pci: Remove __pci_mmap_set_pgprot()
2016-06-09 18:20 ` [PATCH v1 2/3] powerpc/pci: Remove __pci_mmap_set_pgprot() Bjorn Helgaas
@ 2016-06-17 19:54 ` Bjorn Helgaas
0 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2016-06-17 19:54 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Yinghai Lu, Benjamin Herrenschmidt, Michal Simek, Paul Mackerras,
Michael Ellerman, linux-pci, linuxppc-dev, linux-kernel
On Thu, Jun 09, 2016 at 01:20:23PM -0500, Bjorn Helgaas wrote:
> From: Yinghai Lu <yinghai@kernel.org>
>
> The powerpc-specific __pci_mmap_set_pgprot() does two things:
>
> 1) Disables write combining for I/O port space mappings
>
> This only affects procfs mappings. The pci_mmap_resource() sysfs path
> only requests write combining for resources with IORESOURCE_PREFETCH
> set, which doesn't include I/O resources.
>
> The only way to request write combining for I/O port space mappings
> was via the PCIIOC_WRITE_COMBINE ioctl and the proc_bus_pci_mmap()
> path, and we recently changed that path to ignore write combining for
> I/O, so this code in powerpc is no longer needed.
>
> 2) Automatically enables write combining for mappings of prefetchable
> resources, even if not requested by the user
>
> Both procfs (via PCIIOC_MMAP_IS_MEM and PCIIOC_WRITE_COMBINE ioctls)
> and sysfs (via "resourceN_wc" files, which are created for resources
> with IORESOURCE_PREFETCH) provide ways for the user to map PCI memory
> space with write combining.
>
> Users that desire write combining should use one of those ways instead
> of relying on powerpc-specific behavior.
>
> Remove the powerpc-specific __pci_mmap_set_pgprot().
>
> The user-visible effect of this change is that users mapping prefetchable
> PCI memory space via procfs without PCIIOC_WRITE_COMBINE or via sysfs
> "resourceN" (not "resourceN_wc") will get regular uncacheable mappings
> instead of the write combining mappings they used to get.
>
> The new behavior matches the behavior on all other arches that support
> write combining mapping.
Powerpc folks, any thoughts on this?
It's currently on my pci/resource branch, and I plan to merge it for
v4.8 if there are no objections.
> [bhelgaas: changelog]
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> arch/powerpc/kernel/pci-common.c | 37 ++++---------------------------------
> 1 file changed, 4 insertions(+), 33 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index 0f7a60f..8c6beb0 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -356,36 +356,6 @@ static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
> }
>
> /*
> - * Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
> - * device mapping.
> - */
> -static pgprot_t __pci_mmap_set_pgprot(struct pci_dev *dev, struct resource *rp,
> - pgprot_t protection,
> - enum pci_mmap_state mmap_state,
> - int write_combine)
> -{
> -
> - /* Write combine is always 0 on non-memory space mappings. On
> - * memory space, if the user didn't pass 1, we check for a
> - * "prefetchable" resource. This is a bit hackish, but we use
> - * this to workaround the inability of /sysfs to provide a write
> - * combine bit
> - */
> - if (mmap_state != pci_mmap_mem)
> - write_combine = 0;
> - else if (write_combine == 0) {
> - if (rp->flags & IORESOURCE_PREFETCH)
> - write_combine = 1;
> - }
> -
> - /* XXX would be nice to have a way to ask for write-through */
> - if (write_combine)
> - return pgprot_noncached_wc(protection);
> - else
> - return pgprot_noncached(protection);
> -}
> -
> -/*
> * This one is used by /dev/mem and fbdev who have no clue about the
> * PCI device, it tries to find the PCI device first and calls the
> * above routine
> @@ -458,9 +428,10 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> return -EINVAL;
>
> vma->vm_pgoff = offset >> PAGE_SHIFT;
> - vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
> - vma->vm_page_prot,
> - mmap_state, write_combine);
> + if (write_combine)
> + vma->vm_page_prot = pgprot_noncached_wc(vma->vm_page_prot);
> + else
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
>
> ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> vma->vm_end - vma->vm_start, vma->vm_page_prot);
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1 3/3] microblaze/PCI: Remove useless __pci_mmap_set_pgprot()
2016-06-09 18:20 [PATCH v1 0/3] PCI: Clean up write combining usage for user mmap Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 1/3] PCI: Ignore write combining when mapping I/O port space Bjorn Helgaas
2016-06-09 18:20 ` [PATCH v1 2/3] powerpc/pci: Remove __pci_mmap_set_pgprot() Bjorn Helgaas
@ 2016-06-09 18:20 ` Bjorn Helgaas
2 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2016-06-09 18:20 UTC (permalink / raw)
To: Yinghai Lu, Benjamin Herrenschmidt, Michal Simek, Paul Mackerras,
Michael Ellerman
Cc: linux-pci, linuxppc-dev, linux-kernel
The microblaze __pci_mmap_set_pgprot() was apparently copied from powerpc,
where it computes either an uncacheable pgprot_t or a write-combining one.
But on microblaze, we always use the regular uncacheable pgprot_t.
Remove the useless code in __pci_mmap_set_pgprot() and inline the
pgprot_noncached() at the only caller.
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Acked-by: Yinghai Lu <yinghai@kernel.org>
---
arch/microblaze/pci/pci-common.c | 31 +------------------------------
1 file changed, 1 insertion(+), 30 deletions(-)
diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 14cba60..1974567 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -219,33 +219,6 @@ static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
}
/*
- * Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
- * device mapping.
- */
-static pgprot_t __pci_mmap_set_pgprot(struct pci_dev *dev, struct resource *rp,
- pgprot_t protection,
- enum pci_mmap_state mmap_state,
- int write_combine)
-{
- pgprot_t prot = protection;
-
- /* Write combine is always 0 on non-memory space mappings. On
- * memory space, if the user didn't pass 1, we check for a
- * "prefetchable" resource. This is a bit hackish, but we use
- * this to workaround the inability of /sysfs to provide a write
- * combine bit
- */
- if (mmap_state != pci_mmap_mem)
- write_combine = 0;
- else if (write_combine == 0) {
- if (rp->flags & IORESOURCE_PREFETCH)
- write_combine = 1;
- }
-
- return pgprot_noncached(prot);
-}
-
-/*
* This one is used by /dev/mem and fbdev who have no clue about the
* PCI device, it tries to find the PCI device first and calls the
* above routine
@@ -317,9 +290,7 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
return -EINVAL;
vma->vm_pgoff = offset >> PAGE_SHIFT;
- vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
- vma->vm_page_prot,
- mmap_state, write_combine);
+ vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
ret = remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
vma->vm_end - vma->vm_start, vma->vm_page_prot);
^ permalink raw reply related [flat|nested] 5+ messages in thread