On Thu, Nov 27, 2025 at 03:46:19PM +0100, hshoexer wrote:
> Hi,
>
> in SEV-ES mode, guest userland is allowed to execute the vmgexit
> instruction, although it has no control over the GHCB.  Therefore,
> it is important that the GHCB does not contain a valid request after
> use.
>
> In all "vmgexit paths" the GHCB is cleared by ghcb_sync_in() (it
> calls ghcb_clear()) after returning from the hypervisor back into
> the guest.
>
> However, in _ghcb_mem_rw() I missed this when requesting MMIO writes
> from the hypervisor.  The diff below corrects this.
>
> I want to keep this pattern in all vmgexit paths:
>
> 	ghcb_sync_out
> 	vmgexit
> 	ghcb_verify_bm
> 	ghcb_sync_in
>
> Therefore, I shuffled some code around instead of just calling
> vmgexit_clear() in the else branch.
>
> ok?  thoughts?
>

good catch. ok mlarkin.

> Take care,
> HJ.
>
> -----------------------------------------------------------------------
> diff --git a/sys/arch/amd64/amd64/ghcb.c b/sys/arch/amd64/amd64/ghcb.c
> index 2b0fa809570..b6aa2840228 100644
> --- a/sys/arch/amd64/amd64/ghcb.c
> +++ b/sys/arch/amd64/amd64/ghcb.c
> @@ -271,7 +271,7 @@ _ghcb_mem_rw(vaddr_t addr, int valsz, void *val, bool read)
>  	struct ghcb_sync	 syncout, syncin;
>  	struct ghcb_sa		*ghcb;
>  	unsigned long		 s;
> -	struct ghcb_extra_regs	 ghcb_regs;
> +	struct ghcb_extra_regs	 ghcb_regs, *pregs = NULL;
>
>  	KASSERT(val != NULL);
>
> @@ -334,15 +334,15 @@ _ghcb_mem_rw(vaddr_t addr, int valsz, void *val, bool read)
>  		panic("invalid hypervisor response");
>  	}
>
> -	memset(&ghcb_regs, 0, sizeof(ghcb_regs));
> -
>  	if (read) {
> +		memset(&ghcb_regs, 0, sizeof(ghcb_regs));
>  		ghcb_regs.data = val;
>  		ghcb_regs.data_sz = size;
> -
> -		ghcb_sync_in(NULL, &ghcb_regs, ghcb, &syncin);
> +		pregs = &ghcb_regs;
>  	}
>
> +	ghcb_sync_in(NULL, pregs, ghcb, &syncin);
> +
>  	intr_restore(s);
>  }
>
>