On Tue, Jul 01, 2025 at 10:36:33AM +0200, Hans-Jörg Höxer wrote:
> Hi,
>
> this diff restricts #VC trap handling.
>
> Take care,
> HJ.
>

see below

> -----------
> commit bbf8ca5bfcd41771b0ecac3597bf033a603a54c3
> Author: Hans-Joerg Hoexer <hshoexer@genua.de>
> Date:   Thu Mar 20 17:17:31 2025 +0100
>
>     SEV-ES guest: In vctrap() only allow CPUID also from userspace
>
>     CPUID is the only instruction we allow to raise a #VC exception also
>     from user space.  All other instructions are limited to raise #VC
>     from kernel space only.
>
>     With respect to rdmsr/wrmsr this is an additional safe-guard, as
>     these two instructions will raise a #GP anyway when the CPL is
>     greater than 0.
>
>     With respect to in/out userland can be allowed to access IO ports.
>     Howerver, for now we do not want to support this.
>
> diff --git a/sys/arch/amd64/amd64/trap.c b/sys/arch/amd64/amd64/trap.c
> index f68cb2c90d5..9c532846c41 100644
> --- a/sys/arch/amd64/amd64/trap.c
> +++ b/sys/arch/amd64/amd64/trap.c
> @@ -97,7 +97,7 @@
>
>  int	upageflttrap(struct trapframe *, uint64_t);
>  int	kpageflttrap(struct trapframe *, uint64_t);
> -int	vctrap(struct trapframe *);
> +int	vctrap(struct trapframe *, int);
>  void	kerntrap(struct trapframe *);
>  void	usertrap(struct trapframe *);
>  void	ast(struct trapframe *);
> @@ -302,7 +302,7 @@ kpageflttrap(struct trapframe *frame, uint64_t cr2)
>  }
>
>  int
> -vctrap(struct trapframe *frame)
> +vctrap(struct trapframe *frame, int user)
>  {
>  	uint64_t	 sw_exitcode, sw_exitinfo1, sw_exitinfo2;
>  	uint8_t		*rip = (uint8_t *)(frame->tf_rip);
> @@ -340,6 +340,8 @@ vctrap(struct trapframe *frame)
>  		frame->tf_rip += 2;
>  		break;
>  	case SVM_VMEXIT_MSR: {
> +		if (user)
> +			return 0;	/* not allowed from userspace */
>  		if (*rip == 0x0f && *(rip + 1) == 0x30) {
>  			/* WRMSR */
>  			ghcb_sync_val(GHCB_RAX, GHCB_SZ32, &syncout);
> @@ -357,6 +359,8 @@ vctrap(struct trapframe *frame)
>  		break;
>  	    }
>  	case SVM_VMEXIT_IOIO: {
> +		if (user)
> +			return 0;	/* not allowed from userspace */

Does this not break cases that use the IOPB for port access from usermode?

>  		switch (*rip) {
>  		case 0x66: {
>  			switch (*(rip + 1)) {
> @@ -505,7 +509,7 @@ kerntrap(struct trapframe *frame)
>  #endif /* NISA > 0 */
>
>  	case T_VC:
> -		if (vctrap(frame))
> +		if (vctrap(frame, 0))
>  			return;
>  		goto we_re_toast;
>  	}
> @@ -588,9 +592,11 @@ usertrap(struct trapframe *frame)
>  		    : ILL_BADSTK;
>  		break;
>  	case T_VC:
> -		vctrap(frame);
> -		goto out;
> -
> +		if (vctrap(frame, 1))
> +			goto out;
> +		sig = SIGILL;
> +		code = ILL_PRVOPC;
> +		break;
>  	case T_PAGEFLT:			/* page fault */
>  		if (!uvm_map_inentry(p, &p->p_spinentry, PROC_STACK(p),
>  		    "[%s]%d/%d sp=%lx inside %lx-%lx: not MAP_STACK\n",
> diff --git a/sys/arch/amd64/amd64/vector.S b/sys/arch/amd64/amd64/vector.S
> index 59823862c18..4181ab216d0 100644
> --- a/sys/arch/amd64/amd64/vector.S
> +++ b/sys/arch/amd64/amd64/vector.S
> @@ -553,6 +553,7 @@ IDTVEC(vctrap_early)
>  	TRAP_ENTRY_KERN		/* early #VC has to be in kernel mode */
>  	cld
>  	movq	%rsp, %rdi
> +	movq	$0x0, %rsi
>  	call	vctrap
>  	movq	$0,-8(%rsp)
>  	INTRFASTEXIT