[PATCH 1/3] arm64: enable pci resource mapping using sysfs

[PATCH 1/3] arm64: enable pci resource mapping using sysfs

[David Woodhouse]

[-- Attachment #1.1: Type: text/plain, Size: 1930 bytes --]

From: Brijesh Singh <brijess@amazon.com>

To support pci resource mapping from userspace, pci_mmap_page_range
implementation must be done for that platform. This support was
broken for arm64.

This patch copies existing implementation from arm to
enable sysfs mmap.

[dwmw2: Add WC support]
Reviewed-by: Erik Quanstrom <quanstro@amazon.com>
Reviewed-by: Anthony Liguori <aliguori@amazon.com>
Signed-off-by: Brijesh Singh <brijess@amazon.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/arm64/include/asm/pci.h |  4 ++++
 arch/arm64/kernel/pci.c      | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b9a7ba9..f73734f 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -37,5 +37,9 @@ static inline int pci_proc_domain(struct pci_bus *bus)
 }
 #endif  /* CONFIG_PCI */
 
+#define HAVE_PCI_MMAP
+extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
+			       enum pci_mmap_state mmap_state, int write_combine);
+
 #endif  /* __KERNEL__ */
 #endif  /* __ASM_PCI_H */
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4f0e3eb..a9b6542 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -228,3 +228,22 @@ void pcibios_remove_bus(struct pci_bus *bus)
 }
 
 #endif
+
+int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
+			enum pci_mmap_state mmap_state, int write_combine)
+{
+	if (mmap_state == pci_mmap_io)
+		return -EINVAL;
+
+	if (write_combine)
+		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+	else
+		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+	if (remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+			     vma->vm_end - vma->vm_start,
+			     vma->vm_page_prot))
+		return -EAGAIN;
+
+	return 0;
+}
-- 
dwmw2

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] arm64: enable pci resource mapping using sysfs

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]

From: Brijesh Singh <brijess@amazon.com>

To support pci resource mapping from userspace, pci_mmap_page_range
implementation must be done for that platform. This support was
broken for arm64.

This patch copies existing implementation from arm to
enable sysfs mmap.

[dwmw2: Add WC support]
Reviewed-by: Erik Quanstrom <quanstro@amazon.com>
Reviewed-by: Anthony Liguori <aliguori@amazon.com>
Signed-off-by: Brijesh Singh <brijess@amazon.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/arm64/include/asm/pci.h |  4 ++++
 arch/arm64/kernel/pci.c      | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/pci.h b/arch/arm64/include/asm/pci.h
index b9a7ba9..f73734f 100644
--- a/arch/arm64/include/asm/pci.h
+++ b/arch/arm64/include/asm/pci.h
@@ -37,5 +37,9 @@ static inline int pci_proc_domain(struct pci_bus *bus)
 }
 #endif  /* CONFIG_PCI */
 
+#define HAVE_PCI_MMAP
+extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
+			       enum pci_mmap_state mmap_state, int write_combine);
+
 #endif  /* __KERNEL__ */
 #endif  /* __ASM_PCI_H */
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index 4f0e3eb..a9b6542 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -228,3 +228,22 @@ void pcibios_remove_bus(struct pci_bus *bus)
 }
 
 #endif
+
+int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
+			enum pci_mmap_state mmap_state, int write_combine)
+{
+	if (mmap_state == pci_mmap_io)
+		return -EINVAL;
+
+	if (write_combine)
+		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
+	else
+		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+
+	if (remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+			     vma->vm_end - vma->vm_start,
+			     vma->vm_page_prot))
+		return -EAGAIN;
+
+	return 0;
+}
-- 
dwmw2

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[Mark Rutland]

Hi,

On Wed, Mar 15, 2017 at 05:17:46PM +0000, David Woodhouse wrote:
> From: Brijesh Singh <brijess@amazon.com>
> 
> To support pci resource mapping from userspace, pci_mmap_page_range
> implementation must be done for that platform. This support was
> broken for arm64.
> 
> This patch copies existing implementation from arm to
> enable sysfs mmap.

It's not so much "broken" as "not currently supported".

[...]

> +#define HAVE_PCI_MMAP
> +extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> +			       enum pci_mmap_state mmap_state, int write_combine);
> +

Per the prior attempt at this [1], we only want to expose the sysfs
interface, and not the legacy procfs interface, and need the two
decoupled [2].

... or has something changed in the mean time, so that this only exposes
the sysfs interface?

[...]

> +int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> +			enum pci_mmap_state mmap_state, int write_combine)
> +{
> +	if (mmap_state == pci_mmap_io)
> +		return -EINVAL;
> +
> +	if (write_combine)
> +		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> +	else
> +		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

... as per Will's comment in [3], the latter of these should use
pgprot_device.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/421948.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/423083.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[Mark Rutland]

Hi,

On Wed, Mar 15, 2017 at 05:17:46PM +0000, David Woodhouse wrote:
> From: Brijesh Singh <brijess@amazon.com>
> 
> To support pci resource mapping from userspace, pci_mmap_page_range
> implementation must be done for that platform. This support was
> broken for arm64.
> 
> This patch copies existing implementation from arm to
> enable sysfs mmap.

It's not so much "broken" as "not currently supported".

[...]

> +#define HAVE_PCI_MMAP
> +extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> +			       enum pci_mmap_state mmap_state, int write_combine);
> +

Per the prior attempt at this [1], we only want to expose the sysfs
interface, and not the legacy procfs interface, and need the two
decoupled [2].

... or has something changed in the mean time, so that this only exposes
the sysfs interface?

[...]

> +int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> +			enum pci_mmap_state mmap_state, int write_combine)
> +{
> +	if (mmap_state == pci_mmap_io)
> +		return -EINVAL;
> +
> +	if (write_combine)
> +		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> +	else
> +		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);

... as per Will's comment in [3], the latter of these should use
pgprot_device.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/421948.html
[2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/423083.html
[3] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/422571.html

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1155 bytes --]

On Wed, 2017-03-15 at 17:54 +0000, Mark Rutland wrote:
> It's not so much "broken" as "not currently supported".

Yeah, I thought that when I inherited the commit, but didn't get as far
as rephrasing it. Will do so.

> [...]
> 
> > 
> > +#define HAVE_PCI_MMAP
> > +extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> > +			       enum pci_mmap_state mmap_state, int write_combine);
> > +
> Per the prior attempt at this [1], we only want to expose the sysfs
> interface, and not the legacy procfs interface, and need the two
> decoupled [2].

I do not like that idea. The procfs horridness is legacy, sure, but
it's not actually an arch-specific interface. You get to mess with
'legacy' syscalls all you like on a new architecture, but there could
exist arch-agnostic code which uses the procfs interface, surely?

> > +	if (write_combine)
> > +		vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> > +	else
> > +		vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> ... as per Will's comment in [3], the latter of these should use
> pgprot_device.

Will fix that; thanks.

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[Will Deacon]

On Wed, Mar 15, 2017 at 07:18:16PM +0000, David Woodhouse wrote:
> On Wed, 2017-03-15 at 17:54 +0000, Mark Rutland wrote:
> > It's not so much "broken" as "not currently supported".
> 
> Yeah, I thought that when I inherited the commit, but didn't get as far
> as rephrasing it. Will do so.
> 
> > [...]
> > 
> > > 
> > > +#define HAVE_PCI_MMAP
> > > +extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> > > +			       enum pci_mmap_state mmap_state, int write_combine);
> > > +
> > Per the prior attempt at this [1], we only want to expose the sysfs
> > interface, and not the legacy procfs interface, and need the two
> > decoupled [2].
> 
> I do not like that idea. The procfs horridness is legacy, sure, but
> it's not actually an arch-specific interface. You get to mess with
> 'legacy' syscalls all you like on a new architecture, but there could
> exist arch-agnostic code which uses the procfs interface, surely?

Just to be clear here: I'm not against exposing the proc interface if
something actually needs it, but all the requests we've had for this have
been concerned only with the sysfs API. So I'd rather start with just that,
instead of exposing both and have new software written to the proc interface,
which we certainly want to discourage.

Will

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[Will Deacon]

On Wed, Mar 15, 2017 at 07:18:16PM +0000, David Woodhouse wrote:
> On Wed, 2017-03-15 at 17:54 +0000, Mark Rutland wrote:
> > It's not so much "broken" as "not currently supported".
> 
> Yeah, I thought that when I inherited the commit, but didn't get as far
> as rephrasing it. Will do so.
> 
> > [...]
> > 
> > > 
> > > +#define HAVE_PCI_MMAP
> > > +extern int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
> > > +			       enum pci_mmap_state mmap_state, int write_combine);
> > > +
> > Per the prior attempt at this [1], we only want to expose the sysfs
> > interface, and not the legacy procfs interface, and need the two
> > decoupled [2].
> 
> I do not like that idea. The procfs horridness is legacy, sure, but
> it's not actually an arch-specific interface. You get to mess with
> 'legacy' syscalls all you like on a new architecture, but there could
> exist arch-agnostic code which uses the procfs interface, surely?

Just to be clear here: I'm not against exposing the proc interface if
something actually needs it, but all the requests we've had for this have
been concerned only with the sysfs API. So I'd rather start with just that,
instead of exposing both and have new software written to the proc interface,
which we certainly want to discourage.

Will

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Mon, 2017-03-20 at 10:24 +0000, Will Deacon wrote:
> 
> Just to be clear here: I'm not against exposing the proc interface if
> something actually needs it, but all the requests we've had for this have
> been concerned only with the sysfs API. So I'd rather start with just that,
> instead of exposing both and have new software written to the proc interface,
> which we certainly want to discourage.

The point is that they are tied together. An architecture provides its
pci_mmap_page_range() function and sets HAVE_PCI_MMAP, and then the
*generic* code in drivers/pci will use it from both places.

Although as noted, there is a third userspace API in drivers/vfio which
does this *without* any arch-specific code (using pgprot_noncached(),
and failing to set up the VMA correctly on HAVE_IOREMAP_PROT platforms.

I was looking at cleaning that whole mess up, but got distracted by
kdump-on-CPU-1 failures. If I'm going to blame that on firmware, I'll
get back to PCI mmap this week... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]

Re: [PATCH 1/3] arm64: enable pci resource mapping using sysfs

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Mon, 2017-03-20 at 10:24 +0000, Will Deacon wrote:
> 
> Just to be clear here: I'm not against exposing the proc interface if
> something actually needs it, but all the requests we've had for this have
> been concerned only with the sysfs API. So I'd rather start with just that,
> instead of exposing both and have new software written to the proc interface,
> which we certainly want to discourage.

The point is that they are tied together. An architecture provides its
pci_mmap_page_range() function and sets HAVE_PCI_MMAP, and then the
*generic* code in drivers/pci will use it from both places.

Although as noted, there is a third userspace API in drivers/vfio which
does this *without* any arch-specific code (using pgprot_noncached(),
and failing to set up the VMA correctly on HAVE_IOREMAP_PROT platforms.

I was looking at cleaning that whole mess up, but got distracted by
kdump-on-CPU-1 failures. If I'm going to blame that on firmware, I'll
get back to PCI mmap this week... :)

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 4938 bytes --]