Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1390232 > unrolled thread
| Started by | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| First post | 2016-04-28 16:00 +0200 |
| Last post | 2016-05-04 06:20 +0200 |
| Articles | 17 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Bjorn Helgaas <helgaas@kernel.org> - 2016-04-28 16:00 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-04-29 09:20 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-04 01:00 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-04 02:40 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Bjorn Helgaas <helgaas@kernel.org> - 2016-05-04 03:30 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-04 07:10 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-04 08:00 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Bjorn Helgaas <helgaas@kernel.org> - 2016-05-04 17:20 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-04 20:50 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-05 02:30 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-05 18:00 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2016-05-06 00:10 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-06 03:00 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-06 06:20 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Bjorn Helgaas <helgaas@kernel.org> - 2016-05-06 20:30 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource Yinghai Lu <yinghai@kernel.org> - 2016-05-10 08:20 +0200
Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource David Miller <davem@davemloft.net> - 2016-05-04 06:20 +0200
| From | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| Date | 2016-04-28 16:00 +0200 |
| Subject | Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource |
| Message-ID | <rsUE4-41Z-21@gated-at.bofh.it> |
On Wed, Apr 27, 2016 at 09:55:45PM -0700, Yinghai Lu wrote:
> On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > [+cc Ben, Michael]
> > I'm kind of confused here. There are two ways to mmap PCI BARs:
> >
> > /proc/bus/pci/00/02.0 (proc_bus_pci_mmap()):
> > all BARs in one file; MEM/IO determined by ioctl()
> > mmap offset is a CPU physical address in the PCI resource
> >
> > /sys/devices/pci0000:00/0000:00:02.0/resource0 (pci_mmap_resource()):
> > one file per BAR; MEM/IO determined by BAR type
> > mmap offset is between 0 and BAR size
> >
> > Both proc_bus_pci_mmap() and pci_mmap_resource() validate the
> > requested area with pci_mmap_fits() before calling pci_mmap_page_range().
> >
> > In the proc_bus_pci_mmap() path, the offset in vma->vm_pgoff must be
> > within the pdev->resource[], so the user must be supplying a CPU
> > physical address (not an address obtained from pci_resource_to_user()).
> > That vma->vm_pgoff is passed unchanged to pci_mmap_page_range().
> >
> > In the pci_mmap_resource() path, vma->vm_pgoff must be between 0 and
> > the BAR size. Then we add in the pci_resource_to_user() information
> > before passing it to pci_mmap_page_range(). The comment in
> > pci_mmap_resource() says pci_mmap_page_range() expects a "user
> > visible" address, but I don't really believe that based on how
> > proc_bus_pci_mmap() works.
> >
> > Do both proc_bus_pci_mmap() and pci_mmap_resource() work on sparc?
> > It looks like they call pci_mmap_page_range() with different
> > assumptions, so I don't see how they can both work.
>
> for sysfs path: in pci_mmap_resource
> pci_resource_to_user(pdev, i, res, &start, &end);
> vma->vm_pgoff += start >> PAGE_SHIFT;
> then call pci_mmap_page_range()
>
> the fit checking in pci_mmap_fits(),
> pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
> pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
> if (start >= pci_start && start < pci_start + size &&
> start + nr <= pci_start + size)
>
> so proc fs assume resource_start for vm_pgoff ?
>
> but current pci_mmap_page_range want to use bus address
> start aka BAR value.
>
> and we have
>
> /* 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.
> */
>
> so we need to fix pci_mmap_fits(), please check if it is ok, will
> submit it as separated one.
1) The sysfs path uses offsets between 0 and BAR size. This path
should work identically on all arches. "User" addresses are not
involved, so it doesn't make sense that this path calls
pci_resource_to_user() from pci_mmap_resource().
2) The procfs path uses offsets of resource values (CPU physical
addresses) on most architectures. If some arches use something else,
e.g., "user" addresses, the grunge of dealing with them should be in
this path, i.e., in proc_bus_pci_mmap(). This implies that
pci_mmap_page_range() should deal with CPU physical addresses, not bus
addresses, and proc_bus_pci_mmap() should perform any necessary
translation.
3) If my theory that proc_bus_pci_mmap() and pci_mmap_resource() are
calling pci_mmap_page_range() with different assumptions is correct,
you should be able to write a test program that fails for one method,
and your patch should fix that failure.
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index d319a9c..3768c6a 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -969,15 +969,20 @@ void pci_remove_legacy_files(struct pci_bus *b)
> int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma,
> enum pci_mmap_api mmap_api)
> {
> - unsigned long nr, start, size, pci_start;
> + unsigned long nr, start, size;
> + resource_size_t pci_start = 0, pci_end;
>
> if (pci_resource_len(pdev, resno) == 0)
> return 0;
> nr = vma_pages(vma);
> start = vma->vm_pgoff;
> size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
> - pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
> - pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
> + if (mmap_api == PCI_MMAP_PROCFS) {
> + struct resource *res = &pdev->resource[resno];
> +
> + pci_resource_to_user(pdev, resno, res, &pci_start, &pci_end);
> + pci_start >>= PAGE_SHIFT;
> + }
This is the wrong place to deal with this. IMO, any pci_resource_to_user()
fiddling should be done directly in proc_bus_pci_mmap(), and
pci_mmap_fits() should deal only with resources (CPU physical
addresses). Then it wouldn't care where it was called from, so we
could get rid of the pci_mmap_api thing completely.
> if (start >= pci_start && start < pci_start + size &&
> start + nr <= pci_start + size)
> return 1;
> --
> 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
[toc] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-04-29 09:20 +0200 |
| Message-ID | <rtaSu-1U4-21@gated-at.bofh.it> |
| In reply to | #1390232 |
On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: > On Wed, Apr 27, 2016 at 09:55:45PM -0700, Yinghai Lu wrote: >> On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas <helgaas@kernel.org> wrote: >> > [+cc Ben, Michael] >> > I'm kind of confused here. There are two ways to mmap PCI BARs: >> > >> > /proc/bus/pci/00/02.0 (proc_bus_pci_mmap()): >> > all BARs in one file; MEM/IO determined by ioctl() >> > mmap offset is a CPU physical address in the PCI resource >> > >> > /sys/devices/pci0000:00/0000:00:02.0/resource0 (pci_mmap_resource()): >> > one file per BAR; MEM/IO determined by BAR type >> > mmap offset is between 0 and BAR size >> > >> > Both proc_bus_pci_mmap() and pci_mmap_resource() validate the >> > requested area with pci_mmap_fits() before calling pci_mmap_page_range(). >> > >> > In the proc_bus_pci_mmap() path, the offset in vma->vm_pgoff must be >> > within the pdev->resource[], so the user must be supplying a CPU >> > physical address (not an address obtained from pci_resource_to_user()). >> > That vma->vm_pgoff is passed unchanged to pci_mmap_page_range(). >> > >> > In the pci_mmap_resource() path, vma->vm_pgoff must be between 0 and >> > the BAR size. Then we add in the pci_resource_to_user() information >> > before passing it to pci_mmap_page_range(). The comment in >> > pci_mmap_resource() says pci_mmap_page_range() expects a "user >> > visible" address, but I don't really believe that based on how >> > proc_bus_pci_mmap() works. >> > >> > Do both proc_bus_pci_mmap() and pci_mmap_resource() work on sparc? >> > It looks like they call pci_mmap_page_range() with different >> > assumptions, so I don't see how they can both work. >> >> for sysfs path: in pci_mmap_resource >> pci_resource_to_user(pdev, i, res, &start, &end); >> vma->vm_pgoff += start >> PAGE_SHIFT; >> then call pci_mmap_page_range() >> >> the fit checking in pci_mmap_fits(), >> pci_start = (mmap_api == PCI_MMAP_PROCFS) ? >> pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0; >> if (start >= pci_start && start < pci_start + size && >> start + nr <= pci_start + size) >> >> so proc fs assume resource_start for vm_pgoff ? >> >> but current pci_mmap_page_range want to use bus address >> start aka BAR value. >> >> and we have >> >> /* 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. >> */ >> >> so we need to fix pci_mmap_fits(), please check if it is ok, will >> submit it as separated one. > > 1) The sysfs path uses offsets between 0 and BAR size. This path > should work identically on all arches. "User" addresses are not > involved, so it doesn't make sense that this path calls > pci_resource_to_user() from pci_mmap_resource(). > > 2) The procfs path uses offsets of resource values (CPU physical > addresses) on most architectures. If some arches use something else, > e.g., "user" addresses, the grunge of dealing with them should be in > this path, i.e., in proc_bus_pci_mmap(). This implies that > pci_mmap_page_range() should deal with CPU physical addresses, not bus > addresses, and proc_bus_pci_mmap() should perform any necessary > translation. > > 3) If my theory that proc_bus_pci_mmap() and pci_mmap_resource() are > calling pci_mmap_page_range() with different assumptions is correct, > you should be able to write a test program that fails for one method, > and your patch should fix that failure. > ...> > This is the wrong place to deal with this. IMO, any pci_resource_to_user() > fiddling should be done directly in proc_bus_pci_mmap(), and > pci_mmap_fits() should deal only with resources (CPU physical > addresses). Then it wouldn't care where it was called from, so we > could get rid of the pci_mmap_api thing completely. ok, I got it. We should offset vma->vm_pgoff back into [0, BAR) will look at it Monday. Thanks Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-04 01:00 +0200 |
| Message-ID | <ruRsm-dq-9@gated-at.bofh.it> |
| In reply to | #1390820 |
On Fri, Apr 29, 2016 at 12:19 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>>
>> 1) The sysfs path uses offsets between 0 and BAR size. This path
>> should work identically on all arches. "User" addresses are not
>> involved, so it doesn't make sense that this path calls
>> pci_resource_to_user() from pci_mmap_resource().
>>
>> 2) The procfs path uses offsets of resource values (CPU physical
>> addresses) on most architectures. If some arches use something else,
>> e.g., "user" addresses, the grunge of dealing with them should be in
>> this path, i.e., in proc_bus_pci_mmap(). This implies that
>> pci_mmap_page_range() should deal with CPU physical addresses, not bus
>> addresses, and proc_bus_pci_mmap() should perform any necessary
>> translation.
Please check if following is what you want.
BenH and DavidM,
Are you ok to let /proc/bus/pci/devices to expose resource value instead of
BAR value?
powerpc already expose MMIO as resource value, but still keep IO as BAR value?
Or can we just dump /proc/bus/pci support from now?
Thanks
Yinghai
Subject: [RFC PATCH] PCI: Expose resource value in /proc/bus/pci/devices
Some arch where cpu address (resource value) is not same as pci bus address
(BAR value in pci BAR registers), include sparc, powerpc, microblaze.
Orignally in /proc/bus/pci/devices is exposing device BAR value aka is
user value. later in 396a1a5832 ("[POWERPC] Fix mmap of PCI resource with
hack for X"), in will change to use resource start instead.
Also in 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
to check exposed value with resource start/end in proc mmap path.
| start = vma->vm_pgoff;
| size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
| pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
| pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
| if (start >= pci_start && start < pci_start + size &&
| start + nr <= pci_start + size)
That would break sparc that exposed value is still BAR value.
According to Bjorn, we could just pass resource addr instead of BAR.
In the patch:
1. remove those non uniformed pci_resource_to_user.
2. in proc path: proc_bus_pci_mmap==>pci_mmap_fits/pci_mmap_page_range
all use resource start instead.
3. in sysfs path: pci_mmap_resource will just offset with resource start.
4. all pci_mmap_page_range will all have vma->vm_pgoff with in resource range
instead of BAR value.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/microblaze/include/asm/pci.h | 5 ---
arch/microblaze/pci/pci-common.c | 53 ++------------------------------------
arch/mips/include/asm/pci.h | 12 --------
arch/powerpc/include/asm/pci.h | 5 ---
arch/powerpc/kernel/pci-common.c | 53 ++------------------------------------
arch/sparc/include/asm/pci_64.h | 5 ---
arch/sparc/kernel/pci.c | 43 ++++++------------------------
arch/xtensa/kernel/pci.c | 11 ++-----
drivers/pci/pci-sysfs.c | 13 ++-------
drivers/pci/proc.c | 16 ++++-------
include/linux/pci.h | 15 ----------
kernel/trace/trace_mmiotrace.c | 21 +++++----------
12 files changed, 37 insertions(+), 215 deletions(-)
Index: linux-2.6/arch/microblaze/include/asm/pci.h
===================================================================
--- linux-2.6.orig/arch/microblaze/include/asm/pci.h
+++ linux-2.6/arch/microblaze/include/asm/pci.h
@@ -81,11 +81,6 @@ extern pgprot_t pci_phys_mem_access_prot
unsigned long size,
pgprot_t prot);
-#define HAVE_ARCH_PCI_RESOURCE_TO_USER
-extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc,
- resource_size_t *start, resource_size_t *end);
-
extern void pcibios_setup_bus_devices(struct pci_bus *bus);
extern void pcibios_setup_bus_self(struct pci_bus *bus);
Index: linux-2.6/arch/microblaze/pci/pci-common.c
===================================================================
--- linux-2.6.orig/arch/microblaze/pci/pci-common.c
+++ linux-2.6/arch/microblaze/pci/pci-common.c
@@ -169,23 +169,16 @@ static struct resource *__pci_mmap_make_
enum pci_mmap_state mmap_state)
{
struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
int i, res_bit;
if (!hose)
return NULL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -209,7 +202,8 @@ static struct resource *__pci_mmap_make_
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
return rp;
}
@@ -467,45 +461,6 @@ int pci_mmap_legacy_page_range(struct pc
vma->vm_page_prot);
}
-void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc,
- resource_size_t *start, resource_size_t *end)
-{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- resource_size_t offset = 0;
-
- if (hose == NULL)
- return;
-
- if (rsrc->flags & IORESOURCE_IO)
- offset = (unsigned long)hose->io_base_virt - _IO_BASE;
-
- /* We pass a fully fixed up address to userland for MMIO instead of
- * a BAR value because X is lame and expects to be able to use that
- * to pass to /dev/mem !
- *
- * That means that we'll have potentially 64 bits values where some
- * userland apps only expect 32 (like X itself since it thinks only
- * Sparc has 64 bits MMIO) but if we don't do that, we break it on
- * 32 bits CHRPs :-(
- *
- * Hopefully, the sysfs insterface is immune to that gunk. Once X
- * has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
- *
- * BenH.
- */
-#if 0
- else if (rsrc->flags & IORESOURCE_MEM)
- offset = hose->pci_mem_offset;
-#endif
-
- *start = rsrc->start - offset;
- *end = rsrc->end - offset;
-}
-
/**
* pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
* @hose: newly allocated pci_controller to be setup
Index: linux-2.6/arch/mips/include/asm/pci.h
===================================================================
--- linux-2.6.orig/arch/mips/include/asm/pci.h
+++ linux-2.6/arch/mips/include/asm/pci.h
@@ -80,18 +80,6 @@ extern void pcibios_set_master(struct pc
extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
enum pci_mmap_state mmap_state, int write_combine);
-#define HAVE_ARCH_PCI_RESOURCE_TO_USER
-
-static inline void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc, resource_size_t *start,
- resource_size_t *end)
-{
- phys_addr_t size = resource_size(rsrc);
-
- *start = fixup_bigphys_addr(rsrc->start, size);
- *end = rsrc->start + size;
-}
-
/*
* Dynamic DMA mapping stuff.
* MIPS has everything mapped statically.
Index: linux-2.6/arch/powerpc/include/asm/pci.h
===================================================================
--- linux-2.6.orig/arch/powerpc/include/asm/pci.h
+++ linux-2.6/arch/powerpc/include/asm/pci.h
@@ -135,11 +135,6 @@ extern pgprot_t pci_phys_mem_access_prot
unsigned long size,
pgprot_t prot);
-#define HAVE_ARCH_PCI_RESOURCE_TO_USER
-extern void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc,
- resource_size_t *start, resource_size_t *end);
-
extern resource_size_t pcibios_io_space_offset(struct pci_controller *hose);
extern void pcibios_setup_bus_devices(struct pci_bus *bus);
extern void pcibios_setup_bus_self(struct pci_bus *bus);
Index: linux-2.6/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
+++ linux-2.6/arch/powerpc/kernel/pci-common.c
@@ -308,23 +308,16 @@ static struct resource *__pci_mmap_make_
enum pci_mmap_state mmap_state)
{
struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
int i, res_bit;
if (hose == NULL)
return NULL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -348,7 +341,8 @@ static struct resource *__pci_mmap_make_
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
return rp;
}
@@ -606,45 +600,6 @@ int pci_mmap_legacy_page_range(struct pc
vma->vm_page_prot);
}
-void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc,
- resource_size_t *start, resource_size_t *end)
-{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- resource_size_t offset = 0;
-
- if (hose == NULL)
- return;
-
- if (rsrc->flags & IORESOURCE_IO)
- offset = (unsigned long)hose->io_base_virt - _IO_BASE;
-
- /* We pass a fully fixed up address to userland for MMIO instead of
- * a BAR value because X is lame and expects to be able to use that
- * to pass to /dev/mem !
- *
- * That means that we'll have potentially 64 bits values where some
- * userland apps only expect 32 (like X itself since it thinks only
- * Sparc has 64 bits MMIO) but if we don't do that, we break it on
- * 32 bits CHRPs :-(
- *
- * Hopefully, the sysfs insterface is immune to that gunk. Once X
- * has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
- *
- * BenH.
- */
-#if 0
- else if (rsrc->flags & IORESOURCE_MEM)
- offset = hose->pci_mem_offset;
-#endif
-
- *start = rsrc->start - offset;
- *end = rsrc->end - offset;
-}
-
/**
* pci_process_bridge_OF_ranges - Parse PCI bridge resources from device tree
* @hose: newly allocated pci_controller to be setup
Index: linux-2.6/arch/sparc/include/asm/pci_64.h
===================================================================
--- linux-2.6.orig/arch/sparc/include/asm/pci_64.h
+++ linux-2.6/arch/sparc/include/asm/pci_64.h
@@ -53,11 +53,6 @@ static inline int pci_get_legacy_ide_irq
{
return PCI_IRQ_NONE;
}
-
-#define HAVE_ARCH_PCI_RESOURCE_TO_USER
-void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc,
- resource_size_t *start, resource_size_t *end);
#endif /* __KERNEL__ */
#endif /* __SPARC64_PCI_H */
Index: linux-2.6/arch/sparc/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/pci.c
+++ linux-2.6/arch/sparc/kernel/pci.c
@@ -743,30 +743,21 @@ static int __pci_mmap_make_offset_bus(st
enum pci_mmap_state mmap_state)
{
struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
- unsigned long space_size, user_offset, user_size;
+ unsigned long start, end;
+ struct resource *res;
- if (mmap_state == pci_mmap_io) {
- space_size = resource_size(&pbm->io_space);
- } else {
- space_size = resource_size(&pbm->mem_space);
- }
+ if (mmap_state == pci_mmap_io)
+ res = &pbm->io_space;
+ else
+ res = &pbm->mem_space;
/* Make sure the request is in range. */
- user_offset = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
+ start = vma->vm_pgoff << PAGE_SHIFT;
+ end = vma->vm_end - vma->vm_start + start - 1;
- if (user_offset >= space_size ||
- (user_offset + user_size) > space_size)
+ if (!((res->start <= start) && (res->end >= end)))
return -EINVAL;
- if (mmap_state == pci_mmap_io) {
- vma->vm_pgoff = (pbm->io_space.start +
- user_offset) >> PAGE_SHIFT;
- } else {
- vma->vm_pgoff = (pbm->mem_space.start +
- user_offset) >> PAGE_SHIFT;
- }
-
return 0;
}
@@ -982,22 +973,6 @@ int pci64_dma_supported(struct pci_dev *
return (device_mask & dma_addr_mask) == dma_addr_mask;
}
-void pci_resource_to_user(const struct pci_dev *pdev, int bar,
- const struct resource *rp, resource_size_t *start,
- resource_size_t *end)
-{
- struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
- unsigned long offset;
-
- if (rp->flags & IORESOURCE_IO)
- offset = pbm->io_space.start;
- else
- offset = pbm->mem_space.start;
-
- *start = rp->start - offset;
- *end = rp->end - offset;
-}
-
void pcibios_set_master(struct pci_dev *dev)
{
/* No special bus mastering setup handling */
Index: linux-2.6/arch/xtensa/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/xtensa/kernel/pci.c
+++ linux-2.6/arch/xtensa/kernel/pci.c
@@ -288,20 +288,16 @@ __pci_mmap_make_offset(struct pci_dev *d
{
struct pci_controller *pci_ctrl = (struct pci_controller*) dev->sysdata;
unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
- unsigned long io_offset = 0;
int i, res_bit;
if (pci_ctrl == 0)
return -EINVAL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)pci_ctrl->io_space.base;
- offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -325,7 +321,8 @@ __pci_mmap_make_offset(struct pci_dev *d
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- offset += pci_ctrl->io_space.start - io_offset;
+ offset += pci_ctrl->io_space.start -
+ pci_ctrl->io_space.base;
vma->vm_pgoff = offset >> PAGE_SHIFT;
return 0;
}
Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -143,10 +143,9 @@ static ssize_t resource_show(struct devi
for (i = 0; i < max; i++) {
struct resource *res = &pci_dev->resource[i];
- pci_resource_to_user(pci_dev, i, res, &start, &end);
str += sprintf(str, "0x%016llx 0x%016llx 0x%016llx\n",
- (unsigned long long)start,
- (unsigned long long)end,
+ (unsigned long long)res->start,
+ (unsigned long long)res->end,
(unsigned long long)res->flags);
}
return (str - buf);
@@ -999,7 +998,6 @@ static int pci_mmap_resource(struct kobj
struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
struct resource *res = attr->private;
enum pci_mmap_state mmap_type;
- resource_size_t start, end;
int i;
for (i = 0; i < PCI_ROM_RESOURCE; i++)
@@ -1020,12 +1018,7 @@ static int pci_mmap_resource(struct kobj
return -EINVAL;
}
- /* 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, i, res, &start, &end);
- vma->vm_pgoff += start >> PAGE_SHIFT;
+ vma->vm_pgoff += res->start >> PAGE_SHIFT;
mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
return pci_mmap_page_range(pdev, vma, mmap_type, write_combine);
}
Index: linux-2.6/drivers/pci/proc.c
===================================================================
--- linux-2.6.orig/drivers/pci/proc.c
+++ linux-2.6/drivers/pci/proc.c
@@ -343,20 +343,16 @@ static int show_device(struct seq_file *
dev->irq);
/* only print standard and ROM resources to preserve compatibility */
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- resource_size_t start, end;
- pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+ for (i = 0; i <= PCI_ROM_RESOURCE; i++)
seq_printf(m, "\t%16llx",
- (unsigned long long)(start |
+ (unsigned long long)(dev->resource[i].start |
(dev->resource[i].flags & PCI_REGION_FLAG_MASK)));
- }
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- resource_size_t start, end;
- pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+
+ for (i = 0; i <= PCI_ROM_RESOURCE; i++)
seq_printf(m, "\t%16llx",
dev->resource[i].start < dev->resource[i].end ?
- (unsigned long long)(end - start) + 1 : 0);
- }
+ (unsigned long long)resource_size(&dev->resource[i]) : 0);
+
seq_putc(m, '\t');
if (drv)
seq_printf(m, "%s", drv->name);
Index: linux-2.6/include/linux/pci.h
===================================================================
--- linux-2.6.orig/include/linux/pci.h
+++ linux-2.6/include/linux/pci.h
@@ -1545,21 +1545,6 @@ static inline const char *pci_name(const
return dev_name(&pdev->dev);
}
-
-/* Some archs don't want to expose struct resource to userland as-is
- * in sysfs and /proc
- */
-#ifndef HAVE_ARCH_PCI_RESOURCE_TO_USER
-static inline void pci_resource_to_user(const struct pci_dev *dev, int bar,
- const struct resource *rsrc, resource_size_t *start,
- resource_size_t *end)
-{
- *start = rsrc->start;
- *end = rsrc->end;
-}
-#endif /* HAVE_ARCH_PCI_RESOURCE_TO_USER */
-
-
/*
* The world is not perfect and supplies us with broken PCI devices.
* For at least a part of these bugs we need a work-around, so both
Index: linux-2.6/kernel/trace/trace_mmiotrace.c
===================================================================
--- linux-2.6.orig/kernel/trace/trace_mmiotrace.c
+++ linux-2.6/kernel/trace/trace_mmiotrace.c
@@ -62,29 +62,22 @@ static void mmio_trace_start(struct trac
static void mmio_print_pcidev(struct trace_seq *s, const struct pci_dev *dev)
{
int i;
- resource_size_t start, end;
const struct pci_driver *drv = pci_dev_driver(dev);
trace_seq_printf(s, "PCIDEV %02x%02x %04x%04x %x",
dev->bus->number, dev->devfn,
dev->vendor, dev->device, dev->irq);
- /*
- * XXX: is pci_resource_to_user() appropriate, since we are
- * supposed to interpret the __ioremap() phys_addr argument based on
- * these printed values?
- */
- for (i = 0; i < 7; i++) {
- pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+
+ for (i = 0; i < 7; i++)
trace_seq_printf(s, " %llx",
- (unsigned long long)(start |
+ (unsigned long long)(dev->resource[i].start |
(dev->resource[i].flags & PCI_REGION_FLAG_MASK)));
- }
- for (i = 0; i < 7; i++) {
- pci_resource_to_user(dev, i, &dev->resource[i], &start, &end);
+
+ for (i = 0; i < 7; i++)
trace_seq_printf(s, " %llx",
dev->resource[i].start < dev->resource[i].end ?
- (unsigned long long)(end - start) + 1 : 0);
- }
+ (unsigned long long)resource_size(&dev->resource[i]) : 0);
+
if (drv)
trace_seq_printf(s, " %s\n", drv->name);
else
[toc] | [prev] | [next] | [standalone]
| From | Benjamin Herrenschmidt <benh@kernel.crashing.org> |
|---|---|
| Date | 2016-05-04 02:40 +0200 |
| Message-ID | <ruT17-1Cp-3@gated-at.bofh.it> |
| In reply to | #1393853 |
On Tue, 2016-05-03 at 15:52 -0700, Yinghai Lu wrote: > BenH and DavidM, > Are you ok to let /proc/bus/pci/devices to expose resource value > instead of > BAR value? > powerpc already expose MMIO as resource value, but still keep IO as > BAR value? > > Or can we just dump /proc/bus/pci support from now? The problem tends to be old Xserver expectations... That stuff has been a can of worms over the years and we did things in the kernel to work around X limitations. I'm not that keen on touching /proc at all in that regard. Leave it there do what it does today, it's a user visible ABI, don't change it. Cheers, Ben.
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| Date | 2016-05-04 03:30 +0200 |
| Message-ID | <ruTNv-2ke-1@gated-at.bofh.it> |
| In reply to | #1393874 |
On Wed, May 04, 2016 at 10:37:40AM +1000, Benjamin Herrenschmidt wrote:
> On Tue, 2016-05-03 at 15:52 -0700, Yinghai Lu wrote:
> > BenH and DavidM,
> > Are you ok to let /proc/bus/pci/devices to expose resource value
> > instead of
> > BAR value?
> > powerpc already expose MMIO as resource value, but still keep IO as
> > BAR value?
> >
> > Or can we just dump /proc/bus/pci support from now?
>
> The problem tends to be old Xserver expectations...
>
> That stuff has been a can of worms over the years and we did things in
> the kernel to work around X limitations. I'm not that keen on touching
> /proc at all in that regard. Leave it there do what it does today, it's
> a user visible ABI, don't change it.
I did not propose changing any user-visible ABI. To recap what I did
propose:
- The sysfs path uses offsets between 0 and BAR size on all arches.
It uses pci_resource_to_user() today, but I think it should not.
- The procfs path uses offsets of resource values (CPU physical
addresses) on most architectures, but uses something else, e.g.,
BAR values, on others. pci_resource_to_user() does this
translation. The procfs path does not use pci_resource_to_user()
today, but I think it should.
- This implies that pci_mmap_page_range() should deal with resource
values (CPU physical addresses), and proc_bus_pci_mmap() should do
any necessary arch-specific translation from BAR values to
resource values.
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-04 07:10 +0200 |
| Message-ID | <ruXep-5ZP-1@gated-at.bofh.it> |
| In reply to | #1393885 |
On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> On Wed, May 04, 2016 at 10:37:40AM +1000, Benjamin Herrenschmidt wrote:
>>
>> The problem tends to be old Xserver expectations...
>>
>> That stuff has been a can of worms over the years and we did things in
>> the kernel to work around X limitations. I'm not that keen on touching
>> /proc at all in that regard. Leave it there do what it does today, it's
>> a user visible ABI, don't change it.
>
> I did not propose changing any user-visible ABI. To recap what I did
> propose:
I want to avoid introduce one strange pci_user_to_resource.
>
> - The sysfs path uses offsets between 0 and BAR size on all arches.
> It uses pci_resource_to_user() today, but I think it should not.
>
> - The procfs path uses offsets of resource values (CPU physical
> addresses) on most architectures, but uses something else, e.g.,
> BAR values, on others. pci_resource_to_user() does this
> translation. The procfs path does not use pci_resource_to_user()
> today, but I think it should.
current powerpc pci_resource_to_user is strange:
it will return resource start for io mem.
but will return BAR (?) start for io port.
sparc pci_resource_to_user does return BAR value for iomem.
>
> - This implies that pci_mmap_page_range() should deal with resource
> values (CPU physical addresses), and proc_bus_pci_mmap() should do
> any necessary arch-specific translation from BAR values to
> resource values.
so will need one different version pci_user_to_resource.
and can not use pcibios_bus_to_resource directly, and will be another mess.
Can you reconsider to keep the pci_mmap_page_range still use BAR value
and only touch pci_mmap_fits() ?
like i suggested before, and it does not conflict with the patchset.
Subject: [PATCH] PCI: Fix pci_mmap_fits() with proc interface
Passed value is user address, so need to compare it with
user addr that is converted from dev resource.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
drivers/pci/pci-sysfs.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -969,15 +969,19 @@ void pci_remove_legacy_files(struct pci_
int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma,
enum pci_mmap_api mmap_api)
{
- unsigned long nr, start, size, pci_start;
+ unsigned long nr, start, size;
+ resource_size_t pci_start = 0, pci_end;
if (pci_resource_len(pdev, resno) == 0)
return 0;
nr = vma_pages(vma);
start = vma->vm_pgoff;
size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
- pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
- pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
+ if (mmap_api == PCI_MMAP_PROCFS) {
+ pci_resource_to_user(pdev, resno, &pdev->resource[resno],
+ &pci_start, &pci_end);
+ pci_start >>= PAGE_SHIFT;
+ }
if (start >= pci_start && start < pci_start + size &&
start + nr <= pci_start + size)
return 1;
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-04 08:00 +0200 |
| Message-ID | <ruY0P-6oT-9@gated-at.bofh.it> |
| In reply to | #1393949 |
On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> I did not propose changing any user-visible ABI. To recap what I did
>> propose:
>
> I want to avoid introduce one strange pci_user_to_resource.
>
>>
>> - The sysfs path uses offsets between 0 and BAR size on all arches.
>> It uses pci_resource_to_user() today, but I think it should not.
>>
>> - The procfs path uses offsets of resource values (CPU physical
>> addresses) on most architectures, but uses something else, e.g.,
>> BAR values, on others. pci_resource_to_user() does this
>> translation. The procfs path does not use pci_resource_to_user()
>> today, but I think it should.
>
> current powerpc pci_resource_to_user is strange:
> it will return resource start for io mem.
> but will return BAR (?) start for io port.
>
> sparc pci_resource_to_user does return BAR value for iomem.
>
>>
>> - This implies that pci_mmap_page_range() should deal with resource
>> values (CPU physical addresses), and proc_bus_pci_mmap() should do
>> any necessary arch-specific translation from BAR values to
>> resource values.
>
> so will need one different version pci_user_to_resource.
> and can not use pcibios_bus_to_resource directly, and will be another mess.
looks like we can avoid that pci_user_to_resource() via trying out.
Please check it:
Subject: [RFC PATCH] PCI: Let pci_mmap_page_range() take resource addr
Some arch where cpu address (resource value) is not same as pci bus address
(BAR value in pci BAR registers), include sparc, powerpc, microblaze.
In 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
to check exposed value with resource start/end in proc mmap path.
| start = vma->vm_pgoff;
| size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
| pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
| pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
| if (start >= pci_start && start < pci_start + size &&
| start + nr <= pci_start + size)
That would break sparc that exposed value is still BAR value.
According to Bjorn, we could just pass resource addr instead of BAR.
In the patch:
1. in proc path: proc_bus_pci_mmap, try convert back to resource
before calling pci_mmap_page_range
2. in sysfs path: pci_mmap_resource will just offset with resource start.
3. all pci_mmap_page_range will all have vma->vm_pgoff with in resource range
instead of BAR value.
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/microblaze/pci/pci-common.c | 14 ++++----------
arch/powerpc/kernel/pci-common.c | 14 ++++----------
arch/sparc/kernel/pci.c | 27 +++++++++------------------
arch/xtensa/kernel/pci.c | 11 ++++-------
drivers/pci/pci-sysfs.c | 8 +-------
drivers/pci/proc.c | 13 +++++++++++++
6 files changed, 35 insertions(+), 52 deletions(-)
Index: linux-2.6/arch/microblaze/pci/pci-common.c
===================================================================
--- linux-2.6.orig/arch/microblaze/pci/pci-common.c
+++ linux-2.6/arch/microblaze/pci/pci-common.c
@@ -169,23 +169,16 @@ static struct resource *__pci_mmap_make_
enum pci_mmap_state mmap_state)
{
struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
int i, res_bit;
if (!hose)
return NULL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -209,7 +202,8 @@ static struct resource *__pci_mmap_make_
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
return rp;
}
Index: linux-2.6/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
+++ linux-2.6/arch/powerpc/kernel/pci-common.c
@@ -308,23 +308,16 @@ static struct resource *__pci_mmap_make_
enum pci_mmap_state mmap_state)
{
struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
int i, res_bit;
if (hose == NULL)
return NULL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -348,7 +341,8 @@ static struct resource *__pci_mmap_make_
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
return rp;
}
Index: linux-2.6/arch/sparc/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/pci.c
+++ linux-2.6/arch/sparc/kernel/pci.c
@@ -743,30 +743,21 @@ static int __pci_mmap_make_offset_bus(st
enum pci_mmap_state mmap_state)
{
struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
- unsigned long space_size, user_offset, user_size;
+ unsigned long start, end;
+ struct resource *res;
- if (mmap_state == pci_mmap_io) {
- space_size = resource_size(&pbm->io_space);
- } else {
- space_size = resource_size(&pbm->mem_space);
- }
+ if (mmap_state == pci_mmap_io)
+ res = &pbm->io_space;
+ else
+ res = &pbm->mem_space;
/* Make sure the request is in range. */
- user_offset = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
+ start = vma->vm_pgoff << PAGE_SHIFT;
+ end = vma->vm_end - vma->vm_start + start - 1;
- if (user_offset >= space_size ||
- (user_offset + user_size) > space_size)
+ if (!((res->start <= start) && (res->end >= end)))
return -EINVAL;
- if (mmap_state == pci_mmap_io) {
- vma->vm_pgoff = (pbm->io_space.start +
- user_offset) >> PAGE_SHIFT;
- } else {
- vma->vm_pgoff = (pbm->mem_space.start +
- user_offset) >> PAGE_SHIFT;
- }
-
return 0;
}
Index: linux-2.6/arch/xtensa/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/xtensa/kernel/pci.c
+++ linux-2.6/arch/xtensa/kernel/pci.c
@@ -288,20 +288,16 @@ __pci_mmap_make_offset(struct pci_dev *d
{
struct pci_controller *pci_ctrl = (struct pci_controller*) dev->sysdata;
unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
- unsigned long io_offset = 0;
int i, res_bit;
if (pci_ctrl == 0)
return -EINVAL; /* should never happen */
/* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
+ if (mmap_state == pci_mmap_mem)
res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)pci_ctrl->io_space.base;
- offset += io_offset;
+ else
res_bit = IORESOURCE_IO;
- }
/*
* Check that the offset requested corresponds to one of the
@@ -325,7 +321,8 @@ __pci_mmap_make_offset(struct pci_dev *d
/* found it! construct the final physical address */
if (mmap_state == pci_mmap_io)
- offset += pci_ctrl->io_space.start - io_offset;
+ offset += pci_ctrl->io_space.start -
+ pci_ctrl->io_space.base;
vma->vm_pgoff = offset >> PAGE_SHIFT;
return 0;
}
Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -999,7 +999,6 @@ static int pci_mmap_resource(struct kobj
struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
struct resource *res = attr->private;
enum pci_mmap_state mmap_type;
- resource_size_t start, end;
int i;
for (i = 0; i < PCI_ROM_RESOURCE; i++)
@@ -1020,12 +1019,7 @@ static int pci_mmap_resource(struct kobj
return -EINVAL;
}
- /* 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, i, res, &start, &end);
- vma->vm_pgoff += start >> PAGE_SHIFT;
+ vma->vm_pgoff += res->start >> PAGE_SHIFT;
mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
return pci_mmap_page_range(pdev, vma, mmap_type, write_combine);
}
Index: linux-2.6/drivers/pci/proc.c
===================================================================
--- linux-2.6.orig/drivers/pci/proc.c
+++ linux-2.6/drivers/pci/proc.c
@@ -231,13 +231,26 @@ static int proc_bus_pci_mmap(struct file
{
struct pci_dev *dev = PDE_DATA(file_inode(file));
struct pci_filp_private *fpriv = file->private_data;
+ resource_size_t start, end, offset;
+ struct resource *res;
int i, ret;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
+ offset = vma->vm_pgoff << PAGE_SHIFT;
+
/* Make sure the caller is mapping a real resource for this device */
for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ res = &dev->resource[i];
+ if (!res->flags)
+ continue;
+
+ pci_resource_to_user(dev, i, res, &start, &end);
+ if (!(offset >= start && offset <= end))
+ continue;
+
+ vma->vm_pgoff = (res->start + (offset - start)) >> PAGE_SHIFT;
if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
break;
}
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| Date | 2016-05-04 17:20 +0200 |
| Message-ID | <rv6KK-6s5-25@gated-at.bofh.it> |
| In reply to | #1393974 |
On Tue, May 03, 2016 at 10:52:33PM -0700, Yinghai Lu wrote:
> On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >> I did not propose changing any user-visible ABI. To recap what I did
> >> propose:
> >
> > I want to avoid introduce one strange pci_user_to_resource.
> >
> >>
> >> - The sysfs path uses offsets between 0 and BAR size on all arches.
> >> It uses pci_resource_to_user() today, but I think it should not.
> >>
> >> - The procfs path uses offsets of resource values (CPU physical
> >> addresses) on most architectures, but uses something else, e.g.,
> >> BAR values, on others. pci_resource_to_user() does this
> >> translation. The procfs path does not use pci_resource_to_user()
> >> today, but I think it should.
> >
> > current powerpc pci_resource_to_user is strange:
> > it will return resource start for io mem.
> > but will return BAR (?) start for io port.
> >
> > sparc pci_resource_to_user does return BAR value for iomem.
That means it should be implemented using pcibios_resource_to_bus().
> >> - This implies that pci_mmap_page_range() should deal with resource
> >> values (CPU physical addresses), and proc_bus_pci_mmap() should do
> >> any necessary arch-specific translation from BAR values to
> >> resource values.
> >
> > so will need one different version pci_user_to_resource.
> > and can not use pcibios_bus_to_resource directly, and will be another mess.
What mess do you mean? The fact that you could only use
pcibios_bus_to_resource() for MEM, and something else for IO? Even
if we could only use pcibios_bus_to_resource() for MEM, that sounds
like an improvement, not a mess.
> looks like we can avoid that pci_user_to_resource() via trying out.
> Please check it:
>
>
> Subject: [RFC PATCH] PCI: Let pci_mmap_page_range() take resource addr
>
> Some arch where cpu address (resource value) is not same as pci bus address
> (BAR value in pci BAR registers), include sparc, powerpc, microblaze.
This comment is irrelevant. *Lots* of arches have CPU addresses
different from PCI bus addresses, including alpha, arm, ia64, mips,
mn10300, parisc, tile, xtensa, and x86.
> In 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
> to check exposed value with resource start/end in proc mmap path.
> | start = vma->vm_pgoff;
> | size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
> | pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
> | pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
> | if (start >= pci_start && start < pci_start + size &&
> | start + nr <= pci_start + size)
> That would break sparc that exposed value is still BAR value.
>
> According to Bjorn, we could just pass resource addr instead of BAR.
>
> In the patch:
> 1. in proc path: proc_bus_pci_mmap, try convert back to resource
> before calling pci_mmap_page_range
> 2. in sysfs path: pci_mmap_resource will just offset with resource start.
> 3. all pci_mmap_page_range will all have vma->vm_pgoff with in resource range
> instead of BAR value.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> ---
> arch/microblaze/pci/pci-common.c | 14 ++++----------
> arch/powerpc/kernel/pci-common.c | 14 ++++----------
> arch/sparc/kernel/pci.c | 27 +++++++++------------------
> arch/xtensa/kernel/pci.c | 11 ++++-------
> drivers/pci/pci-sysfs.c | 8 +-------
> drivers/pci/proc.c | 13 +++++++++++++
> 6 files changed, 35 insertions(+), 52 deletions(-)
>
> Index: linux-2.6/arch/microblaze/pci/pci-common.c
> ===================================================================
> --- linux-2.6.orig/arch/microblaze/pci/pci-common.c
> +++ linux-2.6/arch/microblaze/pci/pci-common.c
> @@ -169,23 +169,16 @@ static struct resource *__pci_mmap_make_
> enum pci_mmap_state mmap_state)
> {
> struct pci_controller *hose = pci_bus_to_host(dev->bus);
> - unsigned long io_offset = 0;
> int i, res_bit;
>
> if (!hose)
> return NULL; /* should never happen */
>
> /* If memory, add on the PCI bridge address offset */
> - if (mmap_state == pci_mmap_mem) {
> -#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
> - *offset += hose->pci_mem_offset;
> -#endif
> + if (mmap_state == pci_mmap_mem)
> res_bit = IORESOURCE_MEM;
> - } else {
> - io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
> - *offset += io_offset;
> + else
> res_bit = IORESOURCE_IO;
> - }
>
> /*
> * Check that the offset requested corresponds to one of the
> @@ -209,7 +202,8 @@ static struct resource *__pci_mmap_make_
>
> /* found it! construct the final physical address */
> if (mmap_state == pci_mmap_io)
> - *offset += hose->io_base_phys - io_offset;
> + *offset += hose->io_base_phys -
> + ((unsigned long)hose->io_base_virt - _IO_BASE);
> return rp;
> }
Most of __pci_mmap_make_offset() is pointless.
We might need something there for I/O regions, but for MEM, the
vma->vm_pgoff coming into pci_mmap_page_range() should be exactly what
we need and we shouldn't touch it. I think __pci_mmap_make_offset()
actually does leave it alone for MEM, but you have to read the code
carefully to figure that out.
All the validation stuff ("Check that the offset requested corresponds
to one of the resources...") should be removed or moved to
pci_mmap_fits().
> Index: linux-2.6/arch/powerpc/kernel/pci-common.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
> +++ linux-2.6/arch/powerpc/kernel/pci-common.c
> @@ -308,23 +308,16 @@ static struct resource *__pci_mmap_make_
> enum pci_mmap_state mmap_state)
> {
> struct pci_controller *hose = pci_bus_to_host(dev->bus);
> - unsigned long io_offset = 0;
> int i, res_bit;
>
> if (hose == NULL)
> return NULL; /* should never happen */
>
> /* If memory, add on the PCI bridge address offset */
> - if (mmap_state == pci_mmap_mem) {
> -#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
> - *offset += hose->pci_mem_offset;
> -#endif
> + if (mmap_state == pci_mmap_mem)
> res_bit = IORESOURCE_MEM;
> - } else {
> - io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
> - *offset += io_offset;
> + else
> res_bit = IORESOURCE_IO;
> - }
>
> /*
> * Check that the offset requested corresponds to one of the
> @@ -348,7 +341,8 @@ static struct resource *__pci_mmap_make_
>
> /* found it! construct the final physical address */
> if (mmap_state == pci_mmap_io)
> - *offset += hose->io_base_phys - io_offset;
> + *offset += hose->io_base_phys -
> + ((unsigned long)hose->io_base_virt - _IO_BASE);
> return rp;
> }
>
> Index: linux-2.6/arch/sparc/kernel/pci.c
> ===================================================================
> --- linux-2.6.orig/arch/sparc/kernel/pci.c
> +++ linux-2.6/arch/sparc/kernel/pci.c
> @@ -743,30 +743,21 @@ static int __pci_mmap_make_offset_bus(st
> enum pci_mmap_state mmap_state)
> {
> struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
> - unsigned long space_size, user_offset, user_size;
> + unsigned long start, end;
> + struct resource *res;
>
> - if (mmap_state == pci_mmap_io) {
> - space_size = resource_size(&pbm->io_space);
> - } else {
> - space_size = resource_size(&pbm->mem_space);
> - }
> + if (mmap_state == pci_mmap_io)
> + res = &pbm->io_space;
> + else
> + res = &pbm->mem_space;
>
> /* Make sure the request is in range. */
> - user_offset = vma->vm_pgoff << PAGE_SHIFT;
> - user_size = vma->vm_end - vma->vm_start;
> + start = vma->vm_pgoff << PAGE_SHIFT;
> + end = vma->vm_end - vma->vm_start + start - 1;
>
> - if (user_offset >= space_size ||
> - (user_offset + user_size) > space_size)
> + if (!((res->start <= start) && (res->end >= end)))
> return -EINVAL;
>
> - if (mmap_state == pci_mmap_io) {
> - vma->vm_pgoff = (pbm->io_space.start +
> - user_offset) >> PAGE_SHIFT;
> - } else {
> - vma->vm_pgoff = (pbm->mem_space.start +
> - user_offset) >> PAGE_SHIFT;
> - }
> -
> return 0;
> }
>
> Index: linux-2.6/arch/xtensa/kernel/pci.c
> ===================================================================
> --- linux-2.6.orig/arch/xtensa/kernel/pci.c
> +++ linux-2.6/arch/xtensa/kernel/pci.c
> @@ -288,20 +288,16 @@ __pci_mmap_make_offset(struct pci_dev *d
> {
> struct pci_controller *pci_ctrl = (struct pci_controller*) dev->sysdata;
> unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
> - unsigned long io_offset = 0;
> int i, res_bit;
>
> if (pci_ctrl == 0)
> return -EINVAL; /* should never happen */
>
> /* If memory, add on the PCI bridge address offset */
> - if (mmap_state == pci_mmap_mem) {
> + if (mmap_state == pci_mmap_mem)
> res_bit = IORESOURCE_MEM;
> - } else {
> - io_offset = (unsigned long)pci_ctrl->io_space.base;
> - offset += io_offset;
> + else
> res_bit = IORESOURCE_IO;
> - }
>
> /*
> * Check that the offset requested corresponds to one of the
> @@ -325,7 +321,8 @@ __pci_mmap_make_offset(struct pci_dev *d
>
> /* found it! construct the final physical address */
> if (mmap_state == pci_mmap_io)
> - offset += pci_ctrl->io_space.start - io_offset;
> + offset += pci_ctrl->io_space.start -
> + pci_ctrl->io_space.base;
> vma->vm_pgoff = offset >> PAGE_SHIFT;
> return 0;
> }
> Index: linux-2.6/drivers/pci/pci-sysfs.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-sysfs.c
> +++ linux-2.6/drivers/pci/pci-sysfs.c
> @@ -999,7 +999,6 @@ static int pci_mmap_resource(struct kobj
> struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
> struct resource *res = attr->private;
> enum pci_mmap_state mmap_type;
> - resource_size_t start, end;
> int i;
>
> for (i = 0; i < PCI_ROM_RESOURCE; i++)
> @@ -1020,12 +1019,7 @@ static int pci_mmap_resource(struct kobj
> return -EINVAL;
> }
>
> - /* 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, i, res, &start, &end);
> - vma->vm_pgoff += start >> PAGE_SHIFT;
> + vma->vm_pgoff += res->start >> PAGE_SHIFT;
> mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
> return pci_mmap_page_range(pdev, vma, mmap_type, write_combine);
This is a definite improvement. This path doesn't use user addresses,
so getting rid of pci_resource_to_user() here helps a lot.
> }
> Index: linux-2.6/drivers/pci/proc.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/proc.c
> +++ linux-2.6/drivers/pci/proc.c
> @@ -231,13 +231,26 @@ static int proc_bus_pci_mmap(struct file
> {
> struct pci_dev *dev = PDE_DATA(file_inode(file));
> struct pci_filp_private *fpriv = file->private_data;
> + resource_size_t start, end, offset;
> + struct resource *res;
> int i, ret;
>
> if (!capable(CAP_SYS_RAWIO))
> return -EPERM;
>
> + offset = vma->vm_pgoff << PAGE_SHIFT;
> +
> /* Make sure the caller is mapping a real resource for this device */
> for (i = 0; i < PCI_ROM_RESOURCE; i++) {
> + res = &dev->resource[i];
> + if (!res->flags)
> + continue;
> +
> + pci_resource_to_user(dev, i, res, &start, &end);
> + if (!(offset >= start && offset <= end))
> + continue;
> +
> + vma->vm_pgoff = (res->start + (offset - start)) >> PAGE_SHIFT;
> if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
This is sort of OK, but I think we can do better. I don't see any
problem with introducing pci_user_to_resource() as the inverse of
pci_resource_to_user(). I think it will make this code read much
better.
The default pci_user_to_resource() would do nothing, just like the
default pci_resource_to_user().
For sparc, I think pci_user_to_resource() can use
pcibios_bus_to_resource(), and pci_resource_to_user() can be rewritten
to use pcibios_resource_to_bus(). That makes it much more obvious
what's happening.
It looks like microblaze and powerpc should use
pcibios_resource_to_bus() for I/O resources and the default "user
address == resource address" for MMIO (?)
My goal is to make pci_mmap_resource() and proc_bus_pci_mmap() look
very similar, e.g.,
/* locate resource */
pci_user_to_resource() # only in proc_bus_pci_mmap()
if (!pci_mmap_fits()) {
WARN(...);
return -EINVAL;
}
pci_mmap_page_range();
Obviously there are several steps in getting here. Reworking
pci_resource_to_user() to use pcibios_resource_to_bus() when possible
would be a good start.
Bjorn
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-04 20:50 +0200 |
| Message-ID | <rva1Y-TX-7@gated-at.bofh.it> |
| In reply to | #1394439 |
On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> What mess do you mean? The fact that you could only use
> pcibios_bus_to_resource() for MEM, and something else for IO? Even
> if we could only use pcibios_bus_to_resource() for MEM, that sounds
> like an improvement, not a mess.
I means that we need create another 5 pci_user_to_resource
arch/microblaze/pci/pci-common.c:void pci_resource_to_user(const
struct pci_dev *dev, int bar,
arch/mips/include/asm/pci.h:static inline void
pci_resource_to_user(const struct pci_dev *dev, int bar,
arch/powerpc/kernel/pci-common.c:void pci_resource_to_user(const
struct pci_dev *dev, int bar,
arch/sparc/kernel/pci.c:void pci_resource_to_user(const struct pci_dev
*pdev, int bar,
include/linux/pci.h:static inline void pci_resource_to_user(const
struct pci_dev *dev, int bar,
>
> Most of __pci_mmap_make_offset() is pointless.
we can clean up them later.
>
> We might need something there for I/O regions, but for MEM, the
> vma->vm_pgoff coming into pci_mmap_page_range() should be exactly what
> we need and we shouldn't touch it. I think __pci_mmap_make_offset()
> actually does leave it alone for MEM, but you have to read the code
> carefully to figure that out.
>
> All the validation stuff ("Check that the offset requested corresponds
> to one of the resources...") should be removed or moved to
> pci_mmap_fits().
ok, will give it try.
>> @@ -231,13 +231,26 @@ static int proc_bus_pci_mmap(struct file
>> {
>> struct pci_dev *dev = PDE_DATA(file_inode(file));
>> struct pci_filp_private *fpriv = file->private_data;
>> + resource_size_t start, end, offset;
>> + struct resource *res;
>> int i, ret;
>>
>> if (!capable(CAP_SYS_RAWIO))
>> return -EPERM;
>>
>> + offset = vma->vm_pgoff << PAGE_SHIFT;
>> +
>> /* Make sure the caller is mapping a real resource for this device */
>> for (i = 0; i < PCI_ROM_RESOURCE; i++) {
>> + res = &dev->resource[i];
>> + if (!res->flags)
>> + continue;
>> +
>> + pci_resource_to_user(dev, i, res, &start, &end);
>> + if (!(offset >= start && offset <= end))
>> + continue;
>> +
>> + vma->vm_pgoff = (res->start + (offset - start)) >> PAGE_SHIFT;
>> if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
>
> This is sort of OK, but I think we can do better. I don't see any
> problem with introducing pci_user_to_resource() as the inverse of
> pci_resource_to_user(). I think it will make this code read much
> better.
>
> The default pci_user_to_resource() would do nothing, just like the
> default pci_resource_to_user().
>
> For sparc, I think pci_user_to_resource() can use
> pcibios_bus_to_resource(), and pci_resource_to_user() can be rewritten
> to use pcibios_resource_to_bus(). That makes it much more obvious
> what's happening.
>
> It looks like microblaze and powerpc should use
> pcibios_resource_to_bus() for I/O resources and the default "user
> address == resource address" for MMIO (?)
>
> My goal is to make pci_mmap_resource() and proc_bus_pci_mmap() look
> very similar, e.g.,
>
> /* locate resource */
> pci_user_to_resource() # only in proc_bus_pci_mmap()
> if (!pci_mmap_fits()) {
> WARN(...);
> return -EINVAL;
> }
> pci_mmap_page_range();
>
> Obviously there are several steps in getting here. Reworking
> pci_resource_to_user() to use pcibios_resource_to_bus() when possible
> would be a good start.
I would like to avoid adding pci_user_to_resource, and put extra calling
pci_resource_to_user pci_mmap_fits instead.
Thanks
Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-05 02:30 +0200 |
| Message-ID | <rvfkZ-60t-5@gated-at.bofh.it> |
| In reply to | #1394625 |
On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>> My goal is to make pci_mmap_resource() and proc_bus_pci_mmap() look
>> very similar, e.g.,
>>
>> /* locate resource */
>> pci_user_to_resource() # only in proc_bus_pci_mmap()
>> if (!pci_mmap_fits()) {
>> WARN(...);
>> return -EINVAL;
>> }
>> pci_mmap_page_range();
Please check v2.
Subject: [RFC PATCH v2] PCI: Let pci_mmap_page_range() take resource addr
In 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
to check exposed value with resource start/end in proc mmap path.
| start = vma->vm_pgoff;
| size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
| pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
| pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
| if (start >= pci_start && start < pci_start + size &&
| start + nr <= pci_start + size)
That would break sparc that exposed value is still BAR value.
In the patch:
1. in proc path: proc_bus_pci_mmap, try convert back to resource
before calling pci_mmap_page_range
2. in sysfs path: pci_mmap_resource will just offset with resource start.
3. all pci_mmap_page_range will all have vma->vm_pgoff with in resource
range instead of BAR value.
4. remove __pci_mmap_make_offset, as the checking is done
in pci_mmap_fits().
-v2: add pci_user_to_resource and remove __pci_mmap_make_offset
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/microblaze/pci/pci-common.c | 94 ++++++++-----------------------
arch/powerpc/kernel/pci-common.c | 95 ++++++++-----------------------
arch/sparc/kernel/pci.c | 117 ---------------------------------------
arch/xtensa/kernel/pci.c | 73 ++----------------------
drivers/pci/pci-sysfs.c | 23 ++++---
drivers/pci/pci.h | 2
drivers/pci/proc.c | 59 ++++++++++++++++---
7 files changed, 124 insertions(+), 339 deletions(-)
Index: linux-2.6/arch/microblaze/pci/pci-common.c
===================================================================
--- linux-2.6.orig/arch/microblaze/pci/pci-common.c
+++ linux-2.6/arch/microblaze/pci/pci-common.c
@@ -153,63 +153,25 @@ void pcibios_set_master(struct pci_dev *
* -- paulus.
*/
-/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
- resource_size_t *offset,
- enum pci_mmap_state mmap_state)
+static struct resource *pci_find_resource(struct pci_dev *dev,
+ resource_size_t offset, int flags)
{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (!hose)
- return NULL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
+ int i;
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
+ if (!(rp->flags & flags))
+ continue;
- /* Active and same type? */
- if ((flags & res_bit) == 0)
+ if (pci_resource_len(dev, i) == 0)
continue;
/* In the range of this resource? */
- if (*offset < (rp->start & PAGE_MASK) || *offset > rp->end)
+ if (offset < (rp->start & PAGE_MASK) ||
+ offset > rp->end)
continue;
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
return rp;
}
@@ -236,7 +198,7 @@ static pgprot_t __pci_mmap_set_pgprot(st
if (mmap_state != pci_mmap_mem)
write_combine = 0;
else if (write_combine == 0) {
- if (rp->flags & IORESOURCE_PREFETCH)
+ if (rp && (rp->flags & IORESOURCE_PREFETCH))
write_combine = 1;
}
@@ -256,27 +218,13 @@ pgprot_t pci_phys_mem_access_prot(struct
struct pci_dev *pdev = NULL;
struct resource *found = NULL;
resource_size_t offset = ((resource_size_t)pfn) << PAGE_SHIFT;
- int i;
if (page_is_ram(pfn))
return prot;
prot = pgprot_noncached(prot);
for_each_pci_dev(pdev) {
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &pdev->resource[i];
- int flags = rp->flags;
-
- /* Active and same type? */
- if ((flags & IORESOURCE_MEM) == 0)
- continue;
- /* In the range of this resource? */
- if (offset < (rp->start & PAGE_MASK) ||
- offset > rp->end)
- continue;
- found = rp;
- break;
- }
+ found = pci_find_resource(pdev, offset, IORESOURCE_MEM);
if (found)
break;
}
@@ -305,14 +253,24 @@ pgprot_t pci_phys_mem_access_prot(struct
int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
enum pci_mmap_state mmap_state, int write_combine)
{
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
resource_size_t offset =
((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
struct resource *rp;
- int ret;
+ int ret, flags;
+
+ if (!hose)
+ return -EINVAL; /* should never happen */
- rp = __pci_mmap_make_offset(dev, &offset, mmap_state);
- if (rp == NULL)
- return -EINVAL;
+ if (mmap_state == pci_mmap_mem)
+ flags = IORESOURCE_MEM;
+ else
+ flags = IORESOURCE_IO;
+
+ rp = pci_find_resource(dev, offset, flags);
+ if (mmap_state == pci_mmap_io)
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
vma->vm_pgoff = offset >> PAGE_SHIFT;
vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
@@ -491,9 +449,7 @@ void pci_resource_to_user(const struct p
*
* Hopefully, the sysfs insterface is immune to that gunk. Once X
* has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
+ * 2 lines below and pass down a BAR value to userland.
*
* BenH.
*/
Index: linux-2.6/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
+++ linux-2.6/arch/powerpc/kernel/pci-common.c
@@ -292,63 +292,25 @@ static int pci_read_irq_line(struct pci_
* -- paulus.
*/
-/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
- resource_size_t *offset,
- enum pci_mmap_state mmap_state)
+static struct resource *pci_find_resource(struct pci_dev *dev,
+ resource_size_t offset, int flags)
{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (hose == NULL)
- return NULL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
+ int i;
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
+ if (!(rp->flags & flags))
+ continue;
- /* Active and same type? */
- if ((flags & res_bit) == 0)
+ if (pci_resource_len(dev, i) == 0)
continue;
/* In the range of this resource? */
- if (*offset < (rp->start & PAGE_MASK) || *offset > rp->end)
+ if (offset < (rp->start & PAGE_MASK) ||
+ offset > rp->end)
continue;
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
return rp;
}
@@ -374,7 +336,7 @@ static pgprot_t __pci_mmap_set_pgprot(st
if (mmap_state != pci_mmap_mem)
write_combine = 0;
else if (write_combine == 0) {
- if (rp->flags & IORESOURCE_PREFETCH)
+ if (rp && (rp->flags & IORESOURCE_PREFETCH))
write_combine = 1;
}
@@ -385,6 +347,7 @@ static pgprot_t __pci_mmap_set_pgprot(st
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
@@ -398,27 +361,13 @@ pgprot_t pci_phys_mem_access_prot(struct
struct pci_dev *pdev = NULL;
struct resource *found = NULL;
resource_size_t offset = ((resource_size_t)pfn) << PAGE_SHIFT;
- int i;
if (page_is_ram(pfn))
return prot;
prot = pgprot_noncached(prot);
for_each_pci_dev(pdev) {
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &pdev->resource[i];
- int flags = rp->flags;
-
- /* Active and same type? */
- if ((flags & IORESOURCE_MEM) == 0)
- continue;
- /* In the range of this resource? */
- if (offset < (rp->start & PAGE_MASK) ||
- offset > rp->end)
- continue;
- found = rp;
- break;
- }
+ found = pci_find_resource(pdev, offset, IORESOURCE_MEM);
if (found)
break;
}
@@ -448,14 +397,24 @@ pgprot_t pci_phys_mem_access_prot(struct
int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
enum pci_mmap_state mmap_state, int write_combine)
{
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
resource_size_t offset =
((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
struct resource *rp;
- int ret;
+ int ret, flags;
+
+ if (hose == NULL)
+ return -EINVAL; /* should never happen */
+
+ if (mmap_state == pci_mmap_mem)
+ flags = IORESOURCE_MEM;
+ else
+ flags = IORESOURCE_IO;
+ rp = pci_find_resource(dev, offset, flags);
- rp = __pci_mmap_make_offset(dev, &offset, mmap_state);
- if (rp == NULL)
- return -EINVAL;
+ if (mmap_state == pci_mmap_io)
+ offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
vma->vm_pgoff = offset >> PAGE_SHIFT;
vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
@@ -630,9 +589,7 @@ void pci_resource_to_user(const struct p
*
* Hopefully, the sysfs insterface is immune to that gunk. Once X
* has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
+ * 2 lines below and pass down a BAR value to userland.
*
* BenH.
*/
Index: linux-2.6/arch/sparc/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/pci.c
+++ linux-2.6/arch/sparc/kernel/pci.c
@@ -732,119 +732,6 @@ int pcibios_enable_device(struct pci_dev
/* Platform support for /proc/bus/pci/X/Y mmap()s. */
-/* If the user uses a host-bridge as the PCI device, he may use
- * this to perform a raw mmap() of the I/O or MEM space behind
- * that controller.
- *
- * This can be useful for execution of x86 PCI bios initialization code
- * on a PCI card, like the xfree86 int10 stuff does.
- */
-static int __pci_mmap_make_offset_bus(struct pci_dev *pdev, struct
vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
- unsigned long space_size, user_offset, user_size;
-
- if (mmap_state == pci_mmap_io) {
- space_size = resource_size(&pbm->io_space);
- } else {
- space_size = resource_size(&pbm->mem_space);
- }
-
- /* Make sure the request is in range. */
- user_offset = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
-
- if (user_offset >= space_size ||
- (user_offset + user_size) > space_size)
- return -EINVAL;
-
- if (mmap_state == pci_mmap_io) {
- vma->vm_pgoff = (pbm->io_space.start +
- user_offset) >> PAGE_SHIFT;
- } else {
- vma->vm_pgoff = (pbm->mem_space.start +
- user_offset) >> PAGE_SHIFT;
- }
-
- return 0;
-}
-
-/* Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static int __pci_mmap_make_offset(struct pci_dev *pdev,
- struct vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- unsigned long user_paddr, user_size;
- int i, err;
-
- /* First compute the physical address in vma->vm_pgoff,
- * making sure the user offset is within range in the
- * appropriate PCI space.
- */
- err = __pci_mmap_make_offset_bus(pdev, vma, mmap_state);
- if (err)
- return err;
-
- /* If this is a mapping on a host bridge, any address
- * is OK.
- */
- if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_HOST)
- return err;
-
- /* Otherwise make sure it's in the range for one of the
- * device's resources.
- */
- user_paddr = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
-
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &pdev->resource[i];
- resource_size_t aligned_end;
-
- /* Active? */
- if (!rp->flags)
- continue;
-
- /* Same type? */
- if (i == PCI_ROM_RESOURCE) {
- if (mmap_state != pci_mmap_mem)
- continue;
- } else {
- if ((mmap_state == pci_mmap_io &&
- (rp->flags & IORESOURCE_IO) == 0) ||
- (mmap_state == pci_mmap_mem &&
- (rp->flags & IORESOURCE_MEM) == 0))
- continue;
- }
-
- /* Align the resource end to the next page address.
- * PAGE_SIZE intentionally added instead of (PAGE_SIZE - 1),
- * because actually we need the address of the next byte
- * after rp->end.
- */
- aligned_end = (rp->end + PAGE_SIZE) & PAGE_MASK;
-
- if ((rp->start <= user_paddr) &&
- (user_paddr + user_size) <= aligned_end)
- break;
- }
-
- if (i > PCI_ROM_RESOURCE)
- return -EINVAL;
-
- return 0;
-}
-
/* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -868,10 +755,6 @@ int pci_mmap_page_range(struct pci_dev *
{
int ret;
- ret = __pci_mmap_make_offset(dev, vma, mmap_state);
- if (ret < 0)
- return ret;
-
__pci_mmap_set_pgprot(dev, vma, mmap_state);
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
Index: linux-2.6/arch/xtensa/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/xtensa/kernel/pci.c
+++ linux-2.6/arch/xtensa/kernel/pci.c
@@ -272,68 +272,6 @@ pci_controller_num(struct pci_dev *dev)
*/
/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static __inline__ int
-__pci_mmap_make_offset(struct pci_dev *dev, struct vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- struct pci_controller *pci_ctrl = (struct pci_controller*) dev->sysdata;
- unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (pci_ctrl == 0)
- return -EINVAL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)pci_ctrl->io_space.base;
- offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
-
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
-
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
-
- /* Active and same type? */
- if ((flags & res_bit) == 0)
- continue;
-
- /* In the range of this resource? */
- if (offset < (rp->start & PAGE_MASK) || offset > rp->end)
- continue;
-
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- offset += pci_ctrl->io_space.start - io_offset;
- vma->vm_pgoff = offset >> PAGE_SHIFT;
- return 0;
- }
-
- return -EINVAL;
-}
-
-/*
* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -366,11 +304,16 @@ int pci_mmap_page_range(struct pci_dev *
enum pci_mmap_state mmap_state,
int write_combine)
{
+ struct pci_controller *pci_ctrl = (struct pci_controller *)dev->sysdata;
+ unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
int ret;
- ret = __pci_mmap_make_offset(dev, vma, mmap_state);
- if (ret < 0)
- return ret;
+ if (pci_ctrl == 0)
+ return -EINVAL; /* should never happen */
+
+ if (mmap_state == pci_mmap_io)
+ offset += pci_ctrl->io_space.start - pci_ctrl->io_space.base;
+ vma->vm_pgoff = offset >> PAGE_SHIFT;
__pci_mmap_set_pgprot(dev, vma, mmap_state, write_combine);
Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -967,12 +967,23 @@ void pci_remove_legacy_files(struct pci_
#ifdef HAVE_PCI_MMAP
int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma,
+ enum pci_mmap_state mmap_type,
enum pci_mmap_api mmap_api)
{
unsigned long nr, start, size, pci_start;
+ int res_bit;
if (pci_resource_len(pdev, resno) == 0)
return 0;
+
+ if (mmap_type == pci_mmap_mem)
+ res_bit = IORESOURCE_MEM;
+ else
+ res_bit = IORESOURCE_IO;
+
+ if (!(pci_resource_flags(pdev, resno) & res_bit))
+ return 0;
+
nr = vma_pages(vma);
start = vma->vm_pgoff;
size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
@@ -999,7 +1010,6 @@ static int pci_mmap_resource(struct kobj
struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
struct resource *res = attr->private;
enum pci_mmap_state mmap_type;
- resource_size_t start, end;
int i;
for (i = 0; i < PCI_ROM_RESOURCE; i++)
@@ -1011,7 +1021,8 @@ static int pci_mmap_resource(struct kobj
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
- if (!pci_mmap_fits(pdev, i, vma, PCI_MMAP_SYSFS)) {
+ mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
+ if (!pci_mmap_fits(pdev, i, vma, mmap_type, PCI_MMAP_SYSFS)) {
WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page
0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n",
current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff,
pci_name(pdev), i,
@@ -1020,13 +1031,7 @@ static int pci_mmap_resource(struct kobj
return -EINVAL;
}
- /* 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, i, res, &start, &end);
- vma->vm_pgoff += start >> PAGE_SHIFT;
- mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
+ vma->vm_pgoff += res->start >> PAGE_SHIFT;
return pci_mmap_page_range(pdev, vma, mmap_type, write_combine);
}
Index: linux-2.6/drivers/pci/proc.c
===================================================================
--- linux-2.6.orig/drivers/pci/proc.c
+++ linux-2.6/drivers/pci/proc.c
@@ -227,26 +227,67 @@ static long proc_bus_pci_ioctl(struct fi
}
#ifdef HAVE_PCI_MMAP
+
+static int pci_user_to_resource(struct pci_dev *dev, resource_size_t *offset,
+ resource_size_t size, int flags)
+{
+ int i;
+
+ for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ resource_size_t start, end;
+ struct resource *res = &dev->resource[i];
+
+ if (!(res->flags & flags))
+ continue;
+
+ if (pci_resource_len(dev, i) == 0)
+ continue;
+
+ pci_resource_to_user(dev, i, res, &start, &end);
+ if (start <= *offset && (*offset + size - 1) <= end) {
+ *offset = res->start + (*offset - start);
+ return i;
+ }
+
+ return i;
+ }
+
+ return -ENODEV;
+}
+
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;
+ enum pci_mmap_state mmap_type = fpriv->mmap_state;
+ resource_size_t offset, size;
+ int i, ret, flags;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- /* Make sure the caller is mapping a real resource for this device */
- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
- if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
- break;
- }
-
- if (i >= PCI_ROM_RESOURCE)
+ offset = vma->vm_pgoff << PAGE_SHIFT;
+ size = vma->vm_end - vma->vm_start;
+ if (mmap_type == pci_mmap_mem)
+ flags = IORESOURCE_MEM;
+ else
+ flags = IORESOURCE_IO;
+ i = pci_user_to_resource(dev, &offset, size, flags);
+ if (i < 0)
return -ENODEV;
+ vma->vm_pgoff = offset >> PAGE_SHIFT;
+ if (!pci_mmap_fits(dev, i, vma, mmap_type, PCI_MMAP_PROCFS)) {
+ WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page
0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n",
+ current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff,
+ pci_name(dev), i,
+ (u64)pci_resource_start(dev, i),
+ (u64)pci_resource_len(dev, i));
+ return -EINVAL;
+ }
+
ret = pci_mmap_page_range(dev, vma,
- fpriv->mmap_state,
+ mmap_type,
fpriv->write_combine);
if (ret < 0)
return ret;
Index: linux-2.6/drivers/pci/pci.h
===================================================================
--- linux-2.6.orig/drivers/pci/pci.h
+++ linux-2.6/drivers/pci/pci.h
@@ -30,7 +30,7 @@ enum pci_mmap_api {
PCI_MMAP_PROCFS /* mmap on /proc/bus/pci/<BDF> */
};
int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vmai,
- enum pci_mmap_api mmap_api);
+ enum pci_mmap_state mmap_type, enum pci_mmap_api mmap_api);
#endif
int pci_probe_reset_function(struct pci_dev *dev);
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-05 18:00 +0200 |
| Message-ID | <rvtR0-2Dm-7@gated-at.bofh.it> |
| In reply to | #1394807 |
On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
>>> My goal is to make pci_mmap_resource() and proc_bus_pci_mmap() look
>>> very similar, e.g.,
>>>
>>> /* locate resource */
>>> pci_user_to_resource() # only in proc_bus_pci_mmap()
>>> if (!pci_mmap_fits()) {
>>> WARN(...);
>>> return -EINVAL;
>>> }
>>> pci_mmap_page_range();
>
v3, that have more change to pass *res to make powerpc prot setting simple.
Question for BenH and DavidM:
For powerpc io port, we still need extra offset from resource address
to final address.
resource_size_t offset =
((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
+ if (mmap_state == pci_mmap_io) {
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
+
+ /* hose should never be NULL */
+ offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
+ }
vma->vm_pgoff = offset >> PAGE_SHIFT;
but sparc does not need that trick.
why ?
Thanks
Yinghai
---
Subject: [RFC PATCH v3 2/2] PCI: Let pci_mmap_page_range() take resource addr
In 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
to check exposed value with resource start/end in proc mmap path.
| start = vma->vm_pgoff;
| size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
| pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
| pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
| if (start >= pci_start && start < pci_start + size &&
| start + nr <= pci_start + size)
That would break sparc that exposed value is still BAR value.
In the patch:
1. in proc path: proc_bus_pci_mmap, try convert back to resource
before calling pci_mmap_page_range
2. in sysfs path: pci_mmap_resource will just offset with resource start.
3. all pci_mmap_page_range will all have vma->vm_pgoff with in resource
range instead of BAR value.
4. remove __pci_mmap_make_offset, as the checking is done
in pci_mmap_fits().
-v2: add pci_user_to_resource and remove __pci_mmap_make_offset
-v3: pass resource pointer with pci_mmap_page_range()
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
arch/microblaze/pci/pci-common.c | 78 ++------------------------
arch/powerpc/kernel/pci-common.c | 78 ++------------------------
arch/sparc/kernel/pci.c | 117 ---------------------------------------
arch/xtensa/kernel/pci.c | 75 +++----------------------
drivers/pci/pci-sysfs.c | 23 ++++---
drivers/pci/proc.c | 57 ++++++++++++++++---
6 files changed, 88 insertions(+), 340 deletions(-)
Index: linux-2.6/arch/microblaze/pci/pci-common.c
===================================================================
--- linux-2.6.orig/arch/microblaze/pci/pci-common.c
+++ linux-2.6/arch/microblaze/pci/pci-common.c
@@ -154,69 +154,6 @@ void pcibios_set_master(struct pci_dev *
*/
/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
- resource_size_t *offset,
- enum pci_mmap_state mmap_state)
-{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (!hose)
- return NULL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
-
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
-
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
-
- /* Active and same type? */
- if ((flags & res_bit) == 0)
- continue;
-
- /* In the range of this resource? */
- if (*offset < (rp->start & PAGE_MASK) || *offset > rp->end)
- continue;
-
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
- return rp;
- }
-
- return NULL;
-}
-
-/*
* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -308,12 +245,15 @@ int pci_mmap_page_range(struct pci_dev *
{
resource_size_t offset =
((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
- struct resource *rp;
int ret;
- rp = __pci_mmap_make_offset(dev, &offset, mmap_state);
- if (rp == NULL)
- return -EINVAL;
+ if (mmap_state == pci_mmap_io) {
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
+
+ /* hose should never be NULL */
+ *offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
+ }
vma->vm_pgoff = offset >> PAGE_SHIFT;
vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
@@ -492,9 +432,7 @@ void pci_resource_to_user(const struct p
*
* Hopefully, the sysfs insterface is immune to that gunk. Once X
* has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
+ * 2 lines below and pass down a BAR value to userland.
*
* BenH.
*/
Index: linux-2.6/arch/powerpc/kernel/pci-common.c
===================================================================
--- linux-2.6.orig/arch/powerpc/kernel/pci-common.c
+++ linux-2.6/arch/powerpc/kernel/pci-common.c
@@ -293,69 +293,6 @@ static int pci_read_irq_line(struct pci_
*/
/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static struct resource *__pci_mmap_make_offset(struct pci_dev *dev,
- resource_size_t *offset,
- enum pci_mmap_state mmap_state)
-{
- struct pci_controller *hose = pci_bus_to_host(dev->bus);
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (hose == NULL)
- return NULL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
-#if 0 /* See comment in pci_resource_to_user() for why this is disabled */
- *offset += hose->pci_mem_offset;
-#endif
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)hose->io_base_virt - _IO_BASE;
- *offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
-
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
-
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
-
- /* Active and same type? */
- if ((flags & res_bit) == 0)
- continue;
-
- /* In the range of this resource? */
- if (*offset < (rp->start & PAGE_MASK) || *offset > rp->end)
- continue;
-
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- *offset += hose->io_base_phys - io_offset;
- return rp;
- }
-
- return NULL;
-}
-
-/*
* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -451,12 +388,15 @@ int pci_mmap_page_range(struct pci_dev *
{
resource_size_t offset =
((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
- struct resource *rp;
int ret;
- rp = __pci_mmap_make_offset(dev, &offset, mmap_state);
- if (rp == NULL)
- return -EINVAL;
+ if (mmap_state == pci_mmap_io) {
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
+
+ /* hose should never be NULL */
+ offset += hose->io_base_phys -
+ ((unsigned long)hose->io_base_virt - _IO_BASE);
+ }
vma->vm_pgoff = offset >> PAGE_SHIFT;
vma->vm_page_prot = __pci_mmap_set_pgprot(dev, rp,
@@ -631,9 +571,7 @@ void pci_resource_to_user(const struct p
*
* Hopefully, the sysfs insterface is immune to that gunk. Once X
* has been fixed (and the fix spread enough), we can re-enable the
- * 2 lines below and pass down a BAR value to userland. In that case
- * we'll also have to re-enable the matching code in
- * __pci_mmap_make_offset().
+ * 2 lines below and pass down a BAR value to userland.
*
* BenH.
*/
Index: linux-2.6/arch/sparc/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/sparc/kernel/pci.c
+++ linux-2.6/arch/sparc/kernel/pci.c
@@ -732,119 +732,6 @@ int pcibios_enable_device(struct pci_dev
/* Platform support for /proc/bus/pci/X/Y mmap()s. */
-/* If the user uses a host-bridge as the PCI device, he may use
- * this to perform a raw mmap() of the I/O or MEM space behind
- * that controller.
- *
- * This can be useful for execution of x86 PCI bios initialization code
- * on a PCI card, like the xfree86 int10 stuff does.
- */
-static int __pci_mmap_make_offset_bus(struct pci_dev *pdev, struct
vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- struct pci_pbm_info *pbm = pdev->dev.archdata.host_controller;
- unsigned long space_size, user_offset, user_size;
-
- if (mmap_state == pci_mmap_io) {
- space_size = resource_size(&pbm->io_space);
- } else {
- space_size = resource_size(&pbm->mem_space);
- }
-
- /* Make sure the request is in range. */
- user_offset = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
-
- if (user_offset >= space_size ||
- (user_offset + user_size) > space_size)
- return -EINVAL;
-
- if (mmap_state == pci_mmap_io) {
- vma->vm_pgoff = (pbm->io_space.start +
- user_offset) >> PAGE_SHIFT;
- } else {
- vma->vm_pgoff = (pbm->mem_space.start +
- user_offset) >> PAGE_SHIFT;
- }
-
- return 0;
-}
-
-/* Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static int __pci_mmap_make_offset(struct pci_dev *pdev,
- struct vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- unsigned long user_paddr, user_size;
- int i, err;
-
- /* First compute the physical address in vma->vm_pgoff,
- * making sure the user offset is within range in the
- * appropriate PCI space.
- */
- err = __pci_mmap_make_offset_bus(pdev, vma, mmap_state);
- if (err)
- return err;
-
- /* If this is a mapping on a host bridge, any address
- * is OK.
- */
- if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_HOST)
- return err;
-
- /* Otherwise make sure it's in the range for one of the
- * device's resources.
- */
- user_paddr = vma->vm_pgoff << PAGE_SHIFT;
- user_size = vma->vm_end - vma->vm_start;
-
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &pdev->resource[i];
- resource_size_t aligned_end;
-
- /* Active? */
- if (!rp->flags)
- continue;
-
- /* Same type? */
- if (i == PCI_ROM_RESOURCE) {
- if (mmap_state != pci_mmap_mem)
- continue;
- } else {
- if ((mmap_state == pci_mmap_io &&
- (rp->flags & IORESOURCE_IO) == 0) ||
- (mmap_state == pci_mmap_mem &&
- (rp->flags & IORESOURCE_MEM) == 0))
- continue;
- }
-
- /* Align the resource end to the next page address.
- * PAGE_SIZE intentionally added instead of (PAGE_SIZE - 1),
- * because actually we need the address of the next byte
- * after rp->end.
- */
- aligned_end = (rp->end + PAGE_SIZE) & PAGE_MASK;
-
- if ((rp->start <= user_paddr) &&
- (user_paddr + user_size) <= aligned_end)
- break;
- }
-
- if (i > PCI_ROM_RESOURCE)
- return -EINVAL;
-
- return 0;
-}
-
/* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -869,10 +756,6 @@ int pci_mmap_page_range(struct pci_dev *
{
int ret;
- ret = __pci_mmap_make_offset(dev, vma, mmap_state);
- if (ret < 0)
- return ret;
-
__pci_mmap_set_pgprot(dev, vma, mmap_state);
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
Index: linux-2.6/arch/xtensa/kernel/pci.c
===================================================================
--- linux-2.6.orig/arch/xtensa/kernel/pci.c
+++ linux-2.6/arch/xtensa/kernel/pci.c
@@ -272,68 +272,6 @@ pci_controller_num(struct pci_dev *dev)
*/
/*
- * Adjust vm_pgoff of VMA such that it is the physical page offset
- * corresponding to the 32-bit pci bus offset for DEV requested by the user.
- *
- * Basically, the user finds the base address for his device which he wishes
- * to mmap. They read the 32-bit value from the config space base register,
- * add whatever PAGE_SIZE multiple offset they wish, and feed this into the
- * offset parameter of mmap on /proc/bus/pci/XXX for that device.
- *
- * Returns negative error code on failure, zero on success.
- */
-static __inline__ int
-__pci_mmap_make_offset(struct pci_dev *dev, struct vm_area_struct *vma,
- enum pci_mmap_state mmap_state)
-{
- struct pci_controller *pci_ctrl = (struct pci_controller*) dev->sysdata;
- unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
- unsigned long io_offset = 0;
- int i, res_bit;
-
- if (pci_ctrl == 0)
- return -EINVAL; /* should never happen */
-
- /* If memory, add on the PCI bridge address offset */
- if (mmap_state == pci_mmap_mem) {
- res_bit = IORESOURCE_MEM;
- } else {
- io_offset = (unsigned long)pci_ctrl->io_space.base;
- offset += io_offset;
- res_bit = IORESOURCE_IO;
- }
-
- /*
- * Check that the offset requested corresponds to one of the
- * resources of the device.
- */
- for (i = 0; i <= PCI_ROM_RESOURCE; i++) {
- struct resource *rp = &dev->resource[i];
- int flags = rp->flags;
-
- /* treat ROM as memory (should be already) */
- if (i == PCI_ROM_RESOURCE)
- flags |= IORESOURCE_MEM;
-
- /* Active and same type? */
- if ((flags & res_bit) == 0)
- continue;
-
- /* In the range of this resource? */
- if (offset < (rp->start & PAGE_MASK) || offset > rp->end)
- continue;
-
- /* found it! construct the final physical address */
- if (mmap_state == pci_mmap_io)
- offset += pci_ctrl->io_space.start - io_offset;
- vma->vm_pgoff = offset >> PAGE_SHIFT;
- return 0;
- }
-
- return -EINVAL;
-}
-
-/*
* Set vm_page_prot of VMA, as appropriate for this architecture, for a pci
* device mapping.
*/
@@ -367,11 +305,18 @@ int pci_mmap_page_range(struct pci_dev *
enum pci_mmap_state mmap_state,
int write_combine)
{
+ unsigned long offset = vma->vm_pgoff << PAGE_SHIFT;
int ret;
- ret = __pci_mmap_make_offset(dev, vma, mmap_state);
- if (ret < 0)
- return ret;
+ if (mmap_state == pci_mmap_io) {
+ struct pci_controller *pci_ctrl =
+ (struct pci_controller *)dev->sysdata;
+
+ /* pci_ctrl should never be NULL */
+ offset += pci_ctrl->io_space.start - pci_ctrl->io_space.base;
+ }
+
+ vma->vm_pgoff = offset >> PAGE_SHIFT;
__pci_mmap_set_pgprot(dev, vma, mmap_state, write_combine);
Index: linux-2.6/drivers/pci/pci-sysfs.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-sysfs.c
+++ linux-2.6/drivers/pci/pci-sysfs.c
@@ -967,12 +967,23 @@ void pci_remove_legacy_files(struct pci_
#ifdef HAVE_PCI_MMAP
int pci_mmap_fits(struct pci_dev *pdev, int resno, struct vm_area_struct *vma,
+ enum pci_mmap_state mmap_type,
enum pci_mmap_api mmap_api)
{
unsigned long nr, start, size, pci_start;
+ int flags;
if (pci_resource_len(pdev, resno) == 0)
return 0;
+
+ if (mmap_type == pci_mmap_mem)
+ flags = IORESOURCE_MEM;
+ else
+ flags = IORESOURCE_IO;
+
+ if (!(pci_resource_flags(pdev, resno) & flags))
+ return 0;
+
nr = vma_pages(vma);
start = vma->vm_pgoff;
size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
@@ -999,7 +1010,6 @@ static int pci_mmap_resource(struct kobj
struct pci_dev *pdev = to_pci_dev(kobj_to_dev(kobj));
struct resource *res = attr->private;
enum pci_mmap_state mmap_type;
- resource_size_t start, end;
int i;
for (i = 0; i < PCI_ROM_RESOURCE; i++)
@@ -1011,7 +1021,8 @@ static int pci_mmap_resource(struct kobj
if (res->flags & IORESOURCE_MEM && iomem_is_exclusive(res->start))
return -EINVAL;
- if (!pci_mmap_fits(pdev, i, vma, PCI_MMAP_SYSFS)) {
+ mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
+ if (!pci_mmap_fits(pdev, i, vma, mmap_type, PCI_MMAP_SYSFS)) {
WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page
0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n",
current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff,
pci_name(pdev), i,
@@ -1020,13 +1031,7 @@ static int pci_mmap_resource(struct kobj
return -EINVAL;
}
- /* 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, i, res, &start, &end);
- vma->vm_pgoff += start >> PAGE_SHIFT;
- mmap_type = res->flags & IORESOURCE_MEM ? pci_mmap_mem : pci_mmap_io;
+ vma->vm_pgoff += res->start >> PAGE_SHIFT;
return pci_mmap_page_range(pdev, res, vma, mmap_type, write_combine);
}
Index: linux-2.6/drivers/pci/proc.c
===================================================================
--- linux-2.6.orig/drivers/pci/proc.c
+++ linux-2.6/drivers/pci/proc.c
@@ -227,26 +227,65 @@ static long proc_bus_pci_ioctl(struct fi
}
#ifdef HAVE_PCI_MMAP
+
+static int pci_user_to_resource(struct pci_dev *dev, resource_size_t *offset,
+ resource_size_t size, int flags)
+{
+ int i;
+
+ for (i = 0; i < PCI_ROM_RESOURCE; i++) {
+ resource_size_t start, end;
+ struct resource *res = &dev->resource[i];
+
+ if (!(res->flags & flags))
+ continue;
+
+ if (pci_resource_len(dev, i) == 0)
+ continue;
+
+ pci_resource_to_user(dev, i, res, &start, &end);
+ if (start <= *offset && (*offset + size - 1) <= end) {
+ *offset = res->start + (*offset - start);
+ return i;
+ }
+ }
+
+ return -ENODEV;
+}
+
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;
+ enum pci_mmap_state mmap_type = fpriv->mmap_state;
+ resource_size_t offset, size;
+ int i, ret, flags;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- /* Make sure the caller is mapping a real resource for this device */
- for (i = 0; i < PCI_ROM_RESOURCE; i++) {
- if (pci_mmap_fits(dev, i, vma, PCI_MMAP_PROCFS))
- break;
- }
-
- if (i >= PCI_ROM_RESOURCE)
+ offset = vma->vm_pgoff << PAGE_SHIFT;
+ size = vma->vm_end - vma->vm_start;
+ if (mmap_type == pci_mmap_mem)
+ flags = IORESOURCE_MEM;
+ else
+ flags = IORESOURCE_IO;
+ i = pci_user_to_resource(dev, &offset, size, flags);
+ if (i < 0)
return -ENODEV;
+ vma->vm_pgoff = offset >> PAGE_SHIFT;
+ if (!pci_mmap_fits(dev, i, vma, mmap_type, PCI_MMAP_PROCFS)) {
+ WARN(1, "process \"%s\" tried to map 0x%08lx bytes at page
0x%08lx on %s BAR %d (start 0x%16Lx, size 0x%16Lx)\n",
+ current->comm, vma->vm_end-vma->vm_start, vma->vm_pgoff,
+ pci_name(dev), i,
+ (u64)pci_resource_start(dev, i),
+ (u64)pci_resource_len(dev, i));
+ return -EINVAL;
+ }
+
ret = pci_mmap_page_range(dev, &dev->resource[i], vma,
- fpriv->mmap_state,
+ mmap_type,
fpriv->write_combine);
if (ret < 0)
return ret;
[toc] | [prev] | [next] | [standalone]
| From | Benjamin Herrenschmidt <benh@kernel.crashing.org> |
|---|---|
| Date | 2016-05-06 00:10 +0200 |
| Message-ID | <rvzD4-9z-17@gated-at.bofh.it> |
| In reply to | #1395202 |
On Thu, 2016-05-05 at 08:53 -0700, Yinghai Lu wrote:
> For powerpc io port, we still need extra offset from resource address
> to final address.
>
> resource_size_t offset =
> ((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
>
> + if (mmap_state == pci_mmap_io) {
> + struct pci_controller *hose = pci_bus_to_host(dev->bus);
> +
> + /* hose should never be NULL */
> + offset += hose->io_base_phys -
> + ((unsigned long)hose->io_base_virt - _IO_BASE);
> + }
>
> vma->vm_pgoff = offset >> PAGE_SHIFT;
>
> but sparc does not need that trick.
I'm not sure how sparc handles IO space but on powerpc, the IO resource
is not a physical address, it's a virtual address (coming from
ioremap).
Cheers,
Ben.
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-06 03:00 +0200 |
| Message-ID | <rvChA-2DB-7@gated-at.bofh.it> |
| In reply to | #1395402 |
On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Thu, 2016-05-05 at 08:53 -0700, Yinghai Lu wrote:
>> For powerpc io port, we still need extra offset from resource address
>> to final address.
>>
>> resource_size_t offset =
>> ((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
>>
>> + if (mmap_state == pci_mmap_io) {
>> + struct pci_controller *hose = pci_bus_to_host(dev->bus);
>> +
>> + /* hose should never be NULL */
>> + offset += hose->io_base_phys -
>> + ((unsigned long)hose->io_base_virt - _IO_BASE);
>> + }
>>
>> vma->vm_pgoff = offset >> PAGE_SHIFT;
>>
>> but sparc does not need that trick.
>
> I'm not sure how sparc handles IO space but on powerpc, the IO resource
> is not a physical address, it's a virtual address (coming from
> ioremap).
That is interesting. Any reason for that ?
why just cpu_addr in resource directly ?
Thanks
Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-06 06:20 +0200 |
| Message-ID | <rvFp7-6uZ-1@gated-at.bofh.it> |
| In reply to | #1395561 |
On Thu, May 5, 2016 at 5:56 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
>> On Thu, 2016-05-05 at 08:53 -0700, Yinghai Lu wrote:
>>> For powerpc io port, we still need extra offset from resource address
>>> to final address.
>>>
>>> resource_size_t offset =
>>> ((resource_size_t)vma->vm_pgoff) << PAGE_SHIFT;
>>>
>>> + if (mmap_state == pci_mmap_io) {
>>> + struct pci_controller *hose = pci_bus_to_host(dev->bus);
>>> +
>>> + /* hose should never be NULL */
>>> + offset += hose->io_base_phys -
>>> + ((unsigned long)hose->io_base_virt - _IO_BASE);
>>> + }
>>>
>>> vma->vm_pgoff = offset >> PAGE_SHIFT;
>>>
>>> but sparc does not need that trick.
>>
>> I'm not sure how sparc handles IO space but on powerpc, the IO resource
>> is not a physical address, it's a virtual address (coming from
>> ioremap).
>
> That is interesting. Any reason for that ?
>
> why just cpu_addr in resource directly ?
Never mind, I figured it out. sparc64 could use cpu_addr to access
io_port directly.
powerpc64 need to ioremap cpu_addr to virt then use that ioport.
so ioremap early and use virt address as resource value.
otherwise every outb in powerpc64 will need iormap and access then unmap.
Thanks
Yinghai
[toc] | [prev] | [next] | [standalone]
| From | Bjorn Helgaas <helgaas@kernel.org> |
|---|---|
| Date | 2016-05-06 20:30 +0200 |
| Message-ID | <rvSFH-1KJ-1@gated-at.bofh.it> |
| In reply to | #1395202 |
On Thu, May 05, 2016 at 08:53:14AM -0700, Yinghai Lu wrote:
> On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> >> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >>> My goal is to make pci_mmap_resource() and proc_bus_pci_mmap() look
> >>> very similar, e.g.,
> >>>
> >>> /* locate resource */
> >>> pci_user_to_resource() # only in proc_bus_pci_mmap()
> >>> if (!pci_mmap_fits()) {
> >>> WARN(...);
> >>> return -EINVAL;
> >>> }
> >>> pci_mmap_page_range();
> >
>
> v3, that have more change to pass *res to make powerpc prot setting simple.
This looks corrupted. On v4.6-rc2:
$ stg import -M m/yh3
Checking for changes in the working directory ... done
Importing patch "re-patch-v11-04-60-sparc-pci" ... fatal: corrupt patch at line 266
stg import: Diff does not apply cleanly
> ...
> Subject: [RFC PATCH v3 2/2] PCI: Let pci_mmap_page_range() take resource addr
>
> In 8c05cd08a7 ("PCI: fix offset check for sysfs mmapped files"), try
> to check exposed value with resource start/end in proc mmap path.
> | start = vma->vm_pgoff;
> | size = ((pci_resource_len(pdev, resno) - 1) >> PAGE_SHIFT) + 1;
> | pci_start = (mmap_api == PCI_MMAP_PROCFS) ?
> | pci_resource_start(pdev, resno) >> PAGE_SHIFT : 0;
> | if (start >= pci_start && start < pci_start + size &&
> | start + nr <= pci_start + size)
>
> That would break sparc that exposed value is still BAR value.
>
> In the patch:
> 1. in proc path: proc_bus_pci_mmap, try convert back to resource
> before calling pci_mmap_page_range
> 2. in sysfs path: pci_mmap_resource will just offset with resource start.
> 3. all pci_mmap_page_range will all have vma->vm_pgoff with in resource
> range instead of BAR value.
> 4. remove __pci_mmap_make_offset, as the checking is done
> in pci_mmap_fits().
This looks like it could possibly be split into several patches. I
think it's too big to apply as-is.
I'm not sure what bug this is fixing or what improvement it's making.
[toc] | [prev] | [next] | [standalone]
| From | Yinghai Lu <yinghai@kernel.org> |
|---|---|
| Date | 2016-05-10 08:20 +0200 |
| Message-ID | <rx9br-51B-1@gated-at.bofh.it> |
| In reply to | #1396056 |
On Fri, May 6, 2016 at 11:26 AM, Bjorn Helgaas <helgaas@kernel.org> wrote: >> v3, that have more change to pass *res to make powerpc prot setting simple. > > This looks corrupted. On v4.6-rc2: > > $ stg import -M m/yh3 > Checking for changes in the working directory ... done > Importing patch "re-patch-v11-04-60-sparc-pci" ... fatal: corrupt patch at line 266 > stg import: Diff does not apply cleanly Just resent them in plain/text mail. Please have a look. > ... > > This looks like it could possibly be split into several patches. I > think it's too big to apply as-is. Not that big except it remove lots of lines. > > I'm not sure what bug this is fixing or what improvement it's making. fix sparc64 proc mmap path, as it can not pass checking in pci_mmap_fits() with comparing BAR value and resource adder without offset. also it will make sparc/PCI: Use correct offset for bus address to resource and other one much simple. Thanks Yinghai
[toc] | [prev] | [next] | [standalone]
| From | David Miller <davem@davemloft.net> |
|---|---|
| Date | 2016-05-04 06:20 +0200 |
| Subject | Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource |
| Message-ID | <ruWs1-4YW-1@gated-at.bofh.it> |
| In reply to | #1393874 |
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: Wed, 04 May 2016 10:37:40 +1000 > On Tue, 2016-05-03 at 15:52 -0700, Yinghai Lu wrote: >> BenH and DavidM, >> Are you ok to let /proc/bus/pci/devices to expose resource value >> instead of >> BAR value? >> powerpc already expose MMIO as resource value, but still keep IO as >> BAR value? >> >> Or can we just dump /proc/bus/pci support from now? > > The problem tends to be old Xserver expectations... > > That stuff has been a can of worms over the years and we did things in > the kernel to work around X limitations. I'm not that keen on touching > /proc at all in that regard. Leave it there do what it does today, it's > a user visible ABI, don't change it. I agree with Ben, whatever procfs is exporting in the past is what the Xserver and other things expect on sparc64 and therefore is a user facing ABI that can't be changed.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web