[PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Michael S. Tsirkin]

Greg, Martin, does the following make sense?
If it does, should other architectures be updated as well?

---

Convert i386/pci to use io_remap_pfn_range instead of remap_pfn_range.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

Index: linux-2.6.12.2/arch/i386/pci/i386.c
===================================================================
--- linux-2.6.12.2.orig/arch/i386/pci/i386.c
+++ linux-2.6.12.2/arch/i386/pci/i386.c
@@ -295,9 +295,9 @@ int pci_mmap_page_range(struct pci_dev *
 	/* Write-combine setting is ignored, it is changed via the mtrr
 	 * interfaces on this platform.
 	 */
-	if (remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
-			     vma->vm_end - vma->vm_start,
-			     vma->vm_page_prot))
+	if (io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot))
 		return -EAGAIN;
 
 	return 0;


-- 
MST

Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Greg KH]

On Tue, Jul 26, 2005 at 01:32:00AM +0300, Michael S. Tsirkin wrote:
> Greg, Martin, does the following make sense?
> If it does, should other architectures be updated as well?
> 
> ---
> 
> Convert i386/pci to use io_remap_pfn_range instead of remap_pfn_range.
> 
> Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>
> 
> Index: linux-2.6.12.2/arch/i386/pci/i386.c
> ===================================================================
> --- linux-2.6.12.2.orig/arch/i386/pci/i386.c
> +++ linux-2.6.12.2/arch/i386/pci/i386.c
> @@ -295,9 +295,9 @@ int pci_mmap_page_range(struct pci_dev *
>  	/* Write-combine setting is ignored, it is changed via the mtrr
>  	 * interfaces on this platform.
>  	 */
> -	if (remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> -			     vma->vm_end - vma->vm_start,
> -			     vma->vm_page_prot))
> +	if (io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> +			       vma->vm_end - vma->vm_start,
> +			       vma->vm_page_prot))

Hm, you do realize that io_remap_pfn_range() is the same thing as
remap_pfn_range() on i386, right?

So, why would this patch change anything?

thanks,

greg k-h

Re: [openib-general] Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Roland Dreier]

    Greg> Hm, you do realize that io_remap_pfn_range() is the same
    Greg> thing as remap_pfn_range() on i386, right?

    Greg> So, why would this patch change anything?

It's not the same thing under Xen.  I think this patch fixes userspace
access to PCI memory for XenLinux.

 - R.

Re: [openib-general] Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Greg KH]

On Wed, Jul 27, 2005 at 09:30:17PM -0700, Roland Dreier wrote:
>     Greg> Hm, you do realize that io_remap_pfn_range() is the same
>     Greg> thing as remap_pfn_range() on i386, right?
> 
>     Greg> So, why would this patch change anything?
> 
> It's not the same thing under Xen.  I think this patch fixes userspace
> access to PCI memory for XenLinux.

But Xen is a separate arch, and hence, will get different pci arch
specific functions, right?

In short, what is this patch trying to fix?  What is the problem anyone
is seeing with the existing code?

thanks,

greg k-h

Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Michael S. Tsirkin]

Quoting r. Greg KH <gregkh@suse.de>:
> Subject: Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range
> 
> On Tue, Jul 26, 2005 at 01:32:00AM +0300, Michael S. Tsirkin wrote:
> > Greg, Martin, does the following make sense?
> > If it does, should other architectures be updated as well?
> > 
> Hm, you do realize that io_remap_pfn_range() is the same thing as
> remap_pfn_range() on i386, right?
> 
> So, why would this patch change anything?
> 
> thanks,
> 
> greg k-h
> 

It doesnt change any behaviour.
Its a style issue I'm trying to fix: even in arch specific code, isnt it better
to use a generic io_remap_pfn_range to map PCI memory?

-- 
MST

Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Greg KH]

On Thu, Jul 28, 2005 at 09:48:59AM +0300, Michael S. Tsirkin wrote:
> Quoting r. Greg KH <gregkh@suse.de>:
> > Subject: Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range
> > 
> > On Tue, Jul 26, 2005 at 01:32:00AM +0300, Michael S. Tsirkin wrote:
> > > Greg, Martin, does the following make sense?
> > > If it does, should other architectures be updated as well?
> > > 
> > Hm, you do realize that io_remap_pfn_range() is the same thing as
> > remap_pfn_range() on i386, right?
> > 
> > So, why would this patch change anything?
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> It doesnt change any behaviour.
> Its a style issue I'm trying to fix: even in arch specific code, isnt it better
> to use a generic io_remap_pfn_range to map PCI memory?

As I said above, it's the same thing on i386.
But, if you are getting picky, you should fix all of the other arches
too, right?

Anyway, why change this, I don't really see the need.

thanks,

greg k-h

RE: [openib-general] Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Ian Pratt]

> >     Greg> Hm, you do realize that io_remap_pfn_range() is the same
> >     Greg> thing as remap_pfn_range() on i386, right?
> > 
> >     Greg> So, why would this patch change anything?
> > 
> > It's not the same thing under Xen.  I think this patch 
> fixes userspace 
> > access to PCI memory for XenLinux.
> 
> But Xen is a separate arch, and hence, will get different pci 
> arch specific functions, right?
> 
> In short, what is this patch trying to fix?  What is the 
> problem anyone is seeing with the existing code?

As I understand it, remap_pfn_range should be used for mapping pages
that are backed by memory, and io_remap_pfn_range should be used for
mapping MMIO regions.
There's a distinciton between the two for architectures like Sparc and
xen/x86. 

For example, drivers/char/mem.c uses io_remap_pfn_range for mmap'ing
/dev/mem

Ian

Re: [openib-general] Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Greg KH]

On Thu, Jul 28, 2005 at 12:57:51PM +0100, Ian Pratt wrote:
> > >     Greg> Hm, you do realize that io_remap_pfn_range() is the same
> > >     Greg> thing as remap_pfn_range() on i386, right?
> > > 
> > >     Greg> So, why would this patch change anything?
> > > 
> > > It's not the same thing under Xen.  I think this patch 
> > fixes userspace 
> > > access to PCI memory for XenLinux.
> > 
> > But Xen is a separate arch, and hence, will get different pci 
> > arch specific functions, right?
> > 
> > In short, what is this patch trying to fix?  What is the 
> > problem anyone is seeing with the existing code?
> 
> As I understand it, remap_pfn_range should be used for mapping pages
> that are backed by memory, and io_remap_pfn_range should be used for
> mapping MMIO regions.
> There's a distinciton between the two for architectures like Sparc and
> xen/x86. 

I agree, but not for i386, which is what the patch was trying to change.

If this is a fix for xen, fine, then say so in the changelog information
for the patch, as it is, no such information was given.

thanks,

greg k-h

Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range

[Michael S. Tsirkin]

Quoting r. Greg KH <gregkh@suse.de>:
> Subject: Re: [openib-general] Re: [PATCH] arch/xx/pci: remap_pfn_range -> io_remap_pfn_range
> If this is a fix for xen, fine, then say so in the changelog information
> for the patch, as it is, no such information was given.
> 
> thanks,
> 
> greg k-h
> 

Like this?

---

Convert i386/pci to use io_remap_pfn_range instead of remap_pfn_range.
This is good for Xen which reuses i386/pci/i386.c for domain 0 code.

Signed-off-by: Michael S. Tsirkin <mst@mellanox.co.il>

Index: linux-2.6.12.2/arch/i386/pci/i386.c
===================================================================
--- linux-2.6.12.2.orig/arch/i386/pci/i386.c
+++ linux-2.6.12.2/arch/i386/pci/i386.c
@@ -295,9 +295,9 @@ int pci_mmap_page_range(struct pci_dev *
 	/* Write-combine setting is ignored, it is changed via the mtrr
 	 * interfaces on this platform.
 	 */
-	if (remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
-			     vma->vm_end - vma->vm_start,
-			     vma->vm_page_prot))
+	if (io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+			       vma->vm_end - vma->vm_start,
+			       vma->vm_page_prot))
 		return -EAGAIN;
 
 	return 0;

-- 
MST