Date: Thu, 22 Dec 2022 09:22:45 GMT From: =?utf-8?Q?Corvin=20K=C3=B6hne?= <corvink@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org Subject: git: 168726513cdf - stable/13 - vmm: validate icr value Message-ID: <202212220922.2BM9MjdY038691@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by corvink: URL: https://cgit.FreeBSD.org/src/commit/?id=168726513cdf93a18d2dae3862dcffbd8d7a6ccc commit 168726513cdf93a18d2dae3862dcffbd8d7a6ccc Author: Corvin Köhne <c.koehne@beckhoff.com> AuthorDate: 2022-10-12 09:19:32 +0000 Commit: Corvin Köhne <corvink@FreeBSD.org> CommitDate: 2022-12-22 09:12:22 +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 (cherry picked from commit 2a2a64c4b93f1a135f62f54db54f4ec2f89f9127) --- 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 d660b0a3f195..e1a57c4abcfc 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202212220922.2BM9MjdY038691>