Re: gpu/drm, x86, PAT: io_mapping_create_wc and resource_size_t

From: Stefan Lippers-Hollmann
Date: Sun Mar 01 2009 - 08:55:21 EST


Hi

On Samstag, 28. Februar 2009, Linux Kernel Mailing List wrote:
> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=4ab0d47d0ab311eb181532c1ecb6d02905685071
> Commit: 4ab0d47d0ab311eb181532c1ecb6d02905685071
> Parent: 6644107d57a8fa82b47e4c55da4d9d91a612f29c
> Author: Venkatesh Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
> AuthorDate: Tue Feb 24 17:35:12 2009 -0800
> Committer: Ingo Molnar <mingo@xxxxxxx>
> CommitDate: Wed Feb 25 13:09:51 2009 +0100
>
> gpu/drm, x86, PAT: io_mapping_create_wc and resource_size_t
>
> io_mapping_create_wc should take a resource_size_t parameter in place of
> unsigned long. With unsigned long, there will be no way to map greater than 4GB
> address in i386/32 bit.
>
> On x86, greater than 4GB addresses cannot be mapped on i386 without PAE. Return
> error for such a case.
>
> Patch also adds a structure for io_mapping, that saves the base, size and
> type on HAVE_ATOMIC_IOMAP archs, that can be used to verify the offset on
> io_mapping_map calls.
>
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@xxxxxxxxx>
> Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
> Cc: Dave Airlie <airlied@xxxxxxxxxx>
> Cc: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
> Cc: Eric Anholt <eric@xxxxxxxxxx>
> Cc: Keith Packard <keithp@xxxxxxxxxx>
> Signed-off-by: Ingo Molnar <mingo@xxxxxxx>

This patch, as part of 2.6.29-rc6-git5, fails to build for me on i386 (it
builds on amd64) with the attached (slimmed down) config (CONFIG_X86_PAT=y)
and buildlog:

Building modules, stage 2.
MODPOST 809 modules
ERROR: "pgprot_writecombine" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "is_io_mapping_possible" [drivers/gpu/drm/i915/i915.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2

This is a build regression in comparison to 2.6.29-rc6.

Regards
Stefan Lippers-Hollmann
--

> arch/x86/include/asm/iomap.h | 3 ++
> arch/x86/mm/iomap_32.c | 18 ++++++++++++++++
> include/linux/io-mapping.h | 46 +++++++++++++++++++++++++++++++----------
> 3 files changed, 56 insertions(+), 11 deletions(-)
>
> diff --git a/arch/x86/include/asm/iomap.h b/arch/x86/include/asm/iomap.h
> index c1f0628..86af260 100644
> --- a/arch/x86/include/asm/iomap.h
> +++ b/arch/x86/include/asm/iomap.h
> @@ -23,6 +23,9 @@
> #include <asm/pgtable.h>
> #include <asm/tlbflush.h>
>
> +int
> +is_io_mapping_possible(resource_size_t base, unsigned long size);
> +
> void *
> iomap_atomic_prot_pfn(unsigned long pfn, enum km_type type, pgprot_t prot);
>
> diff --git a/arch/x86/mm/iomap_32.c b/arch/x86/mm/iomap_32.c
> index ca53224..6c2b1af 100644
> --- a/arch/x86/mm/iomap_32.c
> +++ b/arch/x86/mm/iomap_32.c
> @@ -20,6 +20,24 @@
> #include <asm/pat.h>
> #include <linux/module.h>
>
> +#ifdef CONFIG_X86_PAE
> +int
> +is_io_mapping_possible(resource_size_t base, unsigned long size)
> +{
> + return 1;
> +}
> +#else
> +int
> +is_io_mapping_possible(resource_size_t base, unsigned long size)
> +{
> + /* There is no way to map greater than 1 << 32 address without PAE */
> + if (base + size > 0x100000000ULL)
> + return 0;
> +
> + return 1;
> +}
> +#endif
> +
> /* Map 'pfn' using fixed map 'type' and protections 'prot'
> */
> void *
> diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h
> index 82df317..cbc2f0c 100644
> --- a/include/linux/io-mapping.h
> +++ b/include/linux/io-mapping.h
> @@ -30,11 +30,14 @@
> * See Documentation/io_mapping.txt
> */
>
> -/* this struct isn't actually defined anywhere */
> -struct io_mapping;
> -
> #ifdef CONFIG_HAVE_ATOMIC_IOMAP
>
> +struct io_mapping {
> + resource_size_t base;
> + unsigned long size;
> + pgprot_t prot;
> +};
> +
> /*
> * For small address space machines, mapping large objects
> * into the kernel virtual space isn't practical. Where
> @@ -43,23 +46,40 @@ struct io_mapping;
> */
>
> static inline struct io_mapping *
> -io_mapping_create_wc(unsigned long base, unsigned long size)
> +io_mapping_create_wc(resource_size_t base, unsigned long size)
> {
> - return (struct io_mapping *) base;
> + struct io_mapping *iomap;
> +
> + if (!is_io_mapping_possible(base, size))
> + return NULL;
> +
> + iomap = kmalloc(sizeof(*iomap), GFP_KERNEL);
> + if (!iomap)
> + return NULL;
> +
> + iomap->base = base;
> + iomap->size = size;
> + iomap->prot = pgprot_writecombine(__pgprot(__PAGE_KERNEL));
> + return iomap;
> }
>
> static inline void
> io_mapping_free(struct io_mapping *mapping)
> {
> + kfree(mapping);
> }
>
> /* Atomic map/unmap */
> static inline void *
> io_mapping_map_atomic_wc(struct io_mapping *mapping, unsigned long offset)
> {
> - offset += (unsigned long) mapping;
> - return iomap_atomic_prot_pfn(offset >> PAGE_SHIFT, KM_USER0,
> - __pgprot(__PAGE_KERNEL_WC));
> + resource_size_t phys_addr;
> + unsigned long pfn;
> +
> + BUG_ON(offset >= mapping->size);
> + phys_addr = mapping->base + offset;
> + pfn = (unsigned long) (phys_addr >> PAGE_SHIFT);
> + return iomap_atomic_prot_pfn(pfn, KM_USER0, mapping->prot);
> }
>
> static inline void
> @@ -71,8 +91,9 @@ io_mapping_unmap_atomic(void *vaddr)
> static inline void *
> io_mapping_map_wc(struct io_mapping *mapping, unsigned long offset)
> {
> - offset += (unsigned long) mapping;
> - return ioremap_wc(offset, PAGE_SIZE);
> + BUG_ON(offset >= mapping->size);
> + resource_size_t phys_addr = mapping->base + offset;
> + return ioremap_wc(phys_addr, PAGE_SIZE);
> }
>
> static inline void
> @@ -83,9 +104,12 @@ io_mapping_unmap(void *vaddr)
>
> #else
>
> +/* this struct isn't actually defined anywhere */
> +struct io_mapping;
> +
> /* Create the io_mapping object*/
> static inline struct io_mapping *
> -io_mapping_create_wc(unsigned long base, unsigned long size)
> +io_mapping_create_wc(resource_size_t base, unsigned long size)
> {
> return (struct io_mapping *) ioremap_wc(base, size);
> }

Attachment: config-2.6.29-rc6-git5.gz
Description: GNU Zip compressed data

Attachment: linux-2.6.29-rc6-git5.build.log.gz
Description: GNU Zip compressed data