From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail05.groups.io (mail05.groups.io [45.79.224.7]) by spool.mail.gandi.net (Postfix) with ESMTPS id 22B7974003E for ; Tue, 4 Jun 2024 13:33:41 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=LJUSoa6VY/d29ITzXgkkmMSlw6IjZ16uoWrv0rFT6m0=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20240206; t=1717508021; v=1; b=pwLSndOVc9yTILwFiSzAstPfE8Tycxjdf4Na7NQ3y86fFYO4TBVKtM5/M6a/tK3AwPBVLz+U Tj0gwhTO6w3vR4053UBmJjzCFgfnc6Y1ZxZ/MWZ6xrQstj0m8FivUztQA9vxoDfytjnAdhCdjME hKQ7k39oz5XdSVgmJTz38Z/wocirlhD11EhElwRIiPLtxB5/KEEjS4ScBQFdCde7Q2CFhJIKozG oLz60wbaTPDim1d1pdpJgtVMS4uESQkebi5+/h0w+nah2qMNf16MHNnsftYBFEy2jF6wrt/2f1p UlNjw73Jt4j5b1XjAeZkME9LN2eYKTlVZu44xoapEmWHQ== X-Received: by 127.0.0.2 with SMTP id YXcDYY7687511xnB8cpgehkQ; Tue, 04 Jun 2024 06:33:40 -0700 X-Received: from sin.source.kernel.org (sin.source.kernel.org [145.40.73.55]) by mx.groups.io with SMTP id smtpd.web10.21700.1717508019335930512 for ; Tue, 04 Jun 2024 06:33:39 -0700 X-Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id 1FA7DCE113C for ; Tue, 4 Jun 2024 13:33:36 +0000 (UTC) X-Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4E4B1C2BBFC for ; Tue, 4 Jun 2024 13:33:35 +0000 (UTC) X-Received: by mail-lj1-f177.google.com with SMTP id 38308e7fff4ca-2ea9386cde0so56030921fa.2 for ; Tue, 04 Jun 2024 06:33:35 -0700 (PDT) X-Gm-Message-State: vwMso095guv2AaRl2lkmq2Gix7686176AA= X-Google-Smtp-Source: AGHT+IFNsp6ZcbQz+18cG2q2HlouG8p7PRifhyCG80IGvLQh6upoLOTyNyYu+t2JfC3VQ0QRGte5vkseGd7035D37+s= X-Received: by 2002:a05:651c:8d:b0:2e7:c29:dadc with SMTP id 38308e7fff4ca-2ea951a9e81mr77291021fa.34.1717508013649; Tue, 04 Jun 2024 06:33:33 -0700 (PDT) MIME-Version: 1.0 References: <28f07a88-f664-43f4-871d-b5d0b82c7727@linaro.org> <547e2870-c3cd-45ec-8a9c-208e835353c3@linaro.org> In-Reply-To: <547e2870-c3cd-45ec-8a9c-208e835353c3@linaro.org> From: "Ard Biesheuvel" Date: Tue, 4 Jun 2024 15:33:21 +0200 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [edk2-devel] ArmCallSmc() and SMCCC specification To: Marcin Juszkiewicz Cc: devel@edk2.groups.io, quic_llindhol@quicinc.com, Ard Biesheuvel , Sami Mujawar Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Tue, 04 Jun 2024 06:33:40 -0700 Resent-From: ardb@kernel.org Reply-To: devel@edk2.groups.io,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=pwLSndOV; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=kernel.org (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 45.79.224.7 as permitted sender) smtp.mailfrom=bounce@groups.io On Tue, 4 Jun 2024 at 14:37, Marcin Juszkiewicz wrote: > > W dniu 3.06.2024 o 18:47, Leif Lindholm via groups.io pisze: > > >> In 2020 we got version C of spec (and then D, E, F) which allows to > >> use more registers: > >> > >> > Allow R4=E2=80=94R7 (SMC32/HVC32) to be used as result registers. > >> > Allow X8=E2=80=94X17 to be used as parameter registers in SMC64/HVC= 64. > >> > Allow X4=E2=80=94X17 to be used as result registers in SMC64/HVC64. > >> > >> And I started to wonder how to update EDK2 to newer version of SMCCC > >> spec as one of in-progress QemuSbsa SMC calls may return more than 4 > >> values. > > > > Yes, definitely. This has been a wishlist item for some time, but in > > reality we've only ever updated these when we needed new functionality. > > > > >> ARM_SMC_ARGS in ArmSmcLib.h can be expanded to handle up to Arg17 in > >> an easy way and guarded by "#if defined(__aarch64__)" to not change it > >> on Arm32. > > My example code for it (including comment update): > > diff --git a/ArmPkg/Include/Library/ArmSmcLib.h b/ArmPkg/Include/Library/= ArmSmcLib.h > index beef0175c35c..af27781b72f6 100644 > --- a/ArmPkg/Include/Library/ArmSmcLib.h > +++ b/ArmPkg/Include/Library/ArmSmcLib.h > @@ -23,14 +23,31 @@ typedef struct { > UINTN Arg5; > UINTN Arg6; > UINTN Arg7; > +#if defined(__aarch64__) > + UINTN Arg8; > + UINTN Arg9; > + UINTN Arg10; > + UINTN Arg11; > + UINTN Arg12; > + UINTN Arg13; > + UINTN Arg14; > + UINTN Arg15; > + UINTN Arg16; > + UINTN Arg17; > +#endif > } ARM_SMC_ARGS; > > /** > Trigger an SMC call > > - SMC calls can take up to 7 arguments and return up to 4 return values. > - Therefore, the 4 first fields in the ARM_SMC_ARGS structure are used > - for both input and output values. > + on SMC32/HVC32 > + - R0 is a function identifier, R1-R7 are arguments > + - R0-R7 are results, R4-R7 must be preserved unless they contain resul= ts > + > + > + on SMC64/HVC64 > + - W0 is a function identifier, X1-X17 are arguments > + - X0-X17 are results, X4-X17 must be preserved unless they contain res= ults > > **/ > VOID > > > >> Then ArmCallSmc() in {AArch64,Arm}/ArmSmc.S needs changes. But here it > >> gets tricky. > >> > >> On Arm we preserve r4-r8 and restore them after call like spec says. > >> Which we do not do on AArch64 as version B of spec did not required > >> that (and this changed in version C). > >> > >> If we start handling more than 4 results then we need to know how many > >> results are expected and restore rest of r4-r7/x4-x17 registers: > > > > Yeah, it feels like we may want to add a version field or/and number of > > registers to save/restore to a new struct. And we should definitely > > align Arm/AArch64 behaviour. > > > > > >> From what I saw in both edk2/ and edk2-platforms/ most of code uses > >> ArmCallSmc() function with ARM_SMC_ARGS structure populared with > >> arguments. ArmCallSmc[0-3]() are used by Smbios, Psci and QemuSbsa > >> code only. > > > > The ArmCallSmc[0-3] helpers were only added as shorthand for most commo= n > > cases. I don't think we should worry about how those work for making an= y > > design decisions. Everything they do can be described by the main > > ArmCallSmc functions and a few lines of struct stuffing. > > >> Now the question is: how to handle change? > > > > It could be that it would be easiest to add a new call function, and > > maybe even ra new egister struct, than trying to adapt the existing one= s > > in ways that doesn't break existing users? > > > > That is if we care about existing users. We could always modify it to > > guarantee breakage and expect users to update their code. Since this is > > a library, and not a protocol, we *mostly* don't need to worry about > > users mixing prebuilt binaries using old structs with new functions. > > >> We could add ArmCallSmc[4-17] but that name only tells how many > >> arguments we pass to SMC call, not how many results we expect. Or > >> should we add NumberOfResults argument to ArmCallSmc() to know which > >> registers we should preserve and which are results? And how > >> complicated this assembly function will become? > > > > I don't think the assembly function needs to be that complicated. It'd > > be a bit tedious with a bunch of tests, but... > > Note: I do not know aarch64 assembly. Did some changes to ArmCallSmc(). > I moved to use x18 register (preserved on stack) instead of x9 one. And > changed code to handle 8 results (without preserving x4-x7). > You shouldn't use X18 - it is the so-called 'platform register' and has a special meaning. For example, this code could be called inside a runtime service invoked by the OS with interrupts enabled, and the OS's interrupt handler might misbehave if X18 was modified. > diff --git a/ArmPkg/Library/ArmSmcLib/AArch64/ArmSmc.S b/ArmPkg/Library/A= rmSmcLib/AArch64/ArmSmc.S > index 4a8c2a8f59ea..0525a0a7887f 100644 > --- a/ArmPkg/Library/ArmSmcLib/AArch64/ArmSmc.S > +++ b/ArmPkg/Library/ArmSmcLib/AArch64/ArmSmc.S > @@ -8,8 +8,10 @@ > #include > > ASM_FUNC(ArmCallSmc) > + // preserve x18 > + str x18, [sp, #-16]! > // Push x0 on the stack - The stack must always be quad-word aligned > - str x0, [sp, #-16]! > + str x0, [sp, #-32]! > This code does not honour the ordinary calling convention - better to stack and unstack the frame pointer and return address. That also gives you X30 (the link register) as a scratch register. // Create a stack frame stp x29, x30, [sp, #-16]! mov x29, sp // Preserve X0 for later use mov x30, x0 // Load the SMC arguments values into the appropriate registers ldp x0, x1, [x30, #0] ldp x2, x3, [x30, #16] ldp x4, x5, [x30, #32] ldp x6, x7, [x30, #48] ldp x8, x9, [x30, #64] ldp x10, x11, [x30, #80] ldp x12, x13, [x30, #96] ldp x14, x15, [x30, #112] ldp x16, x17, [x30, #128] smc #0 // A SMC call can return up to 18 values. stp x0, x1, [x30, #0] stp x2, x3, [x30, #16] stp x4, x5, [x30, #32] stp x6, x7, [x30, #48] stp x8, x9, [x30, #64] stp x10, x11, [x30, #80] stp x12, x13, [x30, #96] stp x14, x15, [x30, #112] stp x16, x17, [x30, #128] mov x0, x30 ldp x29, x30, [sp], #16 ret -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#119448): https://edk2.groups.io/g/devel/message/119448 Mute This Topic: https://groups.io/mt/106403741/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-