Re: [PATCH v2 2/2] ARM: support PHYS_OFFSET minimum aligned at 64KiB boundary - Russell King

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Zhen Lei <thunder.leizhen@huawei.com>
Cc: Jianguo Chen <chenjianguo3@huawei.com>,
	Kefeng Wang <wangkefeng.wang@huawei.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Daniel Lezcano <daniel.lezcano@linaro.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	Libin <huawei.libin@huawei.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Andrew Morton <akpm@linux-foundation.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	patches-armlinux <patches@armlinux.org.uk>
Subject: Re: [PATCH v2 2/2] ARM: support PHYS_OFFSET minimum aligned at 64KiB boundary
Date: Tue, 15 Sep 2020 20:01:43 +0100	[thread overview]
Message-ID: <20200915190143.GP1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200915131615.3138-3-thunder.leizhen@huawei.com>

On Tue, Sep 15, 2020 at 09:16:15PM +0800, Zhen Lei wrote:
> Currently, only support the kernels where the base of physical memory is
> at a 16MiB boundary. Because the add/sub instructions only contains 8bits
> unrotated value. But we can use one more "add/sub" instructions to handle
> bits 23-16. The performance will be slightly affected.
> 
> Since most boards meet 16 MiB alignment, so add a new configuration
> option ARM_PATCH_PHYS_VIRT_RADICAL (default n) to control it. Say Y if
> anyone really needs it.
> 
> All r0-r7 (r1 = machine no, r2 = atags or dtb, in the start-up phase) are
> used in __fixup_a_pv_table() now, but the callee saved r11 is not used in
> the whole head.S file. So choose it.
> 
> Because the calculation of "y = x + __pv_offset[63:24]" have been done,
> so we only need to calculate "y = y + __pv_offset[23:16]", that's why
> the parameters "to" and "from" of __pv_stub() and __pv_add_carry_stub()
> in the scope of CONFIG_ARM_PATCH_PHYS_VIRT_RADICAL are all passed "t"
> (above y).
> 
> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com>
> ---
>  arch/arm/Kconfig              | 18 +++++++++++++++++-
>  arch/arm/include/asm/memory.h | 16 +++++++++++++---
>  arch/arm/kernel/head.S        | 25 +++++++++++++++++++------
>  3 files changed, 49 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e00d94b16658765..19fc2c746e2ce29 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -240,12 +240,28 @@ config ARM_PATCH_PHYS_VIRT
>  	  kernel in system memory.
>  
>  	  This can only be used with non-XIP MMU kernels where the base
> -	  of physical memory is at a 16MB boundary.
> +	  of physical memory is at a 16MiB boundary.
>  
>  	  Only disable this option if you know that you do not require
>  	  this feature (eg, building a kernel for a single machine) and
>  	  you need to shrink the kernel to the minimal size.
>  
> +config ARM_PATCH_PHYS_VIRT_RADICAL
> +	bool "Support PHYS_OFFSET minimum aligned at 64KiB boundary"
> +	default n

Please drop the "default n" - this is the default anyway.

> @@ -236,6 +243,9 @@ static inline unsigned long __phys_to_virt(phys_addr_t x)
>  	 * in place where 'r' 32 bit operand is expected.
>  	 */
>  	__pv_stub((unsigned long) x, t, "sub", __PV_BITS_31_24);
> +#ifdef CONFIG_ARM_PATCH_PHYS_VIRT_RADICAL
> +	__pv_stub((unsigned long) t, t, "sub", __PV_BITS_23_16);

t is already unsigned long, so this cast is not necessary.

I've been debating whether it would be better to use "movw" for this
for ARMv7.  In other words:

	movw	tmp, #16-bit
	adds	%Q0, %1, tmp, lsl #16
	adc	%R0, %R0, #0

It would certainly be less instructions, but at the cost of an
additional register - and we'd have to change the fixup code to
know about movw.

Thoughts?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel