fix: efirt: use compiler ms_abi for EFI runtime calls on amd64

[guest-seuros]

Summary

The assembly trampoline in efi_rt_arch_call had a stack alignment bug for 5-argument EFI calls (GetVariable, SetVariable):

it computed max(N,4)*8 = 40 bytes of stack space, leaving RSP 8-byte aligned instead of the 16 required by the MS x64 ABI.

Firmware compiled with SSE (for example coreboot/EDK2) uses MOVAPS for callee-saved XMM register spills, causing #GP on every GetVariable/SetVariable call.

Replaced the assembly ABI conversion with efi_rt_dispatch(). The assembly trampoline is kept only for fault recovery, delegating the actual call to the C function.

[kib]

Could you try the patch below instead?

diff --git a/sys/amd64/amd64/efirt_support.S b/sys/amd64/amd64/efirt_support.S
index 98063ad561aa..54578f573750 100644
--- a/sys/amd64/amd64/efirt_support.S
+++ b/sys/amd64/amd64/efirt_support.S
@@ -58,6 +58,7 @@ ENTRY(efi_rt_arch_call)
 	cmovbl	%eax, %ecx
 	shll	$3, %ecx
 	subq	%rcx, %rsp
+	andq	$~0xf, %rsp
 
 	cmpl	$0, %ebx
 	jz	1f

[guest-seuros]

I will and report back.

Just to understand this file better, is there a reason why the assembly code is hand-rolled here instead of letting the compiler do it ?
(Not trying to pushback, 1 line of change is better than all this)

PS: i still need to test if there are similar choking points with other payloads.
I used the coreboot driver to notice isolate the divergence between different oses.

Another thing : It is acceptable to open a Diff to document that assembly code ? i used AI to understand the flow.

[guest-seuros]

@kib The patch you provided is solid. worked!

Should i update this diff or you handle it ?

[kib]

In D55662#1274066, @guest-seuros wrote:

I will and report back.

Just to understand this file better, is there a reason why the assembly code is hand-rolled here instead of letting the compiler do it ?
(Not trying to pushback, 1 line of change is better than all this)

The reason why there must be some assembly in any case is because the pcb_onfault mechanism that allows kernel to catch hardware exceptions is nothing more than jump, it replaces the %rip on fault with the value from pcb_onfault.
Since such jump does nothing to restore registers or any other components of the context, we must to know exactly what is the state of the code from where, and where to, we jump.

Then, in this specific case, since I wrote the assembler shim to be able to use pcb_onfault, it is not hard to do the call. What you proposed would work, but I prefer to avoid compiler extensions if possible. My code only needs standard C and amd64 ELF ABI to work, while using thunk in C to call into EFI means that ms_abi extension is used.

PS: i still need to test if there are similar choking points with other payloads.
I used the coreboot driver to notice isolate the divergence between different oses.

Another thing : It is acceptable to open a Diff to document that assembly code ? i used AI to understand the flow.

Nobody would hunt you for opening a diff, and you do not need to ask for permissions. Another thing, would be the diff with AI-generated content accepted. Our policy is that no AI content is allowed.
OTOH, thank you for being honest there.

In D55662#1274200, @guest-seuros wrote:

@kib The patch you provided is solid. worked!

Should i update this diff or you handle it ?

I will handle this myself.

[guest-seuros]

Nobody would hunt you for opening a diff, and you do not need to ask for permissions. Another thing, would be the diff with AI-generated content accepted. Our policy is that no AI content is allowed.
OTOH, thank you for being honest there.

Just to clarify: I do not generate code with AI. The only thing I use AI for is blog images, and sometimes reword my sentences.

The fix in the diff is mine. That is why I asked you for clarification about what i removed.

I did ask AI to explain what the assembly code might be doing, but I treat that explanation as potentially hallucination (it gaslighted me on libraries i authored decade ago).

My indent in asking was to be transparent that I am not completely sure about this fix.

Thank you for the explanation btw, learned new things.