Patchwork uio: Support 36-bit physical addresses on 32-bit systems

login
register
mail settings
Submitter Kumar Gala
Date Oct. 12, 2011, 2:35 p.m.
Message ID <1318430145-19898-1-git-send-email-galak@kernel.crashing.org>
Download mbox | patch
Permalink /patch/119221/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Kumar Gala - Oct. 12, 2011, 2:35 p.m.
From: Kai Jiang <Kai.Jiang@freescale.com>

To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
extend the width of 'addr' in struct uio_mem.  Numerous platforms like
embedded PPC, ARM, and X86 have support for systems with larger physical
address than logical.

Since 'addr' may contain a physical, logical, or virtual address the
easiest solution is to just change the type to 'unsigned long long'
regardless of which type is utilized.

For physical address we can support up to a 44-bit physical address on a
typical 32-bit system as we utilize remap_pfn_range() for the mapping of
the memory region and pfn's are represnted by shifting the address by
the page size (typically 4k).

Signed-off-by: Kai Jiang <Kai.Jiang@freescale.com>
Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
---
 drivers/uio/uio.c          |    8 ++++----
 include/linux/uio_driver.h |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)
Hans J. Koch - Oct. 12, 2011, 3:32 p.m.
On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
> From: Kai Jiang <Kai.Jiang@freescale.com>
> 
> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
> embedded PPC, ARM, and X86 have support for systems with larger physical
> address than logical.
> 
> Since 'addr' may contain a physical, logical, or virtual address the
> easiest solution is to just change the type to 'unsigned long long'
> regardless of which type is utilized.

No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
Please use that.

Thanks,
Hans

> 
> For physical address we can support up to a 44-bit physical address on a
> typical 32-bit system as we utilize remap_pfn_range() for the mapping of
> the memory region and pfn's are represnted by shifting the address by
> the page size (typically 4k).
> 
> Signed-off-by: Kai Jiang <Kai.Jiang@freescale.com>
> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
> ---
>  drivers/uio/uio.c          |    8 ++++----
>  include/linux/uio_driver.h |    2 +-
>  2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
> index d2efe82..a927c51 100644
> --- a/drivers/uio/uio.c
> +++ b/drivers/uio/uio.c
> @@ -69,7 +69,7 @@ static ssize_t map_name_show(struct uio_mem *mem, char *buf)
>  
>  static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
>  {
> -	return sprintf(buf, "0x%lx\n", mem->addr);
> +	return sprintf(buf, "0x%llx\n", mem->addr);
>  }
>  
>  static ssize_t map_size_show(struct uio_mem *mem, char *buf)
> @@ -79,7 +79,7 @@ static ssize_t map_size_show(struct uio_mem *mem, char *buf)
>  
>  static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
>  {
> -	return sprintf(buf, "0x%lx\n", mem->addr & ~PAGE_MASK);
> +	return sprintf(buf, "0x%llx\n", mem->addr & ~PAGE_MASK);
>  }
>  
>  struct map_sysfs_entry {
> @@ -634,8 +634,8 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>  	if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
>  		page = virt_to_page(idev->info->mem[mi].addr + offset);
>  	else
> -		page = vmalloc_to_page((void *)idev->info->mem[mi].addr
> -							+ offset);
> +		page = vmalloc_to_page((void *)(unsigned long)
> +				idev->info->mem[mi].addr + offset);
>  	get_page(page);
>  	vmf->page = page;
>  	return 0;
> diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
> index 665517c..c1fdb19 100644
> --- a/include/linux/uio_driver.h
> +++ b/include/linux/uio_driver.h
> @@ -31,7 +31,7 @@ struct uio_map;
>   */
>  struct uio_mem {
>  	const char		*name;
> -	unsigned long		addr;
> +	unsigned long long	addr;
>  	unsigned long		size;
>  	int			memtype;
>  	void __iomem		*internal_addr;
> -- 
> 1.7.3.4
> 
>
Kumar Gala - Oct. 12, 2011, 4:07 p.m.
On Oct 12, 2011, at 10:32 AM, Hans J. Koch wrote:

> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
>> From: Kai Jiang <Kai.Jiang@freescale.com>
>> 
>> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
>> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
>> embedded PPC, ARM, and X86 have support for systems with larger physical
>> address than logical.
>> 
>> Since 'addr' may contain a physical, logical, or virtual address the
>> easiest solution is to just change the type to 'unsigned long long'
>> regardless of which type is utilized.
> 
> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
> Please use that.

Do we believe phys_addr_t is always greater than or equal to size need for logical & virtual addresses?

- k


> Thanks,
> Hans
> 
>> 
>> For physical address we can support up to a 44-bit physical address on a
>> typical 32-bit system as we utilize remap_pfn_range() for the mapping of
>> the memory region and pfn's are represnted by shifting the address by
>> the page size (typically 4k).
>> 
>> Signed-off-by: Kai Jiang <Kai.Jiang@freescale.com>
>> Signed-off-by: Minghuan Lian <Minghuan.Lian@freescale.com>
>> Signed-off-by: Kumar Gala <galak@kernel.crashing.org>
>> ---
>> drivers/uio/uio.c          |    8 ++++----
>> include/linux/uio_driver.h |    2 +-
>> 2 files changed, 5 insertions(+), 5 deletions(-)
>> 
>> diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
>> index d2efe82..a927c51 100644
>> --- a/drivers/uio/uio.c
>> +++ b/drivers/uio/uio.c
>> @@ -69,7 +69,7 @@ static ssize_t map_name_show(struct uio_mem *mem, char *buf)
>> 
>> static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
>> {
>> -	return sprintf(buf, "0x%lx\n", mem->addr);
>> +	return sprintf(buf, "0x%llx\n", mem->addr);
>> }
>> 
>> static ssize_t map_size_show(struct uio_mem *mem, char *buf)
>> @@ -79,7 +79,7 @@ static ssize_t map_size_show(struct uio_mem *mem, char *buf)
>> 
>> static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
>> {
>> -	return sprintf(buf, "0x%lx\n", mem->addr & ~PAGE_MASK);
>> +	return sprintf(buf, "0x%llx\n", mem->addr & ~PAGE_MASK);
>> }
>> 
>> struct map_sysfs_entry {
>> @@ -634,8 +634,8 @@ static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
>> 	if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
>> 		page = virt_to_page(idev->info->mem[mi].addr + offset);
>> 	else
>> -		page = vmalloc_to_page((void *)idev->info->mem[mi].addr
>> -							+ offset);
>> +		page = vmalloc_to_page((void *)(unsigned long)
>> +				idev->info->mem[mi].addr + offset);
>> 	get_page(page);
>> 	vmf->page = page;
>> 	return 0;
>> diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
>> index 665517c..c1fdb19 100644
>> --- a/include/linux/uio_driver.h
>> +++ b/include/linux/uio_driver.h
>> @@ -31,7 +31,7 @@ struct uio_map;
>>  */
>> struct uio_mem {
>> 	const char		*name;
>> -	unsigned long		addr;
>> +	unsigned long long	addr;
>> 	unsigned long		size;
>> 	int			memtype;
>> 	void __iomem		*internal_addr;
>> -- 
>> 1.7.3.4
>> 
>>
Geert Uytterhoeven - Oct. 12, 2011, 4:15 p.m.
On Wed, Oct 12, 2011 at 18:07, Kumar Gala <galak@kernel.crashing.org> wrote:
> On Oct 12, 2011, at 10:32 AM, Hans J. Koch wrote:
>> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
>>> From: Kai Jiang <Kai.Jiang@freescale.com>
>>>
>>> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
>>> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
>>> embedded PPC, ARM, and X86 have support for systems with larger physical
>>> address than logical.
>>>
>>> Since 'addr' may contain a physical, logical, or virtual address the
>>> easiest solution is to just change the type to 'unsigned long long'
>>> regardless of which type is utilized.
>>
>> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
>> Please use that.
>
> Do we believe phys_addr_t is always greater than or equal to size need for logical & virtual addresses?

Yes:

#ifdef CONFIG_PHYS_ADDR_T_64BIT
typedef u64 phys_addr_t;
#else
typedef u32 phys_addr_t;
#endif

config PHYS_ADDR_T_64BIT
        def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Hans J. Koch - Oct. 12, 2011, 4:16 p.m.
On Wed, Oct 12, 2011 at 11:07:09AM -0500, Kumar Gala wrote:
> 
> On Oct 12, 2011, at 10:32 AM, Hans J. Koch wrote:
> 
> > On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
> >> From: Kai Jiang <Kai.Jiang@freescale.com>
> >> 
> >> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
> >> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
> >> embedded PPC, ARM, and X86 have support for systems with larger physical
> >> address than logical.
> >> 
> >> Since 'addr' may contain a physical, logical, or virtual address the
> >> easiest solution is to just change the type to 'unsigned long long'
> >> regardless of which type is utilized.
> > 
> > No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
> > Please use that.
> 
> Do we believe phys_addr_t is always greater than or equal to size need for logical & virtual addresses?

Yes.

Hans
Hans J. Koch - Oct. 12, 2011, 4:19 p.m.
On Wed, Oct 12, 2011 at 05:32:29PM +0200, Hans J. Koch wrote:
> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
> > From: Kai Jiang <Kai.Jiang@freescale.com>
> > 
> > To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
> > extend the width of 'addr' in struct uio_mem.  Numerous platforms like
> > embedded PPC, ARM, and X86 have support for systems with larger physical
> > address than logical.
> > 
> > Since 'addr' may contain a physical, logical, or virtual address the
> > easiest solution is to just change the type to 'unsigned long long'
> > regardless of which type is utilized.
> 
> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
> Please use that.

I forgot: If you resend this, please update the documentation as well.
(Documentation/DocBook/uio-howto.tmpl)

Thanks,
Hans
Kumar Gala - Oct. 12, 2011, 6:40 p.m.
On Oct 12, 2011, at 11:19 AM, Hans J. Koch wrote:

> On Wed, Oct 12, 2011 at 05:32:29PM +0200, Hans J. Koch wrote:
>> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
>>> From: Kai Jiang <Kai.Jiang@freescale.com>
>>> 
>>> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
>>> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
>>> embedded PPC, ARM, and X86 have support for systems with larger physical
>>> address than logical.
>>> 
>>> Since 'addr' may contain a physical, logical, or virtual address the
>>> easiest solution is to just change the type to 'unsigned long long'
>>> regardless of which type is utilized.
>> 
>> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
>> Please use that.
> 
> I forgot: If you resend this, please update the documentation as well.
> (Documentation/DocBook/uio-howto.tmpl)

What would you look added or modified here?

- k
Hans J. Koch - Oct. 12, 2011, 8:23 p.m.
On Wed, Oct 12, 2011 at 01:40:22PM -0500, Kumar Gala wrote:
> 
> On Oct 12, 2011, at 11:19 AM, Hans J. Koch wrote:
> 
> > On Wed, Oct 12, 2011 at 05:32:29PM +0200, Hans J. Koch wrote:
> >> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
> >>> From: Kai Jiang <Kai.Jiang@freescale.com>
> >>> 
> >>> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
> >>> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
> >>> embedded PPC, ARM, and X86 have support for systems with larger physical
> >>> address than logical.
> >>> 
> >>> Since 'addr' may contain a physical, logical, or virtual address the
> >>> easiest solution is to just change the type to 'unsigned long long'
> >>> regardless of which type is utilized.
> >> 
> >> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
> >> Please use that.
> > 
> > I forgot: If you resend this, please update the documentation as well.
> > (Documentation/DocBook/uio-howto.tmpl)
> 
> What would you look added or modified here?

struct uio_mem ?

Hans

> 
> - k
Kumar Gala - Oct. 12, 2011, 8:58 p.m.
On Oct 12, 2011, at 3:23 PM, Hans J. Koch wrote:

> On Wed, Oct 12, 2011 at 01:40:22PM -0500, Kumar Gala wrote:
>> 
>> On Oct 12, 2011, at 11:19 AM, Hans J. Koch wrote:
>> 
>>> On Wed, Oct 12, 2011 at 05:32:29PM +0200, Hans J. Koch wrote:
>>>> On Wed, Oct 12, 2011 at 09:35:45AM -0500, Kumar Gala wrote:
>>>>> From: Kai Jiang <Kai.Jiang@freescale.com>
>>>>> 
>>>>> To support >32-bit physical addresses for UIO_MEM_PHYS type we need to
>>>>> extend the width of 'addr' in struct uio_mem.  Numerous platforms like
>>>>> embedded PPC, ARM, and X86 have support for systems with larger physical
>>>>> address than logical.
>>>>> 
>>>>> Since 'addr' may contain a physical, logical, or virtual address the
>>>>> easiest solution is to just change the type to 'unsigned long long'
>>>>> regardless of which type is utilized.
>>>> 
>>>> No. There's phys_addr_t for that purpose, defined in include/linux/types.h.
>>>> Please use that.
>>> 
>>> I forgot: If you resend this, please update the documentation as well.
>>> (Documentation/DocBook/uio-howto.tmpl)
>> 
>> What would you look added or modified here?
> 
> struct uio_mem ?
> 
> Hans

New version sent, let me know if there are any other aspects to the DocBook that need updating.

- k
Tabi Timur-B04825 - Oct. 13, 2011, 2:10 p.m.
On Wed, Oct 12, 2011 at 11:15 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>
>> Do we believe phys_addr_t is always greater than or equal to size need for logical & virtual addresses?
>
> Yes:
>
> #ifdef CONFIG_PHYS_ADDR_T_64BIT
> typedef u64 phys_addr_t;
> #else
> typedef u32 phys_addr_t;
> #endif

This isn't really an answer to the question.  This just says that
phys_addr_t can be 64-bit.  I don't see anywhere in the kernel that we
*enforce* or *require* that sizeof(phys_addr_t) >= sizeof(void *).
Geert Uytterhoeven - Oct. 13, 2011, 2:25 p.m.
On Thu, Oct 13, 2011 at 16:10, Tabi Timur-B04825 <B04825@freescale.com> wrote:
> On Wed, Oct 12, 2011 at 11:15 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>>
>>> Do we believe phys_addr_t is always greater than or equal to size need for logical & virtual addresses?
>>
>> Yes:
>>
>> #ifdef CONFIG_PHYS_ADDR_T_64BIT
>> typedef u64 phys_addr_t;
>> #else
>> typedef u32 phys_addr_t;
>> #endif
>
> This isn't really an answer to the question.  This just says that
> phys_addr_t can be 64-bit.  I don't see anywhere in the kernel that we
> *enforce* or *require* that sizeof(phys_addr_t) >= sizeof(void *).

You deleted this part:

config PHYS_ADDR_T_64BIT
       def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT

which enforces that PHYS_ADDR_T_64BIT is enabled if 64BIT is set.

It still doesn't protect against 64-bit architectures not setting 64BIT,
but they have worse problems ;-)

On 32-bit platforms, void * is 32-bit, and phys_addr_t is either
32-bit or 64-bit.
On 64-bit platforms (which are required to set 64BIT), void * is 64-bit,
just like phys_addr_t.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Timur Tabi - Oct. 13, 2011, 2:35 p.m.
Geert Uytterhoeven wrote:

> You deleted this part:
> 
> config PHYS_ADDR_T_64BIT
>        def_bool 64BIT || ARCH_PHYS_ADDR_T_64BIT
> 
> which enforces that PHYS_ADDR_T_64BIT is enabled if 64BIT is set.

Ok, I didn't catch that before, but it makes sense now.  Thanks.

Patch

diff --git a/drivers/uio/uio.c b/drivers/uio/uio.c
index d2efe82..a927c51 100644
--- a/drivers/uio/uio.c
+++ b/drivers/uio/uio.c
@@ -69,7 +69,7 @@  static ssize_t map_name_show(struct uio_mem *mem, char *buf)
 
 static ssize_t map_addr_show(struct uio_mem *mem, char *buf)
 {
-	return sprintf(buf, "0x%lx\n", mem->addr);
+	return sprintf(buf, "0x%llx\n", mem->addr);
 }
 
 static ssize_t map_size_show(struct uio_mem *mem, char *buf)
@@ -79,7 +79,7 @@  static ssize_t map_size_show(struct uio_mem *mem, char *buf)
 
 static ssize_t map_offset_show(struct uio_mem *mem, char *buf)
 {
-	return sprintf(buf, "0x%lx\n", mem->addr & ~PAGE_MASK);
+	return sprintf(buf, "0x%llx\n", mem->addr & ~PAGE_MASK);
 }
 
 struct map_sysfs_entry {
@@ -634,8 +634,8 @@  static int uio_vma_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
 	if (idev->info->mem[mi].memtype == UIO_MEM_LOGICAL)
 		page = virt_to_page(idev->info->mem[mi].addr + offset);
 	else
-		page = vmalloc_to_page((void *)idev->info->mem[mi].addr
-							+ offset);
+		page = vmalloc_to_page((void *)(unsigned long)
+				idev->info->mem[mi].addr + offset);
 	get_page(page);
 	vmf->page = page;
 	return 0;
diff --git a/include/linux/uio_driver.h b/include/linux/uio_driver.h
index 665517c..c1fdb19 100644
--- a/include/linux/uio_driver.h
+++ b/include/linux/uio_driver.h
@@ -31,7 +31,7 @@  struct uio_map;
  */
 struct uio_mem {
 	const char		*name;
-	unsigned long		addr;
+	unsigned long long	addr;
 	unsigned long		size;
 	int			memtype;
 	void __iomem		*internal_addr;