arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi Russel!

I've noticed sheevaplug is not able to boot with
kirkwood_defconfig anymore. After dumb bisection
I've figured out that
    CONFIG_ARM_PATCH_PHYS_VIRT=y
was the problem.

The commit in question is :
> c1becedc8871645278832fabdc6fe138082a495b is the first bad commit
> commit c1becedc8871645278832fabdc6fe138082a495b
> Author: Russell King <rmk+kernel@arm.linux.org.uk>
> Date:   Wed Aug 10 10:23:45 2011 +0100
> 
>     ARM: enable ARM_PATCH_PHYS_VIRT by default

In order to make the box boot I had to set:
    CONFIG_EMBEDDED=y
    # CONFIG_ARM_PATCH_PHYS_VIRT is not set
   CONFIG_PHYS_OFFSET=0x0

Do you have an idea why it does not boot anymore by default?

Thanks!

-- 

  Sergei

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

[Ian Campbell]

On Sun, 2012-04-08 at 11:59 +0300, Sergei Trofimovich wrote:
> In order to make the box boot I had to set:
>     CONFIG_EMBEDDED=y
>     # CONFIG_ARM_PATCH_PHYS_VIRT is not set
>    CONFIG_PHYS_OFFSET=0x0
> 
> Do you have an idea why it does not boot anymore by default?

There is a u-boot commit which fixes this issue:

commit 679530278d5a79d34e356ad2d452f4400953bfc2
Author: Michael Walle <[michael@walle.cc]>
Date:   Mon Feb 6 22:42:10 2012 +0530

    arm, arm-kirkwood: disable l2c before linux boot
    
    The decompressor expects the L2 cache to be disabled. This fixes booting
    some kernels, which have CONFIG_ARM_PATCH_PHYS_VIRT enabled.
    
    Signed-off-by: Michael Walle <michael@walle.cc>
    Acked-by: Prafulla Wadaskar <prafulla@marvell.com>
    Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
    Cc: Prafulla Wadaskar <prafulla@marvell.com>
    Cc: Wolfgang Denk <wd@denx.de>

Ian.

-- 
Ian Campbell


If I can have honesty, it's easier to overlook mistakes.
		-- Kirk, "Space Seed", stardate 3141.9

Re: arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

[Sergei Trofimovich]

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

On Mon, 09 Apr 2012 22:28:28 +0100
Ian Campbell <ijc@hellion.org.uk> wrote:

> On Sun, 2012-04-08 at 11:59 +0300, Sergei Trofimovich wrote:
> > In order to make the box boot I had to set:
> >     CONFIG_EMBEDDED=y
> >     # CONFIG_ARM_PATCH_PHYS_VIRT is not set
> >    CONFIG_PHYS_OFFSET=0x0
> > 
> > Do you have an idea why it does not boot anymore by default?
> 
> There is a u-boot commit which fixes this issue:
> 
> commit 679530278d5a79d34e356ad2d452f4400953bfc2
> Author: Michael Walle <[michael@walle.cc]>
> Date:   Mon Feb 6 22:42:10 2012 +0530
> 
>     arm, arm-kirkwood: disable l2c before linux boot
>     
>     The decompressor expects the L2 cache to be disabled. This fixes booting
>     some kernels, which have CONFIG_ARM_PATCH_PHYS_VIRT enabled.
>     
>     Signed-off-by: Michael Walle <michael@walle.cc>
>     Acked-by: Prafulla Wadaskar <prafulla@marvell.com>
>     Cc: Albert ARIBAUD <albert.u.boot@aribaud.net>
>     Cc: Prafulla Wadaskar <prafulla@marvell.com>
>     Cc: Wolfgang Denk <wd@denx.de>

Thank you, Ian. Updating u-boot helped!

Sorry for the noise.

-- 

  Sergei

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

[Gerlando Falauto]

Hi everyone,

On 04/10/2012 08:12 PM, Sergei Trofimovich wrote:
> On Mon, 09 Apr 2012 22:28:28 +0100
> Ian Campbell<ijc@hellion.org.uk>  wrote:
>
>> On Sun, 2012-04-08 at 11:59 +0300, Sergei Trofimovich wrote:
>>> In order to make the box boot I had to set:
>>>      CONFIG_EMBEDDED=y
>>>      # CONFIG_ARM_PATCH_PHYS_VIRT is not set
>>>     CONFIG_PHYS_OFFSET=0x0
>>>
>>> Do you have an idea why it does not boot anymore by default?
>>
>> There is a u-boot commit which fixes this issue:
>>
>> commit 679530278d5a79d34e356ad2d452f4400953bfc2
>> Author: Michael Walle<[michael@walle.cc]>
>> Date:   Mon Feb 6 22:42:10 2012 +0530
>>
>>      arm, arm-kirkwood: disable l2c before linux boot
>>
>>      The decompressor expects the L2 cache to be disabled. This fixes booting
>>      some kernels, which have CONFIG_ARM_PATCH_PHYS_VIRT enabled.
>>
>>      Signed-off-by: Michael Walle<michael@walle.cc>
>>      Acked-by: Prafulla Wadaskar<prafulla@marvell.com>
>>      Cc: Albert ARIBAUD<albert.u.boot@aribaud.net>
>>      Cc: Prafulla Wadaskar<prafulla@marvell.com>
>>      Cc: Wolfgang Denk<wd@denx.de>
>
> Thank you, Ian. Updating u-boot helped!
>
> Sorry for the noise.

I guess if updating u-boot is not an option, there's no other way around 
it than disabling CONFIG_ARM_PATCH_PHYS_VIRT altogether.

Could someone (Russell?) simply state what the consequences are of 
disabling such option? (Or, conversely, what its advantages are?)
Only reference I found is the original commit, but I couldn't really 
understand what it does (or to what purpose), see at the bottom.

Thank you very much!
Gerlando

commit dc21af99fadcfa0ae65b52fd0895f85824f0c288
Author: Russell King <rmk+kernel@arm.linux.org.uk>
Date:   Tue Jan 4 19:09:43 2011 +0000

     ARM: P2V: introduce phys_to_virt/virt_to_phys runtime patching

     This idea came from Nicolas, Eric Miao produced an initial version,
     which was then rewritten into this.

     Patch the physical to virtual translations at runtime.  As we modify
     the code, this makes it incompatible with XIP kernels, but allows us
     to achieve this with minimal loss of performance.

     As many translations are of the form:

         physical = virtual + (PHYS_OFFSET - PAGE_OFFSET)
         virtual = physical - (PHYS_OFFSET - PAGE_OFFSET)

     we generate an 'add' instruction for __virt_to_phys(), and a 'sub'
     instruction for __phys_to_virt().  We calculate at run time 
(PHYS_OFFSET
     - PAGE_OFFSET) by comparing the address prior to MMU initialization 
with
     where it should be once the MMU has been initialized, and place this
     constant into the above add/sub instructions.

     Once we have (PHYS_OFFSET - PAGE_OFFSET), we can calculate the real
     PHYS_OFFSET as PAGE_OFFSET is a build-time constant, and save this for
     the C-mode PHYS_OFFSET variable definition to use.

     At present, we are unable to support Realview with Sparsemem enabled
     as this uses a complex mapping function, and MSM as this requires a
     constant which will not fit in our math instruction.

     Add a module version magic string for this feature to prevent
     incompatible modules being loaded.

     Tested-by: Tony Lindgren <tony@atomide.com>
     Reviewed-by: Nicolas Pitre <nicolas.pitre@linaro.org>
     Tested-by: Nicolas Pitre <nicolas.pitre@linaro.org>
     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Re: arm: CONFIG_ARM_PATCH_PHYS_VIRT=y is broken on kirkwood

[Nicolas Pitre]

On Fri, 7 Dec 2012, Gerlando Falauto wrote:

> Hi everyone,
> 
> On 04/10/2012 08:12 PM, Sergei Trofimovich wrote:
> > On Mon, 09 Apr 2012 22:28:28 +0100
> > Ian Campbell<ijc@hellion.org.uk>  wrote:
> > 
> > > On Sun, 2012-04-08 at 11:59 +0300, Sergei Trofimovich wrote:
> > > > In order to make the box boot I had to set:
> > > >      CONFIG_EMBEDDED=y
> > > >      # CONFIG_ARM_PATCH_PHYS_VIRT is not set
> > > >     CONFIG_PHYS_OFFSET=0x0
> > > > 
> > > > Do you have an idea why it does not boot anymore by default?
> > > 
> > > There is a u-boot commit which fixes this issue:
> > > 
> > > commit 679530278d5a79d34e356ad2d452f4400953bfc2
> > > Author: Michael Walle<[michael@walle.cc]>
> > > Date:   Mon Feb 6 22:42:10 2012 +0530
> > > 
> > >      arm, arm-kirkwood: disable l2c before linux boot
> > > 
> > >      The decompressor expects the L2 cache to be disabled. This fixes
> > > booting
> > >      some kernels, which have CONFIG_ARM_PATCH_PHYS_VIRT enabled.
> > > 
> > >      Signed-off-by: Michael Walle<michael@walle.cc>
> > >      Acked-by: Prafulla Wadaskar<prafulla@marvell.com>
> > >      Cc: Albert ARIBAUD<albert.u.boot@aribaud.net>
> > >      Cc: Prafulla Wadaskar<prafulla@marvell.com>
> > >      Cc: Wolfgang Denk<wd@denx.de>
> > 
> > Thank you, Ian. Updating u-boot helped!
> > 
> > Sorry for the noise.
> 
> I guess if updating u-boot is not an option, there's no other way around it
> than disabling CONFIG_ARM_PATCH_PHYS_VIRT altogether.
> 
> Could someone (Russell?) simply state what the consequences are of disabling
> such option? (Or, conversely, what its advantages are?)
> Only reference I found is the original commit, but I couldn't really
> understand what it does (or to what purpose), see at the bottom.

Disabling this option will not do you any harm.  It will prevent your 
kernel binary from being usable on another platform as the physical RAM 
address will then be hardcoded in the compiled kernel, but there are 
still other things that prevents such sharing in this case anyway.

One thing that this option allowed is to remove all the physical RAM 
offset information for most platform out of the source tree.  So to not 
have this option might mean that you might be prompted to provide it at 
kernel configuration time.

Nicolas