c17a6554 broke 64BIT_PHYS_ADDR for 32 bit systems

c17a6554 broke 64BIT_PHYS_ADDR for 32 bit systems

[Thomas Lange]

Dear Ralf,

commit c17a6554 unintentionally(?) modified the PAGE_MASK type
from (int) to (long unsigned int).

This breaks ioremap (and possibly more) when using 64BIT_PHYS_ADDR on
32 bit systems.
Example of failing code from ioremap.c:

	phys_addr &= PAGE_MASK;

Since phys_addr is 64 bit (unsigned long long) when 64BIT_PHYS_ADDR and
PAGE_MASK is 32bit (long unsigned int), the upper 32 bits will always
be zeroed which is not what we want/expect.

The code above works if PAGE_MASK is a _signed_ 32bit int though.

Some possible fixes:

A) Simply revert the commit. Makes ioremap work again, but then PAGE_MASK
    is a signed int. Do we really want a mask that is 'signed'?

B) Don't use PAGE_MASK for physical addresses. x86 defines this:

   /* Cast PAGE_MASK to a signed type so that it is sign-extended if
      virtual addresses are 32-bits but physical addresses are larger
      (ie, 32-bit PAE). */
   #define PHYSICAL_PAGE_MASK	(((signed long)PAGE_MASK) & __PHYSICAL_MASK)

   Perhaps mips need something similar?


This is an issue with 3.8 and doesn't seem to be solved in master either.

Regards,
/Thomas

Re: c17a6554 broke 64BIT_PHYS_ADDR for 32 bit systems

[Geert Uytterhoeven]

On Wed, Apr 17, 2013 at 8:11 PM, Thomas Lange <thomas@corelatus.se> wrote:
> commit c17a6554 unintentionally(?) modified the PAGE_MASK type
> from (int) to (long unsigned int).
>
> This breaks ioremap (and possibly more) when using 64BIT_PHYS_ADDR on
> 32 bit systems.
> Example of failing code from ioremap.c:
>
>         phys_addr &= PAGE_MASK;
>
> Since phys_addr is 64 bit (unsigned long long) when 64BIT_PHYS_ADDR and
> PAGE_MASK is 32bit (long unsigned int), the upper 32 bits will always
> be zeroed which is not what we want/expect.
>
> The code above works if PAGE_MASK is a _signed_ 32bit int though.
>
> Some possible fixes:
>
> A) Simply revert the commit. Makes ioremap work again, but then PAGE_MASK
>    is a signed int. Do we really want a mask that is 'signed'?

Already fixed, cfr. https://lkml.org/lkml/2013/4/22/518

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