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

["zakinko%snowrabbit.org@localhost via gnats" <gnats-admin%NetBSD.org@localhost>]

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, nick.hudson%gmx.co.uk@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: Mon,  7 Sep 2026 02:53:44 +0900 (JST)

 Thanks for catching that -- I had missed it.  I read the pointer as:
 HypervisorId only exists from FADT revision 6, so the revision (and the
 length) should be checked before the field is read.  Is that the right
 understanding?
 
 Two things I went on to do.  I am not confident about either, so I would
 be grateful for your view:
 
 - The FADT reading moved into a small helper in dev/acpi,
   acpi_fadt_hypervisor_id(), so that the next user of the field need not
   copy it.
 
 - vmt_match() now probes only when the firmware names VMware, rather
   than skipping only when it names something else.  The difference shows
   on a hypervisor that traps MDCCSR_EL0 but leaves the field zero: the
   earlier form still panics there (I checked with a QEMU 11.1.0 patched
   to write zeros).  The ACPI 6.0 note for the field reads to me as
   "probe only on a vendor you recognise", but I may be reading too much
   into it -- if you would rather keep the old behaviour on a zero field,
   it is a single condition to flip.
 
 Rebuilt from trunk, -Werror -Wall clean.  Boots on stock QEMU 11.1 (hvf;
 "QEMU", vmt does not attach), on the zero-field QEMU, and on VMware
 Fusion 26 ("VMware", vmt0 attaches and answers the host).
 
 --- sys/dev/acpi/acpi_util.h.orig
 +++ sys/dev/acpi/acpi_util.h
 @@ -114,4 +114,6 @@
  ACPI_STATUS	 acpi_claim_childdevs(device_t, struct acpi_devnode *,
  			const char *);
  
 +bool		 acpi_fadt_hypervisor_id(char *, size_t);
 +
  #endif	/* !_SYS_DEV_ACPI_ACPI_UTIL_H */
 --- sys/dev/acpi/acpi_util.c.orig
 +++ sys/dev/acpi/acpi_util.c
 @@ -1422,3 +1422,42 @@
  
  	return AE_OK;
  }
 +
 +/*
 + * Return the Hypervisor Vendor Identity from the FADT as a NUL-terminated
 + * string, or false if there is none to report.
 + *
 + * ACPI 6.0 added the field (5.2.9, FADT Format, offset 268): eight bytes
 + * that name the hypervisor vendor, "usually following the name of the
 + * hypervisor product", with no version information in it.  QEMU writes
 + * "QEMU" and VMware writes "VMware", zero padded.  Firmware places zero
 + * bytes in it when no hypervisor is present, so zero is reported as
 + * absence.  The specification's own note is that a guest "can consult it
 + * and act on the result, based on whether it recognized the vendor",
 + * which is what a caller does with the string.
 + *
 + * The field only exists from FADT revision 6.  ACPICA zeroes its copy of
 + * the FADT before filling it from the firmware's table, so on an older,
 + * shorter table the field reads as zero rather than as garbage.  That is
 + * a property of the copy and not something to lean on, so both the
 + * revision and the length are checked before the field is read, as
 + * open-vm-tools does.  The bytes are copied in table order rather than
 + * through the UINT64, so that this reads the same on big-endian machines.
 + */
 +bool
 +acpi_fadt_hypervisor_id(char *buf, size_t buflen)
 +{
 +	const size_t len = sizeof(AcpiGbl_FADT.HypervisorId);
 +
 +	KASSERT(buflen > len);
 +
 +	if (AcpiGbl_FADT.Header.Revision < 6 ||
 +	    AcpiGbl_FADT.Header.Length < ACPI_FADT_V6_SIZE)
 +		return false;
 +	if (AcpiGbl_FADT.HypervisorId == 0)
 +		return false;
 +
 +	memcpy(buf, &AcpiGbl_FADT.HypervisorId, len);
 +	buf[len] = '\0';
 +	return true;
 +}
 --- sys/arch/aarch64/aarch64/vmt.c.orig
 +++ sys/arch/aarch64/aarch64/vmt.c
 @@ -32,7 +32,16 @@
  
  #include <sys/device.h>
  #include <sys/module.h>
 +#include <sys/systm.h>
  
 +#ifdef _KERNEL_OPT
 +#include "acpica.h"
 +#endif
 +
 +#if NACPICA > 0
 +#include <dev/acpi/acpivar.h>
 +#endif
 +
  #include <dev/vmt/vmtreg.h>
  #include <dev/vmt/vmtvar.h>
  
 @@ -48,10 +57,29 @@
  static int
  vmt_match(device_t parent, cfdata_t match, void *aux)
  {
 +#if NACPICA > 0
 +	char hv[sizeof(AcpiGbl_FADT.HypervisorId) + 1];
 +#endif
 +
  	/* vmt should not attach to more than a single CPU. */
  	if (vmt_attached)
  		return 0;
  
 +#if NACPICA > 0
 +	/*
 +	 * The backdoor is a read of MDCCSR_EL0 with a magic value in x7.  A
 +	 * hypervisor other than VMware is free to answer that read with an
 +	 * undefined instruction trap, which is fatal in kernel mode; QEMU
 +	 * does so once the Apple vGIC is in use.  Probe only when the
 +	 * firmware names VMware; a hypervisor that names something else,
 +	 * or nothing, is not one whose API this driver knows.  See
 +	 * port-arm/60655.
 +	 */
 +	if (!acpi_fadt_hypervisor_id(hv, sizeof(hv)) ||
 +	    strncmp(hv, "VMware", 6) != 0)
 +		return 0;
 +#endif
 +
  	return vmt_probe();
  }