git: 2a2a64c4b93f - main - vmm: validate icr value

From: Emmanuel Vadot <manu_at_FreeBSD.org>
Date: Fri, 14 Oct 2022 10:03:51 UTC
The branch main has been updated by manu:

URL: https://cgit.FreeBSD.org/src/commit/?id=2a2a64c4b93f1a135f62f54db54f4ec2f89f9127

commit 2a2a64c4b93f1a135f62f54db54f4ec2f89f9127
Author:     Corvin Köhne <c.koehne@beckhoff.com>
AuthorDate: 2022-10-12 09:19:32 +0000
Commit:     Emmanuel Vadot <manu@FreeBSD.org>
CommitDate: 2022-10-14 10:03:05 +0000

    vmm: validate icr value
    
    Not all combinations of icr values are allowed. Neither Intel nor AMD
    document what happens when an invalid value is written to the icr.
    Ignore the IPI. So, the guest will note that the IPI wasn't delivered.
    
    Reviewed by:            jhb
    Differential Revision:  https://reviews.freebsd.org/D36946
    Sponsored by:           Beckhoff Automation GmbH & Co. KG
---
 sys/amd64/vmm/io/vlapic.c | 91 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 88 insertions(+), 3 deletions(-)

diff --git a/sys/amd64/vmm/io/vlapic.c b/sys/amd64/vmm/io/vlapic.c
index c5db5af0ca95..8b547d9a9453 100644
--- a/sys/amd64/vmm/io/vlapic.c
+++ b/sys/amd64/vmm/io/vlapic.c
@@ -953,6 +953,86 @@ vlapic_get_cr8(struct vlapic *vlapic)
 	return (tpr >> 4);
 }
 
+static bool
+vlapic_is_icr_valid(uint64_t icrval)
+{
+	uint32_t mode = icrval & APIC_DELMODE_MASK;
+	uint32_t level = icrval & APIC_LEVEL_MASK;
+	uint32_t trigger = icrval & APIC_TRIGMOD_MASK;
+	uint32_t shorthand = icrval & APIC_DEST_MASK;
+
+	switch (mode) {
+	case APIC_DELMODE_FIXED:
+		if (trigger == APIC_TRIGMOD_EDGE)
+			return (true);
+		/*
+		 * AMD allows a level assert IPI and Intel converts a level
+		 * assert IPI into an edge IPI.
+		 */
+		if (trigger == APIC_TRIGMOD_LEVEL && level == APIC_LEVEL_ASSERT)
+			return (true);
+		break;
+	case APIC_DELMODE_LOWPRIO:
+	case APIC_DELMODE_SMI:
+	case APIC_DELMODE_NMI:
+	case APIC_DELMODE_INIT:
+		if (trigger == APIC_TRIGMOD_EDGE &&
+		    (shorthand == APIC_DEST_DESTFLD ||
+			shorthand == APIC_DEST_ALLESELF))
+			return (true);
+		/*
+		 * AMD allows a level assert IPI and Intel converts a level
+		 * assert IPI into an edge IPI.
+		 */
+		if (trigger == APIC_TRIGMOD_LEVEL &&
+		    level == APIC_LEVEL_ASSERT &&
+		    (shorthand == APIC_DEST_DESTFLD ||
+			shorthand == APIC_DEST_ALLESELF))
+			return (true);
+		/*
+		 * An level triggered deassert INIT is defined in the Intel
+		 * Multiprocessor Specification and the Intel Software Developer
+		 * Manual. Due to the MPS it's required to send a level assert
+		 * INIT to a cpu and then a level deassert INIT. Some operating
+		 * systems e.g. FreeBSD or Linux use that algorithm. According
+		 * to the SDM a level deassert INIT is only supported by Pentium
+		 * and P6 processors. It's always send to all cpus regardless of
+		 * the destination or shorthand field. It resets the arbitration
+		 * id register. This register is not software accessible and
+		 * only required for the APIC bus arbitration. So, the level
+		 * deassert INIT doesn't need any emulation and we should ignore
+		 * it. The SDM also defines that newer processors don't support
+		 * the level deassert INIT and it's not valid any more. As it's
+		 * defined for older systems, it can't be invalid per se.
+		 * Otherwise, backward compatibility would be broken. However,
+		 * when returning false here, it'll be ignored which is the
+		 * desired behaviour.
+		 */
+		if (mode == APIC_DELMODE_INIT &&
+		    trigger == APIC_TRIGMOD_LEVEL &&
+		    level == APIC_LEVEL_DEASSERT)
+			return (false);
+		break;
+	case APIC_DELMODE_STARTUP:
+		if (shorthand == APIC_DEST_DESTFLD ||
+		    shorthand == APIC_DEST_ALLESELF)
+			return (true);
+		break;
+	case APIC_DELMODE_RR:
+		/* Only available on AMD! */
+		if (trigger == APIC_TRIGMOD_EDGE &&
+		    shorthand == APIC_DEST_DESTFLD)
+			return (true);
+		break;
+	case APIC_DELMODE_RESV:
+		return (false);
+	default:
+		__assert_unreachable();
+	}
+
+	return (false);
+}
+
 int
 vlapic_icrlo_write_handler(struct vlapic *vlapic, bool *retu)
 {
@@ -998,6 +1078,14 @@ vlapic_icrlo_write_handler(struct vlapic *vlapic, bool *retu)
 		__assert_unreachable();
 	}
 
+	/*
+	 * Ignore invalid combinations of the icr.
+	 */
+	if (!vlapic_is_icr_valid(icrval)) {
+		VLAPIC_CTR1(vlapic, "Ignoring invalid ICR %016lx", icrval);
+		return (0);
+	}
+
 	/*
 	 * ipimask is a set of vCPUs needing userland handling of the current
 	 * IPI.
@@ -1031,9 +1119,6 @@ vlapic_icrlo_write_handler(struct vlapic *vlapic, bool *retu)
 
 		break;
 	case APIC_DELMODE_INIT:
-		if ((icrval & APIC_LEVEL_MASK) == APIC_LEVEL_DEASSERT)
-			break;
-
 		CPU_FOREACH_ISSET(i, &dmask) {
 			/*
 			 * Userland which doesn't support the IPI exit requires