2005-03-16 11:51:17

by Keir Fraser

[permalink] [raw]
Subject: [PATCH] Xen/i386 cleanups - io_remap_pfn_range

--- linux-2.6-old/drivers/char/agp/frontend.c 2005-03-16 10:30:25 +00:00
+++ linux-2.6-new/drivers/char/agp/frontend.c 2005-03-16 10:34:58 +00:00
@@ -628,7 +628,7 @@
DBG("client vm_ops=%p", kerninfo.vm_ops);
if (kerninfo.vm_ops) {
vma->vm_ops = kerninfo.vm_ops;
- } else if (remap_pfn_range(vma, vma->vm_start,
+ } else if (io_remap_pfn_range(vma, vma->vm_start,
(kerninfo.aper_base + offset) >> PAGE_SHIFT,
size, vma->vm_page_prot)) {
goto out_again;
@@ -644,7 +644,7 @@
DBG("controller vm_ops=%p", kerninfo.vm_ops);
if (kerninfo.vm_ops) {
vma->vm_ops = kerninfo.vm_ops;
- } else if (remap_pfn_range(vma, vma->vm_start,
+ } else if (io_remap_pfn_range(vma, vma->vm_start,
kerninfo.aper_base >> PAGE_SHIFT,
size, vma->vm_page_prot)) {
goto out_again;
--- linux-2.6-old/drivers/char/drm/drm_vm.c 2005-03-16 10:30:25 +00:00
+++ linux-2.6-new/drivers/char/drm/drm_vm.c 2005-03-16 10:34:58 +00:00
@@ -630,7 +630,7 @@
vma->vm_end - vma->vm_start,
vma->vm_page_prot, 0))
#else
- if (remap_pfn_range(DRM_RPR_ARG(vma) vma->vm_start,
+ if (io_remap_pfn_range(vma, vma->vm_start,
(VM_OFFSET(vma) + offset) >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
vma->vm_page_prot))
--- linux-2.6-old/drivers/char/drm/i810_dma.c 2005-03-16 10:30:25 +00:00
+++ linux-2.6-new/drivers/char/drm/i810_dma.c 2005-03-16 10:34:58 +00:00
@@ -119,7 +119,7 @@
buf_priv->currently_mapped = I810_BUF_MAPPED;
unlock_kernel();

- if (remap_pfn_range(DRM_RPR_ARG(vma) vma->vm_start,
+ if (io_remap_pfn_range(vma, vma->vm_start,
VM_OFFSET(vma) >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
vma->vm_page_prot)) return -EAGAIN;
--- linux-2.6-old/drivers/char/drm/i830_dma.c 2005-03-16 10:30:25 +00:00
+++ linux-2.6-new/drivers/char/drm/i830_dma.c 2005-03-16 10:34:58 +00:00
@@ -121,7 +121,7 @@
buf_priv->currently_mapped = I830_BUF_MAPPED;
unlock_kernel();

- if (remap_pfn_range(DRM_RPR_ARG(vma) vma->vm_start,
+ if (io_remap_pfn_range(vma, vma->vm_start,
VM_OFFSET(vma) >> PAGE_SHIFT,
vma->vm_end - vma->vm_start,
vma->vm_page_prot)) return -EAGAIN;
--- linux-2.6-old/drivers/char/hpet.c 2005-03-16 10:30:25 +00:00
+++ linux-2.6-new/drivers/char/hpet.c 2005-03-16 10:34:58 +00:00
@@ -76,6 +76,7 @@
struct hpets {
struct hpets *hp_next;
struct hpet __iomem *hp_hpet;
+ unsigned long hp_hpet_phys;
struct time_interpolator *hp_interpolator;
unsigned long hp_period;
unsigned long hp_delta;
@@ -265,7 +266,7 @@
return -EINVAL;

devp = file->private_data;
- addr = (unsigned long)devp->hd_hpet;
+ addr = devp->hd_hpets->hp_hpet_phys;

if (addr & (PAGE_SIZE - 1))
return -ENOSYS;
@@ -274,7 +275,7 @@
vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
addr = __pa(addr);

- if (remap_pfn_range(vma, vma->vm_start, addr >> PAGE_SHIFT,
+ if (io_remap_pfn_range(vma, vma->vm_start, addr >> PAGE_SHIFT,
PAGE_SIZE, vma->vm_page_prot)) {
printk(KERN_ERR "remap_pfn_range failed in hpet.c\n");
return -EAGAIN;
@@ -795,6 +796,7 @@

hpetp->hp_which = hpet_nhpet++;
hpetp->hp_hpet = hdp->hd_address;
+ hpetp->hp_hpet_phys = hdp->hd_phys_address;

hpetp->hp_ntimer = hdp->hd_nirqs;

--- linux-2.6-old/drivers/sbus/char/flash.c 2005-03-16 10:30:37 +00:00
+++ linux-2.6-new/drivers/sbus/char/flash.c 2005-03-16 10:34:58 +00:00
@@ -75,7 +75,7 @@
pgprot_val(vma->vm_page_prot) |= _PAGE_E;
vma->vm_flags |= (VM_SHM | VM_LOCKED);

- if (remap_pfn_range(vma, vma->vm_start, addr, size, vma->vm_page_prot))
+ if (io_remap_pfn_range(vma, vma->vm_start, addr, size, vma->vm_page_prot))
return -EAGAIN;

return 0;
--- linux-2.6-old/include/linux/mm.h 2005-03-16 10:31:07 +00:00
+++ linux-2.6-new/include/linux/mm.h 2005-03-16 10:34:58 +00:00
@@ -815,6 +815,10 @@
extern int check_user_page_readable(struct mm_struct *mm, unsigned long address);
int remap_pfn_range(struct vm_area_struct *, unsigned long,
unsigned long, unsigned long, pgprot_t);
+/* Allow arch override for mapping of device and I/O (non-RAM) pages. */
+#ifndef io_remap_pfn_range
+#define io_remap_pfn_range remap_pfn_range
+#endif

#ifdef CONFIG_PROC_FS
void __vm_stat_account(struct mm_struct *, unsigned long, struct file *, long);


2005-03-16 16:48:46

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [PATCH] Xen/i386 cleanups - io_remap_pfn_range

Keir Fraser wrote:
> This patch introduces a new interface function for mapping bus/device
> memory: io_remap_pfn_range. This accepts the same parameters as
> remap_pfn_range (indeed, by default it is implemented by this existing
> function) but should be used in any situation where the caller is not
> simply remapping ordinary RAM. For example, when mapping device
> registers the new function should be used.
>
> The distinction between remapping device memory and ordinary RAM is
> critical for the Xen hypervisor. This may also serve as a starting
> point for cleaning up remaining users of io_remap_page_range (in
> particular, the several sparc-specific sections in various drivers
> that use a special form of io_remap_page_range).
>
> I have audited the drivers/ and sound/ directories. Most uses of
> remap_pfn_range are okay, but there are a small handful that are
> remapping device memory (mostly AGP and DRM drivers).
>
> Of particular driver is the HPET driver, whose mmap function is broken
> even for native (non-Xen) builds. If nothing else, vmalloc_to_phys
> should be used instead of __pa to convert an ioremapped virtual
> address to a valid physical address. The fix in this patch is to
> remember the original bus address as probed at boot time and to pass
> this to io_remap_pfn_range.

I'll look over this shortly. I posted an io_remap_pfn_range() patch
to the linux-mm mailing list last week:
http://marc.theaimsgroup.com/?l=linux-ultrasparc&m=111049550001120&w=2

One of the things that it needs to take into account is that
io_remap_page_range() on sparc/sparc64 has 6 parameters instead of 5.
My patch tries to remedy that...

--
~Randy

2005-03-17 04:41:37

by Randy.Dunlap

[permalink] [raw]
Subject: Re: [PATCH] Xen/i386 cleanups - io_remap_pfn_range

Keir Fraser wrote:
> This patch introduces a new interface function for mapping bus/device
> memory: io_remap_pfn_range. This accepts the same parameters as
> remap_pfn_range (indeed, by default it is implemented by this existing
> function) but should be used in any situation where the caller is not
> simply remapping ordinary RAM. For example, when mapping device
> registers the new function should be used.
>
> The distinction between remapping device memory and ordinary RAM is
> critical for the Xen hypervisor. This may also serve as a starting
> point for cleaning up remaining users of io_remap_page_range (in
> particular, the several sparc-specific sections in various drivers
> that use a special form of io_remap_page_range).
>
> I have audited the drivers/ and sound/ directories. Most uses of
> remap_pfn_range are okay, but there are a small handful that are
> remapping device memory (mostly AGP and DRM drivers).
>
> Of particular driver is the HPET driver, whose mmap function is broken
> even for native (non-Xen) builds. If nothing else, vmalloc_to_phys
> should be used instead of __pa to convert an ioremapped virtual
> address to a valid physical address. The fix in this patch is to
> remember the original bus address as probed at boot time and to pass
> this to io_remap_pfn_range.
>
> Signed-off-by: Keir Fraser <[email protected]>

Hi-

Our io_remap_pfn_range() patches don't contain many collisions.
My first patch [adding io_remap_pfn_range() to all arches]
<http://marc.theaimsgroup.com/?l=linux-mm&m=111049473410099&w=2>
does go a little further than yours in that regard.

Also, I was under the impression (only, so this is a question)
that this type of construct (from your patch):

+#ifndef io_remap_pfn_range
+#define io_remap_pfn_range remap_pfn_range
+#endif

only works for #defines (macros), while in some arches
io_remap_page_range() (and presumably io_remap_pfn_range)
is a function [sparc32/64] or inline function [mips].

My first patch referenced a future patch to convert
all callers of io_remap_page_range() to io_remap_pfn_range(),
which I have now done and built succesfully on 8 arches.
I'll post it now.

--
~Randy

2005-03-17 08:57:47

by Keir Fraser

[permalink] [raw]
Subject: Re: [PATCH] Xen/i386 cleanups - io_remap_pfn_range


On 17 Mar 2005, at 04:41, Randy.Dunlap wrote:

> Our io_remap_pfn_range() patches don't contain many collisions.
> My first patch [adding io_remap_pfn_range() to all arches]
> <http://marc.theaimsgroup.com/?l=linux-mm&m=111049473410099&w=2>
> does go a little further than yours in that regard.
>
> Also, I was under the impression (only, so this is a question)
> that this type of construct (from your patch):
>
> +#ifndef io_remap_pfn_range
> +#define io_remap_pfn_range remap_pfn_range
> +#endif
>
> only works for #defines (macros), while in some arches
> io_remap_page_range() (and presumably io_remap_pfn_range)
> is a function [sparc32/64] or inline function [mips].
>
> My first patch referenced a future patch to convert
> all callers of io_remap_page_range() to io_remap_pfn_range(),
> which I have now done and built succesfully on 8 arches.
> I'll post it now.

The way in which you introduce io_remap_pfn_range() into all
architectures is much better than my method, and doesn't depend on
io_remap_pfn_range being a macro.
Apart from that, yes: our driver patches are quite disjoint and
complement each other.
Hopefully a combined patch could eliminate some of the 'ifdef sparc's
that are scattered around. :-)

-- Keir