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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200916075706.GQ1551@shell.armlinux.org.uk>
Date:   Wed, 16 Sep 2020 08:57:07 +0100
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     "Leizhen (ThunderTown)" <thunder.leizhen@...wei.com>
Cc:     Daniel Lezcano <daniel.lezcano@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Catalin Marinas <catalin.marinas@....com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        patches-armlinux <patches@...linux.org.uk>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Libin <huawei.libin@...wei.com>,
        Kefeng Wang <wangkefeng.wang@...wei.com>,
        Jianguo Chen <chenjianguo3@...wei.com>
Subject: Re: [PATCH v2 2/2] ARM: support PHYS_OFFSET minimum aligned at 64KiB
 boundary

On Wed, Sep 16, 2020 at 09:57:15AM +0800, Leizhen (ThunderTown) wrote:
> On 2020/9/16 3:01, Russell King - ARM Linux admin wrote:
> > 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@...wei.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.
> 
> OK, I will remove it.
> 
> > 
> >> @@ -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.
> 
> Oh, yes, yes, I copied from the above statement, but forgot to remove it.
> 
> > 
> > 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.
> 
> It's one less instruction for 64KiB boundary && (sizeof(phys_addr_t) == 8),
> and no increase or decrease for 64KiB boundary && (sizeof(phys_addr_t) == 4),
> but one more instruction for 16MiB boundary.
> 
> And maybe: 16MiB is widely used, but 64KiB is rarely used.
> 
> So I'm inclined to the current revision.

Multiplatform kernels (which will be what distros build) will have to
enable this option if they wish to support this platform. So, in that
case it doesn't just impacting a single platform, but all platforms.

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