Re: [PATCH 0 of 4] x86: some more patches

[Andi Kleen <ak@suse.de>]

On Wednesday 16 January 2008 15:54:27 Ingo Molnar wrote:
> 
> * Andi Kleen <ak@suse.de> wrote:
> 
> > >  #define PHYSICAL_PAGE_MASK	(PAGE_MASK & __PHYSICAL_MASK)
> > 
> > 
> > I haven't tested yet, but we looked at that one earlier and I thought 
> > it was ok because
> > 
> > #define __PHYSICAL_MASK          _AT(phys_addr_t, (_AC(1,ULL) << __PHYSICAL_MASK_SHIFT) - 1)
> > 
> > and 
> > 
> > typedef u64     phys_addr_t;
> > 
> > for PAE. So the expression above should have been already 64bit.
> 
> no. The problem is that PAGE_MASK is:

I covered that in the end of the email.
> 
> 
> (PHYSICAL_PAGE_MASK is broken too in the same way, i just fixed that in 
> my tree - but it's not used by anything on 32-bit PAE but by PAGE_MASK)

Yes, but if you cast to long the result will be 32bit again. Or do you
rely on it being always used in 64bit signed context with sign extension?
While that might work it would seem rather fragile to me, just asking
for similar future bugs.

-Andi