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, 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: Fri, 11 Sep 2026 07:34:13 +0900 (JST)

 Thanks for looking at it again.
 
 I have taken out the sentence about a hypervisor being free to trap
 the read.  A hypervisor that does not give a RAZ MDCCSR_EL0 is simply
 broken, and QEMU was; the comment in vmt_match() now says that and
 notes that QEMU has fixed it (Peter Maydell applied the patch to
 target-arm.next on 8 September with a cc:stable tag).  Nothing else in
 the diff changed.
 
 I kept the helper since it seemed acceptable to you.  If you would
 rather have the FADT read inline in vmt_match(), please say so and I
 will send that form instead.
 
 Rebuilt GENERIC64 from the same trunk tree with the comment change,
 -Werror -Wall clean, and booted it on stock QEMU 11.1.0 under hvf: it
 reaches login, vmt does not attach, and QEMU reports no unhandled
 sysreg read.  The zero-field QEMU and VMware Fusion runs reported
 before were with this code minus the comment, so I did not repeat
 them.
 
 --- sys/dev/acpi/acpi_util.h.orig
 +++ sys/dev/acpi/acpi_util.h
 @@ -114,4 +114,6 @@ ACPI_STATUS	 acpi_dsm_query(ACPI_HANDLE, uint8_t *, ACPI_INTEGER,
  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 @@ acpi_claim_childdevs(device_t dev, struct acpi_devnode *devnode,
  
  	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,6 +32,15 @@
  
  #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 bool vmt_attached = false;
  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.
 +	 * Probe only when the firmware names VMware; a hypervisor that
 +	 * names something else, or nothing, is not one whose API this
 +	 * driver knows, and there is nothing to find.  (QEMU 11.1 under
 +	 * hvf also trapped the read as an undefined instruction, which is
 +	 * fatal in kernel mode.  That was a QEMU bug, since fixed.)  See
 +	 * port-arm/60655.
 +	 */
 +	if (!acpi_fadt_hypervisor_id(hv, sizeof(hv)) ||
 +	    strncmp(hv, "VMware", 6) != 0)
 +		return 0;
 +#endif
 +
  	return vmt_probe();
  }
  
 



Home | Main Index | Thread Index | Old Index