Index | Thread | Search

From:
Hans-Jörg Höxer <hshoexer@genua.de>
Subject:
SEV-ES guest: Fix GHCB trap frame synchronisation for 32-bit values
To:
<tech@openbsd.org>
Date:
Tue, 8 Jul 2025 11:45:14 +0200

Download raw body.

Thread
Hi,

In 64-bit mode, when performing 32-bit operations with a GPR destination,
the 32-bit value gets zero extended to the full 64-bit destination size.
This means, unlike 8-bit or 16-bit values, the upper 32 bits of the
destination register are not retained for 32-bit values.

Therefore, when syncing values back to the stack frame, use a "all 1"
mask for 32-bit values.

Thus duplicate mask array into two, one for syncing out from trap frame
into GHCB and one for syncing back in from GHCB to the trap frame.  In the
latter adjust mask for 32-bit values.

This was pointed out by Sebastian Sturm (ssturm@genua.de).

Take care,
Hans-Joerg

---------------------
commit 7176ba3ed665f7e9ff2e12289331108bc5995f26
Author: Hans-Joerg Hoexer <hshoexer@genua.de>
Date:   Mon Jul 7 13:54:22 2025 +0200

    SEV-ES guest: Fix GHCB trap frame synchronisation for 32-bit values
    
    In 64-bit mode, when performing 32-bit operations with a GPR
    destination, the 32-bit value gets zero extended to the full 64-bit
    destination size.  This means, unlike 8-bit or 16-bit values, the
    upper 32 bits of the destination register are not retained for
    32-bit values.
    
    Therefore, when syncing values back to the stack frame, use a "all
    1" mask for 32-bit values.
    
    Thus duplicate mask array into two, one for syncing out from trap
    frame into GHCB and one for syncing back in from GHCB to the trap
    frame.  In the latter adjust mask for 32-bit values.
    
    This was pointed out by Sebastian Sturm (ssturm@genua.de).

diff --git a/sys/arch/amd64/amd64/ghcb.c b/sys/arch/amd64/amd64/ghcb.c
index c2ce3bab498..904680b0400 100644
--- a/sys/arch/amd64/amd64/ghcb.c
+++ b/sys/arch/amd64/amd64/ghcb.c
@@ -22,12 +22,27 @@
 #include <machine/frame.h>
 #include <machine/ghcb.h>
 
-/* Mask for adjusting GPR sizes. */
-const uint64_t ghcb_sz_masks[] = {
+/* Masks for adjusting GPR sizes. */
+const uint64_t ghcb_sz_masks_out[] = {
     0x00000000000000ffULL, 0x000000000000ffffULL,
     0x00000000ffffffffULL, 0xffffffffffffffffULL
 };
 
+/*
+ * In 64-bit mode, when performing 32-bit operations with a GPR
+ * destination, the 32-bit value gets zero extended to the full
+ * 64-bit destination size.  This means, unlike 8-bit or 16-bit
+ * values, the upper 32 bits of the destination register are not
+ * retained for 32-bit values.
+ *
+ * Therefore, when syncing values back to the stack frame, use a
+ * "all 1" mask for 32-bit values.
+ */
+const uint64_t ghcb_sz_masks_in[] = {
+    0x00000000000000ffULL, 0x000000000000ffffULL,
+    0xffffffffffffffffULL, 0xffffffffffffffffULL
+};
+
 vaddr_t ghcb_vaddr;
 paddr_t ghcb_paddr;
 
@@ -179,13 +194,13 @@ ghcb_sync_out(struct trapframe *frame, uint64_t exitcode, uint64_t exitinfo1,
 	    sizeof(ghcb->valid_bitmap));
 
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_RAX))
-		ghcb->v_rax = frame->tf_rax & ghcb_sz_masks[gsout->sz_a];
+		ghcb->v_rax = frame->tf_rax & ghcb_sz_masks_out[gsout->sz_a];
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_RBX))
-		ghcb->v_rbx = frame->tf_rbx & ghcb_sz_masks[gsout->sz_b];
+		ghcb->v_rbx = frame->tf_rbx & ghcb_sz_masks_out[gsout->sz_b];
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_RCX))
-		ghcb->v_rcx = frame->tf_rcx & ghcb_sz_masks[gsout->sz_c];
+		ghcb->v_rcx = frame->tf_rcx & ghcb_sz_masks_out[gsout->sz_c];
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_RDX))
-		ghcb->v_rdx = frame->tf_rdx & ghcb_sz_masks[gsout->sz_d];
+		ghcb->v_rdx = frame->tf_rdx & ghcb_sz_masks_out[gsout->sz_d];
 
 	if (ghcb_valbm_isset(gsout->valid_bitmap, GHCB_SW_EXITCODE))
 		ghcb->v_sw_exitcode = exitcode;
@@ -206,20 +221,20 @@ ghcb_sync_in(struct trapframe *frame, struct ghcb_sa *ghcb,
     struct ghcb_sync *gsin)
 {
 	if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_RAX)) {
-		frame->tf_rax &= ~ghcb_sz_masks[gsin->sz_a];
-		frame->tf_rax |= (ghcb->v_rax & ghcb_sz_masks[gsin->sz_a]);
+		frame->tf_rax &= ~ghcb_sz_masks_in[gsin->sz_a];
+		frame->tf_rax |= (ghcb->v_rax & ghcb_sz_masks_in[gsin->sz_a]);
 	}
 	if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_RBX)) {
-		frame->tf_rbx &= ~ghcb_sz_masks[gsin->sz_b];
-		frame->tf_rbx |= (ghcb->v_rbx & ghcb_sz_masks[gsin->sz_b]);
+		frame->tf_rbx &= ~ghcb_sz_masks_in[gsin->sz_b];
+		frame->tf_rbx |= (ghcb->v_rbx & ghcb_sz_masks_in[gsin->sz_b]);
 	}
 	if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_RCX)) {
-		frame->tf_rcx &= ~ghcb_sz_masks[gsin->sz_c];
-		frame->tf_rcx |= (ghcb->v_rcx & ghcb_sz_masks[gsin->sz_c]);
+		frame->tf_rcx &= ~ghcb_sz_masks_in[gsin->sz_c];
+		frame->tf_rcx |= (ghcb->v_rcx & ghcb_sz_masks_in[gsin->sz_c]);
 	}
 	if (ghcb_valbm_isset(gsin->valid_bitmap, GHCB_RDX)) {
-		frame->tf_rdx &= ~ghcb_sz_masks[gsin->sz_d];
-		frame->tf_rdx |= (ghcb->v_rdx & ghcb_sz_masks[gsin->sz_d]);
+		frame->tf_rdx &= ~ghcb_sz_masks_in[gsin->sz_d];
+		frame->tf_rdx |= (ghcb->v_rdx & ghcb_sz_masks_in[gsin->sz_d]);
 	}
 
 	ghcb_clear(ghcb);