From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751693AbeA3M13 (ORCPT ); Tue, 30 Jan 2018 07:27:29 -0500 Received: from foss.arm.com ([217.140.101.70]:53026 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751464AbeA3M11 (ORCPT ); Tue, 30 Jan 2018 07:27:27 -0500 Subject: Re: [PATCH v2 15/16] arm/arm64: smccc: Implement SMCCC v1.1 inline primitive To: Ard Biesheuvel Cc: Linux Kernel Mailing List , linux-arm-kernel , kvmarm , Catalin Marinas , Will Deacon , Peter Maydell , Christoffer Dall , Lorenzo Pieralisi , Mark Rutland , Robin Murphy , Jon Masters References: <20180129174559.1866-1-marc.zyngier@arm.com> <20180129174559.1866-16-marc.zyngier@arm.com> From: Marc Zyngier Organization: ARM Ltd Message-ID: <03484caa-2e18-7126-7d99-8d0b1e0a83c6@arm.com> Date: Tue, 30 Jan 2018 12:27:23 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-GB Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 29/01/18 21:45, Ard Biesheuvel wrote: > On 29 January 2018 at 17:45, Marc Zyngier wrote: >> One of the major improvement of SMCCC v1.1 is that it only clobbers >> the first 4 registers, both on 32 and 64bit. This means that it >> becomes very easy to provide an inline version of the SMC call >> primitive, and avoid performing a function call to stash the >> registers that would otherwise be clobbered by SMCCC v1.0. >> >> Signed-off-by: Marc Zyngier >> --- >> include/linux/arm-smccc.h | 157 ++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 157 insertions(+) >> >> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h >> index dd44d8458c04..bc5843728909 100644 >> --- a/include/linux/arm-smccc.h >> +++ b/include/linux/arm-smccc.h >> @@ -150,5 +150,162 @@ asmlinkage void __arm_smccc_hvc(unsigned long a0, unsigned long a1, >> >> #define arm_smccc_hvc_quirk(...) __arm_smccc_hvc(__VA_ARGS__) >> >> +/* SMCCC v1.1 implementation madness follows */ >> +#ifdef CONFIG_ARM64 >> + >> +#define SMCCC_SMC_INST "smc #0" >> +#define SMCCC_HVC_INST "hvc #0" >> + >> +#define __arm_smccc_1_1_prologue(inst) \ >> + inst "\n" \ >> + "cbz %[ptr], 1f\n" \ >> + "stp %x[r0], %x[r1], %[ra0]\n" \ >> + "stp %x[r2], %x[r3], %[ra2]\n" \ >> + "1:\n" \ >> + : [ra0] "=Ump" (*(&___res->a0)), \ >> + [ra2] "=Ump" (*(&___res->a2)), >> + >> +#define __arm_smccc_1_1_epilogue : "memory" >> + >> +#endif >> + >> +#ifdef CONFIG_ARM >> +#include >> +#include >> + >> +#define SMCCC_SMC_INST __SMC(0) >> +#define SMCCC_HVC_INST __HVC(0) >> + >> +#define __arm_smccc_1_1_prologue(inst) \ >> + inst "\n" \ >> + "cmp %[ptr], #0\n" \ >> + "stmne %[ptr], {%[r0], %[r1], %[r2], %[r3]}\n" \ >> + : "=m" (*___res), >> + >> +#define __arm_smccc_1_1_epilogue : "memory", "cc" >> + >> +#endif >> + >> +#define __constraint_write_0 \ >> + [r0] "+r" (r0), [r1] "=r" (r1), [r2] "=r" (r2), [r3] "=r" (r3) >> +#define __constraint_write_1 \ >> + [r0] "+r" (r0), [r1] "+r" (r1), [r2] "=r" (r2), [r3] "=r" (r3) >> +#define __constraint_write_2 \ >> + [r0] "+r" (r0), [r1] "+r" (r1), [r2] "+r" (r2), [r3] "=r" (r3) >> +#define __constraint_write_3 \ >> + [r0] "+r" (r0), [r1] "+r" (r1), [r2] "+r" (r2), [r3] "+r" (r3) > > It seems you need +r for all arguments, otherwise the compiler will > notice that the value is never used, and may assign the register to > 'res' instead, i.e., > > 3e4: 320107e0 mov w0, #0x80000001 // #-2147483647 > 3e8: 320183e1 mov w1, #0x80008000 // #-2147450880 > 3ec: 910123a2 add x2, x29, #0x48 > 3f0: d4000002 hvc #0x0 > 3f4: b4000062 cbz x2, 400 > 3f8: a90487a0 stp x0, x1, [x29, #72] > 3fc: a9058fa2 stp x2, x3, [x29, #88] > > (for the code generated in the next patch) Well spotted. I think this is because of the lack of early-clobber for the unassigned registers. The compiler assumes the whole sequence is a single instruction, with the output registers being affected at the end. If we mark those with '=&r', we will prevent GCC from emitting this kind of horror. Note that with Robin's trick of moving the assignment back to C code, this is a bit moot as this is really a single instruction (smc/hvc), and there is no intermediate register evaluation. Thanks, M. -- Jazz is not dead. It just smells funny...