Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, May 6, 2016 at 11:26 AM, Bjorn Helgaas  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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, May 6, 2016 at 11:26 AM, Bjorn Helgaas  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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Thu, May 05, 2016 at 08:53:14AM -0700, Yinghai Lu wrote:
> On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu  wrote:
> > On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
> >> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Thu, May 05, 2016 at 08:53:14AM -0700, Yinghai Lu wrote:
> On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu  wrote:
> > On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
> >> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, May 5, 2016 at 5:56 PM, Yinghai Lu  wrote:
> On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
>  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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, May 5, 2016 at 5:56 PM, Yinghai Lu  wrote:
> On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
>  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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
 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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, May 5, 2016 at 3:02 PM, Benjamin Herrenschmidt
 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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Benjamin Herrenschmidt]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Benjamin Herrenschmidt]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu  wrote:
> On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
>> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 


---
 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 = >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 > 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 5:25 PM, Yinghai Lu  wrote:
> On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
>> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 


---
 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 = >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 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 

---
 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 = >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 & 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 11:46 AM, Yinghai Lu  wrote:
> On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 

---
 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 = >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 & 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 = >resource[i];
>> +if (!res->flags)
>> +continue;
>> +
>> +pci_resource_to_user(dev, i, res, , );
>> +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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Wed, May 4, 2016 at 8:17 AM, Bjorn Helgaas  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 = >resource[i];
>> +if (!res->flags)
>> +continue;
>> +
>> +pci_resource_to_user(dev, i, res, , );
>> +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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Tue, May 03, 2016 at 10:52:33PM -0700, Yinghai Lu wrote:
> On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu  wrote:
> > On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 
> 
> ---
>  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)
> -

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Tue, May 03, 2016 at 10:52:33PM -0700, Yinghai Lu wrote:
> On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu  wrote:
> > On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 
> 
> ---
>  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;
> +

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu  wrote:
> On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 

---
 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 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Tue, May 3, 2016 at 10:08 PM, Yinghai Lu  wrote:
> On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 

---
 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 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 

---
 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, >resource[resno],
+ _start, _end);
+pci_start >>= PAGE_SHIFT;
+}
 if (start >= pci_start && start < pci_start + size &&
 start + nr <= pci_start + size)
 return 1;

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Tue, May 3, 2016 at 6:25 PM, Bjorn Helgaas  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 

---
 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, >resource[resno],
+ _start, _end);
+pci_start >>= PAGE_SHIFT;
+}
 if (start >= pci_start && start < pci_start + size &&
 start + nr <= pci_start + size)
 return 1;

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[David Miller]

From: Benjamin Herrenschmidt 
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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[David Miller]

From: Benjamin Herrenschmidt 
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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Benjamin Herrenschmidt]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Benjamin Herrenschmidt]

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.

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, Apr 29, 2016 at 12:19 AM, Yinghai Lu  wrote:
> On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas  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 

---
 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_tpci_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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, Apr 29, 2016 at 12:19 AM, Yinghai Lu  wrote:
> On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas  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 

---
 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_tpci_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;
-}

 /*
  * 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas  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  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/pci:00/: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, , );
>> 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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Thu, Apr 28, 2016 at 6:56 AM, Bjorn Helgaas  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  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/pci:00/: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, , );
>> 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

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Wed, Apr 27, 2016 at 09:55:45PM -0700, Yinghai Lu wrote:
> On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas  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/pci:00/: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, , );
> 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 = >resource[resno];
> +
> +   pci_resource_to_user(pdev, resno, res, _start, _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 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

On Wed, Apr 27, 2016 at 09:55:45PM -0700, Yinghai Lu wrote:
> On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas  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/pci:00/: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, , );
> 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 = >resource[resno];
> +
> +   pci_resource_to_user(pdev, resno, res, _start, _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 + 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas  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/pci:00/: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, , );
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.

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 = >resource[resno];
+
+   pci_resource_to_user(pdev, resno, res, _start, _end);
+   pci_start >>= PAGE_SHIFT;
+   }
if (start >= pci_start && start < pci_start + size &&
start + nr <= pci_start + size)
return 1;

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Yinghai Lu]

On Fri, Apr 22, 2016 at 1:49 PM, Bjorn Helgaas  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/pci:00/: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, , );
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.

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 = >resource[resno];
+
+   pci_resource_to_user(pdev, resno, res, _start, _end);
+   pci_start >>= PAGE_SHIFT;
+   }
if (start >= pci_start && start < pci_start + size &&
start + nr <= pci_start + size)
return 1;

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

[+cc Ben, Michael]

On Thu, Apr 07, 2016 at 05:15:17PM -0700, Yinghai Lu wrote:
> After we added 64bit mmio parsing, we got some "no compatible bridge window"
> warning on anther new model that support 64bit resource.
> 
> It turns out that we can not use mem_space.start as 64bit mem space
> offset, aka there is mem_space.start != offset.
> 
> Use child_phys_addr to calculate exact offset and record offset in
> pbm.
> 
> After patch we get correct offset.
> 
> /pci@305: PCI IO [io  0x2007e-0x2007e0fff] offset 2007e
> /pci@305: PCI MEM [mem 0x20010-0x27eff] offset 2
> /pci@305: PCI MEM64 [mem 0x20001-0x2000d] offset 2
> ...
> pci_sun4v f02ae7f8: PCI host bridge to bus :00
> pci_bus :00: root bus resource [io  0x2007e-0x2007e0fff] (bus 
> address [0x-0xfff])
> pci_bus :00: root bus resource [mem 0x20010-0x27eff] (bus 
> address [0x0010-0x7eff])
> pci_bus :00: root bus resource [mem 0x20001-0x2000d] (bus 
> address [0x1-0xd])
> ...

> @@ -733,30 +733,32 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>  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(>io_space);
> - } else {
> - space_size = resource_size(>mem_space);
> - }
> + unsigned long user_offset, user_size;
> + struct resource res, *root_bus_res;
> + struct pci_bus_region region;
> + struct pci_bus *bus;
>  
>   /* 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)
> + region.start = user_offset;
> + region.end = user_offset + user_size - 1;
> + memset(, 0, sizeof(res));
> + if (mmap_state == pci_mmap_io)
> + res.flags = IORESOURCE_IO;
> + else
> + res.flags = IORESOURCE_MEM;
> +
> + pcibios_bus_to_resource(pdev->bus, , );
> + bus = pdev->bus;
> + while (bus->parent)
> + bus = bus->parent;
> + root_bus_res = pci_find_bus_resource(bus, );
> + if (!root_bus_res)
>   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;
> - }
> + vma->vm_pgoff = res.start >> PAGE_SHIFT;
>  
>   return 0;
>  }

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/pci:00/: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.

In any case, pci_mmap_page_range() on sparc converts that
vma->vm_pgoff back to a CPU physical address, so there's an awful lot
of work going on in the pci_mmap_resource() path to convert the mmap
offset to a "user" address and then convert it back again.

There's also quite a bit of validation done in the arch code (in
__pci_mmap_make_offset() and __pci_mmap_make_offset_bus()) that looks
partly redundant with pci_mmap_fits() and not necessarily
arch-specific.

As far as I can see, pci_mmap_resource() doesn't need to have any
connection at all with pci_resource_to_user().  All it needs is the
pdev->resource[] (which has the CPU physical address of the BAR) and
vma->vm_pgoff 

Re: [PATCH v11 04/60] sparc/PCI: Use correct offset for bus address to resource

[Bjorn Helgaas]

[+cc Ben, Michael]

On Thu, Apr 07, 2016 at 05:15:17PM -0700, Yinghai Lu wrote:
> After we added 64bit mmio parsing, we got some "no compatible bridge window"
> warning on anther new model that support 64bit resource.
> 
> It turns out that we can not use mem_space.start as 64bit mem space
> offset, aka there is mem_space.start != offset.
> 
> Use child_phys_addr to calculate exact offset and record offset in
> pbm.
> 
> After patch we get correct offset.
> 
> /pci@305: PCI IO [io  0x2007e-0x2007e0fff] offset 2007e
> /pci@305: PCI MEM [mem 0x20010-0x27eff] offset 2
> /pci@305: PCI MEM64 [mem 0x20001-0x2000d] offset 2
> ...
> pci_sun4v f02ae7f8: PCI host bridge to bus :00
> pci_bus :00: root bus resource [io  0x2007e-0x2007e0fff] (bus 
> address [0x-0xfff])
> pci_bus :00: root bus resource [mem 0x20010-0x27eff] (bus 
> address [0x0010-0x7eff])
> pci_bus :00: root bus resource [mem 0x20001-0x2000d] (bus 
> address [0x1-0xd])
> ...

> @@ -733,30 +733,32 @@ int pcibios_enable_device(struct pci_dev *dev, int mask)
>  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(>io_space);
> - } else {
> - space_size = resource_size(>mem_space);
> - }
> + unsigned long user_offset, user_size;
> + struct resource res, *root_bus_res;
> + struct pci_bus_region region;
> + struct pci_bus *bus;
>  
>   /* 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)
> + region.start = user_offset;
> + region.end = user_offset + user_size - 1;
> + memset(, 0, sizeof(res));
> + if (mmap_state == pci_mmap_io)
> + res.flags = IORESOURCE_IO;
> + else
> + res.flags = IORESOURCE_MEM;
> +
> + pcibios_bus_to_resource(pdev->bus, , );
> + bus = pdev->bus;
> + while (bus->parent)
> + bus = bus->parent;
> + root_bus_res = pci_find_bus_resource(bus, );
> + if (!root_bus_res)
>   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;
> - }
> + vma->vm_pgoff = res.start >> PAGE_SHIFT;
>  
>   return 0;
>  }

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/pci:00/: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.

In any case, pci_mmap_page_range() on sparc converts that
vma->vm_pgoff back to a CPU physical address, so there's an awful lot
of work going on in the pci_mmap_resource() path to convert the mmap
offset to a "user" address and then convert it back again.

There's also quite a bit of validation done in the arch code (in
__pci_mmap_make_offset() and __pci_mmap_make_offset_bus()) that looks
partly redundant with pci_mmap_fits() and not necessarily
arch-specific.

As far as I can see, pci_mmap_resource() doesn't need to have any
connection at all with pci_resource_to_user().  All it needs is the
pdev->resource[] (which has the CPU physical address of the BAR) and
vma->vm_pgoff