Re: MIPS: c17a6554 broke 64BIT_PHYS_ADDR for 32 bit systems

[Ralf Baechle <ralf@linux-mips.org>]

On Sat, Apr 20, 2013 at 11:42:54AM +0200, Thomas Lange 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'?
> 
> 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.

Thanks for noting this issue.  Manuel Lauss reported the same issue and
I already fixed it yesterday by reverting the offending patch.  Post rc7
reverting was the right things to do - and now that it's reverted I don't
really see the need for another patch?

Linus noticed that the revert is for an issue of which more instances might
exist and so hacked sparse to look for it, see message id

  CA+55aFy0jPr5udwJx8cZ-QFs-OgZzQ_NJm_VSj_cS7tsKUn3Kw@mail.gmail.com

on the linux-kernel mailing list or simply https://lkml.org/lkml/2013/4/22/518.

  Ralf