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]

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();
 }