Build breakage from 'ARM: mm: use phys_addr_t appropriately in p2v and v2p conversions'

Commit Message

Jason Gunthorpe Nov. 25, 2013, 11:34 p.m. UTC

On Mon, Nov 25, 2013 at 11:20:03PM +0000, Russell King - ARM Linux wrote:
> On Mon, Nov 25, 2013 at 03:36:36PM -0700, Jason Gunthorpe wrote:
> > Which is after your new inlines..
> > 
> > An elegant fix wasn't obvious to me :)
> 
> Same here... it's all rather complicated because of all those ifdefs.
> Nothing easy comes to mind about how to fix this one.  I'll look into
> it at some point.
> 
> If not, it's going to have to be a revert - or we bite the bullet and
> start killing off some of these Kconfig options such as making
> ARM_PATCH_PHYS_VIRT mandatory.

This is enough to fix the build:


I don't seen an obvious major downside to loosing the inline, static
checking will have to be done with the ARM_PATCH_PHYS_VIRT branch..

I'm working on runtime testing this, let me know if you want a proper
patch.

Regards,
Jason

Comments

Russell King - ARM Linux Nov. 25, 2013, 11:39 p.m. UTC | #1

On Mon, Nov 25, 2013 at 04:34:58PM -0700, Jason Gunthorpe wrote:
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index eaf428c..b886eba 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -239,15 +239,8 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>  
>  #else
>  
> -static inline phys_addr_t __virt_to_phys(unsigned long x)
> -{
> -       return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
> -}
> -
> -static inline unsigned long __phys_to_virt(phys_addr_t x)
> -{
> -       return x - PHYS_OFFSET + PAGE_OFFSET;
> -}
> +#define __virt_to_phys(x)      ((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET)
> +#define __phys_to_virt(x)      ((unsigned long)((phys_addr_t)(x) - PHYS_OFFSET + PAGE_OFFSET))
> 
>  #endif
>  #endif
> 
> I don't seen an obvious major downside to loosing the inline, static
> checking will have to be done with the ARM_PATCH_PHYS_VIRT branch..

Well, at this point I'm seriously thinking that killing the __ versions
of this is the right thing to do: the original idea was that the __
versions were the underlying mathematical conversions, with all the
casting kept out of them - thereby making it easy for them to be
overridden without introducing subtle bugs.

That's been lost now with all the casting, so there's little point them
really existing.

Santosh Shilimkar Nov. 25, 2013, 11:48 p.m. UTC | #2

On Monday 25 November 2013 06:39 PM, Russell King - ARM Linux wrote:
> On Mon, Nov 25, 2013 at 04:34:58PM -0700, Jason Gunthorpe wrote:
>> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
>> index eaf428c..b886eba 100644
>> --- a/arch/arm/include/asm/memory.h
>> +++ b/arch/arm/include/asm/memory.h
>> @@ -239,15 +239,8 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>>  
>>  #else
>>  
>> -static inline phys_addr_t __virt_to_phys(unsigned long x)
>> -{
>> -       return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
>> -}
>> -
>> -static inline unsigned long __phys_to_virt(phys_addr_t x)
>> -{
>> -       return x - PHYS_OFFSET + PAGE_OFFSET;
>> -}
>> +#define __virt_to_phys(x)      ((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET)
>> +#define __phys_to_virt(x)      ((unsigned long)((phys_addr_t)(x) - PHYS_OFFSET + PAGE_OFFSET))
>>
>>  #endif
>>  #endif
>>
>> I don't seen an obvious major downside to loosing the inline, static
>> checking will have to be done with the ARM_PATCH_PHYS_VIRT branch..
> 
> Well, at this point I'm seriously thinking that killing the __ versions
> of this is the right thing to do: the original idea was that the __
> versions were the underlying mathematical conversions, with all the
> casting kept out of them - thereby making it easy for them to be
> overridden without introducing subtle bugs.
> 
> That's been lost now with all the casting, so there's little point them
> really existing.
> 
Just saw the thread. I was assuming that ARM_PATCH_PHYS_VIRT is always
enabled when the PLAT_PHYS_OFFSET was killed. Sorry for the build breakage.
NoMMU and machines with mach/memory.h still don't have ARM_PATCH_PHYS_VIRT
enabled so they would break.

Looks like you have proposed already a patch with PLAT_PHYS_OFFSET
which should help the case. Thanks.

Regards,
Santosh

Patch

diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index eaf428c..b886eba 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -239,15 +239,8 @@  static inline unsigned long __phys_to_virt(phys_addr_t x)
 
 #else
 
-static inline phys_addr_t __virt_to_phys(unsigned long x)
-{
-       return (phys_addr_t)x - PAGE_OFFSET + PHYS_OFFSET;
-}
-
-static inline unsigned long __phys_to_virt(phys_addr_t x)
-{
-       return x - PHYS_OFFSET + PAGE_OFFSET;
-}
+#define __virt_to_phys(x)      ((phys_addr_t)(x) - PAGE_OFFSET + PHYS_OFFSET)
+#define __phys_to_virt(x)      ((unsigned long)((phys_addr_t)(x) - PHYS_OFFSET + PAGE_OFFSET))

 #endif
 #endif