| Message ID | 1318430145-19898-1-git-send-email-galak@kernel.crashing.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Kumar Gala |
| Headers | show |
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 > >
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 >> >>
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
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
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
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
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
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
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 *).
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
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.
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;