NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: port-arm/60655: vmt(4) probes the VMware backdoor unconditionally on aarch64 and panics the kernel on hypervisors that do not implement it



The following reply was made to PR port-arm/60655; it has been noted by GNATS.

From: zakinko%snowrabbit.org@localhost
To: gnats-bugs%NetBSD.org@localhost
Cc: port-arm-maintainer%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost,
	gnats-admin%NetBSD.org@localhost, skrll%NetBSD.org@localhost, zakinko%snowrabbit.org@localhost
Subject: Re: port-arm/60655: vmt(4) probes the VMware backdoor unconditionally on aarch64 and panics the kernel on hypervisors that do not implement it
Date: Sat,  5 Sep 2026 10:27:02 +0900 (JST)

 You were right that the version is the variable, and it narrows further than
 that.  It is one commit, and it can be shown without rebuilding anything:
 
 	qemu 11.1.0, -M virt (= virt-11.1)		panics
 	qemu 11.1.0, -M virt-11.0			boots
 	qemu 11.1.0, -M virt,kernel-irqchip=off		boots
 	qemu 10.2.0, -M virt				boots
 
 Same host, same NetBSD 11.0 GENERIC64 arm64.img, same command line otherwise.
 The commit is 37863fff59 ("hvf: arm: enable vGIC by default for virt-11.1 and
 later"), which makes kernel-irqchip default to on for the new machine type.
 
 With the vGIC in play the MDCCSR_EL0 read is trapped out to QEMU, and QEMU's
 hvf path has no case for that register, so it injects an undefined
 instruction.  Its own trace point says so, at the pc from the panic:
 
 	hvf_unhandled_sysreg_read unhandled sysreg read at
 	  pc=0xffffc000003fa318: 0x0020c002 (op0=2 op1=3 crn=0 crm=1 op2=0)
 
 The TCG path does not do this: debug_helper.c defines MDCCSR_EL0 as
 ARM_CP_CONST with resetvalue 0, and the comment there says it is RAZ
 specifically to avoid spurious SIGILLs "as the support cannot be probed for".
 I have sent a patch to qemu-devel making hvf do the same, under the subject
 "target/arm/hvf: implement MDCCSR_EL0 as RAZ"; it has not appeared in the
 archive yet.  It is not fixed upstream either: master is 1168 commits past
 v11.1.0 and none of them touch accel/hvf or target/arm/hvf, and 11.1.1 has
 none.
 
 That said, I do not think this is only QEMU's problem, and I would like your
 opinion on the NetBSD side.
 
 A hypervisor is free to trap MDCCSR_EL0 and refuse it, and QEMU's own comment
 is the point: the guest cannot ask beforehand.  The register is
 architecturally mandatory, so testing for its presence answers "yes" and the
 probe dies all the same.  On x86 the same driver is safe by construction --
 the backdoor there is an inl from an unused I/O port, which cannot fault -- so
 the exposure is specific to aarch64, where the probe is an instruction a
 hypervisor can decline.
 
 The machinery to survive it is already in the tree.  trap_el1h_error() does
 exactly this for SErrors, with a comment saying it would otherwise panic
 unconditionally but bus_space_peek(9) has to probe.  trap_el1h_sync() has no
 such escape, so ESR_EC_UNKNOWN falls to the default arm and panics.
 
 Built from trunk, cross-compiled for evbarm-aarch64, -Werror -Wall clean.
 With that kernel, a stock unpatched QEMU 11.1.0 on -M virt -accel hvf reaches
 login, while the trace point above still fires once -- so the kernel is
 surviving the injected undefined instruction, rather than the register having
 become readable.
 
 One thing I am unsure of, and would rather raise myself: this reuses the
 onfault machinery, which copyin/copyout also use.  A genuine kernel bug that
 executed an undefined instruction while an onfault was set would now return
 EFAULT instead of panicking.  A dedicated flag would be narrower.  I went this
 way because trap_el1h_error() already took the broad approach for the same
 kind of probe, but if you would rather have something narrower I will redo it.
 
 --- sys/arch/aarch64/aarch64/trap.c.orig
 +++ sys/arch/aarch64/aarch64/trap.c
 @@ -244,6 +244,25 @@
  		panic("No debugger in kernel");
  #endif
  		break;
 +
 +	case ESR_EC_UNKNOWN:
 +		/*
 +		 * An undefined instruction in the kernel is normally fatal,
 +		 * but a driver may have to execute one on purpose: an
 +		 * instruction that some hypervisors answer and others refuse,
 +		 * with no way to ask beforehand.  vmt(4) does this to look
 +		 * for the VMware backdoor.  Let such a probe recover the way
 +		 * bus_space_peek(9) recovers from an SError, see
 +		 * trap_el1h_error() below.
 +		 */
 +		if (curcpu()->ci_intr_depth == 0) {
 +			struct faultbuf * const fb = cpu_disable_onfault();
 +			if (fb != NULL) {
 +				cpu_jump_onfault(tf, fb, EFAULT);
 +				break;
 +			}
 +		}
 +		goto fatal;
  
  	case ESR_EC_FP_ACCESS:
  		if ((curlwp->l_flag & (LW_SYSTEM|LW_SYSTEM_FPU)) ==
 @@ -258,6 +277,7 @@
  	case ESR_EC_ILL_STATE:
  	case ESR_EC_BTE_A64:
  	default:
 + fatal:
  		panic("Trap: fatal %s: pc=%016" PRIx64 " sp=%016" PRIx64
  		    " esr=%08x", eclass_trapname(eclass), tf->tf_pc, tf->tf_sp,
  		    esr);
 --- sys/arch/aarch64/aarch64/vmt.c.orig
 +++ sys/arch/aarch64/aarch64/vmt.c
 @@ -33,6 +33,8 @@
  #include <sys/device.h>
  #include <sys/module.h>
  
 +#include <aarch64/machdep.h>
 +
  #include <dev/vmt/vmtreg.h>
  #include <dev/vmt/vmtvar.h>
  
 @@ -48,11 +50,27 @@
  static int
  vmt_match(device_t parent, cfdata_t match, void *aux)
  {
 +	struct faultbuf fb;
 +	int rv;
 +
  	/* vmt should not attach to more than a single CPU. */
  	if (vmt_attached)
  		return 0;
  
 -	return vmt_probe();
 +	/*
 +	 * On aarch64 the backdoor is a read of MDCCSR_EL0 with a magic
 +	 * value in x7.  The register is mandatory, but a hypervisor is
 +	 * free to trap it and answer with an undefined instruction, and
 +	 * the guest cannot ask which one it is on beforehand.  QEMU 11.1
 +	 * with the Apple vGIC does exactly that, and the probe took the
 +	 * whole kernel down with it.  Catch the trap and report no vmt.
 +	 */
 +	if (cpu_set_onfault(&fb) != 0)
 +		return 0;
 +	rv = vmt_probe();
 +	cpu_unset_onfault();
 +
 +	return rv;
  }
  
  static void
 



Home | Main Index | Thread Index | Old Index