From: Oliver Upton <oupton@google.com> To: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu Cc: Marc Zyngier <maz@kernel.org>, Peter Shier <pshier@google.com>, Ricardo Koller <ricarkol@google.com>, Jing Zhang <jingzhangos@google.com>, Raghavendra Rao Anata <rananta@google.com>, James Morse <james.morse@arm.com>, Alexandru Elisei <alexandru.elisei@arm.com>, Suzuki K Poulose <suzuki.poulose@arm.com>, Andrew Jones <drjones@redhat.com>, Oliver Upton <oupton@google.com> Subject: [PATCH v2 0/4] KVM: arm64: Fix some races in CPU_ON PSCI call Date: Wed, 18 Aug 2021 20:21:29 +0000 [thread overview] Message-ID: <20210818202133.1106786-1-oupton@google.com> (raw) The CPU_ON PSCI call requires careful coordination between vCPUs in KVM, as it allows callers to send a payload (pc, context id) to another vCPU to start execution. There are a couple of races in the handling of CPU_ON: - KVM uses the kvm->lock to serialize the write-side of a vCPU's reset state. However, kvm_vcpu_reset() doesn't take the lock on the read-size, meaning the vCPU could be reset with interleaved state from two separate CPU_ON calls. - If a targeted vCPU never enters the guest again (say, the VMM was getting ready to migrate), then the reset payload is never actually folded in to the vCPU's registers. Despite this, the calling vCPU has already made the target runnable. Migrating the target vCPU at this time will result in execution from its old PC, not execution coming out of the reset state at the requested address. Patch 1 addresses the read-side race in KVM's CPU_ON implementation. Patch 2 fixes the KVM/VMM race by resetting a vCPU (if requested) whenever the VMM tries to read out its registers. Gross, but it avoids exposing the vcpu_reset_state structure through some other UAPI. That is undesirable, as we really are only trying to paper over the implementation details of PSCI in KVM. Patch 3 is unrelated, and is based on my own reading of the PSCI specification. In short, if you invoke PSCI_ON from AArch64, then you must set the Aff3 bits. This is impossible if you use the 32 bit function, since the arguments are only 32 bits. Just return INVALID_PARAMS to the guest in this case. This series cleanly applies to v5.14-rc6 The series was tested with the included KVM selftest on an Ampere Mt. Jade system. Broken behavior was verified using the same test on v5.14-rc6, sans this series. v1: http://lore.kernel.org/r/20210818085047.1005285-1-oupton@google.com v1 -> v2: - avoid memcpy for reading reset state (Marc) - promote reset_state.reset = false to WRITE_ONCE() (Marc) - rephrase comment, commit msg (Marc) - cite the PSCI spec precisely (Marc) - drop unnecessary mask-down to 32 bits in CPU_ON (Marc) - rebased on top of v5.14-rc6 Oliver Upton (4): KVM: arm64: Fix read-side race on updates to vcpu reset state KVM: arm64: Handle PSCI resets before userspace touches vCPU state KVM: arm64: Enforce reserved bits for PSCI target affinities selftests: KVM: Introduce psci_cpu_on_test arch/arm64/kvm/arm.c | 9 ++ arch/arm64/kvm/psci.c | 20 ++- arch/arm64/kvm/reset.c | 16 ++- tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/aarch64/psci_cpu_on_test.c | 121 ++++++++++++++++++ .../selftests/kvm/include/aarch64/processor.h | 3 + 7 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c -- 2.33.0.rc1.237.g0d66db33f3-goog
WARNING: multiple messages have this Message-ID (diff)
From: Oliver Upton <oupton@google.com> To: kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu Cc: Marc Zyngier <maz@kernel.org>, Peter Shier <pshier@google.com>, Raghavendra Rao Anata <rananta@google.com> Subject: [PATCH v2 0/4] KVM: arm64: Fix some races in CPU_ON PSCI call Date: Wed, 18 Aug 2021 20:21:29 +0000 [thread overview] Message-ID: <20210818202133.1106786-1-oupton@google.com> (raw) The CPU_ON PSCI call requires careful coordination between vCPUs in KVM, as it allows callers to send a payload (pc, context id) to another vCPU to start execution. There are a couple of races in the handling of CPU_ON: - KVM uses the kvm->lock to serialize the write-side of a vCPU's reset state. However, kvm_vcpu_reset() doesn't take the lock on the read-size, meaning the vCPU could be reset with interleaved state from two separate CPU_ON calls. - If a targeted vCPU never enters the guest again (say, the VMM was getting ready to migrate), then the reset payload is never actually folded in to the vCPU's registers. Despite this, the calling vCPU has already made the target runnable. Migrating the target vCPU at this time will result in execution from its old PC, not execution coming out of the reset state at the requested address. Patch 1 addresses the read-side race in KVM's CPU_ON implementation. Patch 2 fixes the KVM/VMM race by resetting a vCPU (if requested) whenever the VMM tries to read out its registers. Gross, but it avoids exposing the vcpu_reset_state structure through some other UAPI. That is undesirable, as we really are only trying to paper over the implementation details of PSCI in KVM. Patch 3 is unrelated, and is based on my own reading of the PSCI specification. In short, if you invoke PSCI_ON from AArch64, then you must set the Aff3 bits. This is impossible if you use the 32 bit function, since the arguments are only 32 bits. Just return INVALID_PARAMS to the guest in this case. This series cleanly applies to v5.14-rc6 The series was tested with the included KVM selftest on an Ampere Mt. Jade system. Broken behavior was verified using the same test on v5.14-rc6, sans this series. v1: http://lore.kernel.org/r/20210818085047.1005285-1-oupton@google.com v1 -> v2: - avoid memcpy for reading reset state (Marc) - promote reset_state.reset = false to WRITE_ONCE() (Marc) - rephrase comment, commit msg (Marc) - cite the PSCI spec precisely (Marc) - drop unnecessary mask-down to 32 bits in CPU_ON (Marc) - rebased on top of v5.14-rc6 Oliver Upton (4): KVM: arm64: Fix read-side race on updates to vcpu reset state KVM: arm64: Handle PSCI resets before userspace touches vCPU state KVM: arm64: Enforce reserved bits for PSCI target affinities selftests: KVM: Introduce psci_cpu_on_test arch/arm64/kvm/arm.c | 9 ++ arch/arm64/kvm/psci.c | 20 ++- arch/arm64/kvm/reset.c | 16 ++- tools/testing/selftests/kvm/.gitignore | 1 + tools/testing/selftests/kvm/Makefile | 1 + .../selftests/kvm/aarch64/psci_cpu_on_test.c | 121 ++++++++++++++++++ .../selftests/kvm/include/aarch64/processor.h | 3 + 7 files changed, 162 insertions(+), 9 deletions(-) create mode 100644 tools/testing/selftests/kvm/aarch64/psci_cpu_on_test.c -- 2.33.0.rc1.237.g0d66db33f3-goog _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
next reply other threads:[~2021-08-18 20:21 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-18 20:21 Oliver Upton [this message] 2021-08-18 20:21 ` [PATCH v2 0/4] KVM: arm64: Fix some races in CPU_ON PSCI call Oliver Upton 2021-08-18 20:21 ` [PATCH v2 1/4] KVM: arm64: Fix read-side race on updates to vcpu reset state Oliver Upton 2021-08-18 20:21 ` Oliver Upton 2021-08-18 20:21 ` [PATCH v2 2/4] KVM: arm64: Handle PSCI resets before userspace touches vCPU state Oliver Upton 2021-08-18 20:21 ` Oliver Upton 2021-08-18 20:21 ` [PATCH v2 3/4] KVM: arm64: Enforce reserved bits for PSCI target affinities Oliver Upton 2021-08-18 20:21 ` Oliver Upton 2021-08-18 20:21 ` [PATCH v2 4/4] selftests: KVM: Introduce psci_cpu_on_test Oliver Upton 2021-08-18 20:21 ` Oliver Upton 2021-08-19 8:18 ` [PATCH v2 0/4] KVM: arm64: Fix some races in CPU_ON PSCI call Marc Zyngier 2021-08-19 8:18 ` Marc Zyngier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210818202133.1106786-1-oupton@google.com \ --to=oupton@google.com \ --cc=alexandru.elisei@arm.com \ --cc=drjones@redhat.com \ --cc=james.morse@arm.com \ --cc=jingzhangos@google.com \ --cc=kvm@vger.kernel.org \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=maz@kernel.org \ --cc=pshier@google.com \ --cc=rananta@google.com \ --cc=ricarkol@google.com \ --cc=suzuki.poulose@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.