[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

[Randy.Dunlap]

On Thu, 28 Jul 2005 12:57:51 +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. 
> 
> For example, drivers/char/mem.c uses io_remap_pfn_range for mmap'ing
> /dev/mem

That is my (limited) understanding also, but when I built
io_remap_pfn_range(), I didn't search all callers of
remap_pfn_range() to see which ones that I could fix (or break)
by changing them.  Mostly due to the possible breakage part.

---
~Randy

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

[Ian Pratt]

 
> > For example, drivers/char/mem.c uses io_remap_pfn_range for 
> mmap'ing 
> > /dev/mem
> 
> That is my (limited) understanding also, but when I built 
> io_remap_pfn_range(), I didn't search all callers of
> remap_pfn_range() to see which ones that I could fix (or 
> break) by changing them.  Mostly due to the possible breakage part.

Yep, its not entirely trivial to determine the intended usage. In
particular, some of the sound drivers require both versions in the same
file. 

It's probably best to rely on Xen/Sparc users to find them on a case by
case basis and ask for them to be fixed (unless someone's feeling
brave...)

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