[edk2-devel] ArmCallSmc() and SMCCC specification

[edk2-devel] ArmCallSmc() and SMCCC specification

[Marcin Juszkiewicz]

EDK2/ArmPkg/Library/ArmSmcLib has code to do SMC calls.

There are ArmCallSmc[0-3]() functions for up to 3 arguments/results and 
ArmCallSmc() function which can use 7 arguments and get 4 results back.

This implementation looks like version B (Nov 2016) of SMCCC 
specification [1] with one more register used.

1. https://developer.arm.com/documentation/den0028/b/


In 2020 we got version C of spec (and then D, E, F) which allows to use 
more registers:

 > Allow R4—R7 (SMC32/HVC32) to be used as result registers.
 > Allow X8—X17 to be used as parameter registers in SMC64/HVC64.
 > Allow X4—X17 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.


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.


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:

 > When an SMC32/HVC32 call is made from AArch32:
 > • A Function Identifier is passed in register R0.
 > • Arguments are passed in registers R1-R7.
 > • Results are returned in R0-R7.
 > • The registers R4-R7 must be preserved unless they contain results,
 >   as specified in the function definition.
 > • Registers R8-R14 are saved by the function that is called, and must
 >   be preserved over the SMC or HVC call.
 >
 > When an SMC64/HVC64 call is made from AArch64:
 > • A Function Identifier is passed in register W0.
 > • Arguments are passed in registers X1-X17.
 > • Results are returned in X0-X17.
 > • Registers X4-X17 must be preserved unless they contain results, as
 >   specified in the function definition.
 > • Registers X18-X30 and stack pointers SP_EL0 and SP_ELx are saved by 
 >   the function that is called, and must be preserved over the SMC or
 >   HVC call.


 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.


Now the question is: how to handle change?

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?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119408): https://edk2.groups.io/g/devel/message/119408
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmCallSmc() and SMCCC specification

[Leif Lindholm]

Hi Marcin.

A few high-level thoughs below.

On 2024-05-31 08:14, Marcin Juszkiewicz wrote:
> EDK2/ArmPkg/Library/ArmSmcLib has code to do SMC calls.
> 
> There are ArmCallSmc[0-3]() functions for up to 3 arguments/results and 
> ArmCallSmc() function which can use 7 arguments and get 4 results back.
> 
> This implementation looks like version B (Nov 2016) of SMCCC 
> specification [1] with one more register used.
> 
> 1. https://developer.arm.com/documentation/den0028/b/
> 
> In 2020 we got version C of spec (and then D, E, F) which allows to use 
> more registers:
> 
>  > Allow R4—R7 (SMC32/HVC32) to be used as result registers.
>  > Allow X8—X17 to be used as parameter registers in SMC64/HVC64.
>  > Allow X4—X17 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.
> 
> 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 common 
cases. I don't think we should worry about how those work for making any 
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 ones 
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...

/
     Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119431): https://edk2.groups.io/g/devel/message/119431
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmCallSmc() and SMCCC specification

[Marcin Juszkiewicz]

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—R7 (SMC32/HVC32) to be used as result registers.
>>  > Allow X8—X17 to be used as parameter registers in SMC64/HVC64.
>>  > Allow X4—X17 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 results
+
+
+  on SMC64/HVC64
+  - W0 is a function identifier, X1-X17 are arguments
+  - X0-X17 are results, X4-X17 must be preserved unless they contain results

  **/
  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 common 
> cases. I don't think we should worry about how those work for making any 
> 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 ones 
> 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).

diff --git a/ArmPkg/Library/ArmSmcLib/AArch64/ArmSmc.S b/ArmPkg/Library/ArmSmcLib/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 <AsmMacroIoLibV8.h>

  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]!

    // Load the SMC arguments values into the appropriate registers
    ldp   x6, x7, [x0, #48]
@@ -19,14 +21,18 @@ ASM_FUNC(ArmCallSmc)

    smc   #0

-  // Pop the ARM_SMC_ARGS structure address from the stack into x9
-  ldr   x9, [sp], #16
+  // Pop the ARM_SMC_ARGS structure address from the stack into x18
+  ldr   x18, [sp], #32

    // Store the SMC returned values into the ARM_SMC_ARGS structure.
-  // A SMC call can return up to 4 values - we do not need to store back x4-x7.
-  stp   x2, x3, [x9, #16]
-  stp   x0, x1, [x9, #0]
+  // A SMC call can return up to 18 values, let handle 8 for now
+  stp   x6, x7, [x18, #48]
+  stp   x4, x5, [x18, #32]
+  stp   x2, x3, [x18, #16]
+  stp   x0, x1, [x18, #0]

-  mov   x0, x9
+  mov   x0, x18
+  // restore x18
+  ldr   x18, [sp], #16

    ret


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#119446): https://edk2.groups.io/g/devel/message/119446
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]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] ArmCallSmc() and SMCCC specification

[Ard Biesheuvel]

On Tue, 4 Jun 2024 at 14:37, Marcin Juszkiewicz
<marcin.juszkiewicz@linaro.org> 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—R7 (SMC32/HVC32) to be used as result registers.
> >>  > Allow X8—X17 to be used as parameter registers in SMC64/HVC64.
> >>  > Allow X4—X17 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 results
> +
> +
> +  on SMC64/HVC64
> +  - W0 is a function identifier, X1-X17 are arguments
> +  - X0-X17 are results, X4-X17 must be preserved unless they contain results
>
>   **/
>   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 common
> > cases. I don't think we should worry about how those work for making any
> > 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 ones
> > 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/ArmSmcLib/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 <AsmMacroIoLibV8.h>
>
>   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


-=-=-=-=-=-=-=-=-=-=-=-
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]
-=-=-=-=-=-=-=-=-=-=-=-