Thread (8 messages) 8 messages, 3 authors, 2021-07-30

Re: [PATCH v2 3/3] KVM: arm64: Use generic KVM xfer to guest work function

From: Oliver Upton <hidden>
Date: 2021-07-30 14:33:39
Also in: kvm, kvmarm, lkml

Marc,

On Fri, Jul 30, 2021 at 2:41 AM Marc Zyngier [off-list ref] wrote:
Hi Oliver,

On Thu, 29 Jul 2021 23:09:16 +0100,
Oliver Upton [off-list ref] wrote:
quoted
Clean up handling of checks for pending work by switching to the generic
infrastructure to do so.

We pick up handling for TIF_NOTIFY_RESUME from this switch, meaning that
task work will be correctly handled.

Signed-off-by: Oliver Upton <redacted>
---
 arch/arm64/kvm/Kconfig |  1 +
 arch/arm64/kvm/arm.c   | 27 ++++++++++++++-------------
 2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/kvm/Kconfig b/arch/arm64/kvm/Kconfig
index a4eba0908bfa..8bc1fac5fa26 100644
--- a/arch/arm64/kvm/Kconfig
+++ b/arch/arm64/kvm/Kconfig
@@ -26,6 +26,7 @@ menuconfig KVM
      select HAVE_KVM_ARCH_TLB_FLUSH_ALL
      select KVM_MMIO
      select KVM_GENERIC_DIRTYLOG_READ_PROTECT
+     select KVM_XFER_TO_GUEST_WORK
      select SRCU
      select KVM_VFIO
      select HAVE_KVM_EVENTFD
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 60d0a546d7fd..9762e2129813 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -6,6 +6,7 @@

 #include <linux/bug.h>
 #include <linux/cpu_pm.h>
+#include <linux/entry-kvm.h>
 #include <linux/errno.h>
 #include <linux/err.h>
 #include <linux/kvm_host.h>
@@ -714,6 +715,13 @@ static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu)
              static_branch_unlikely(&arm64_mismatched_32bit_el0);
 }

+static bool kvm_vcpu_exit_request(struct kvm_vcpu *vcpu)
+{
+     return kvm_request_pending(vcpu) ||
+                     need_new_vmid_gen(&vcpu->arch.hw_mmu->vmid) ||
+                     xfer_to_guest_mode_work_pending();
Here's what xfer_to_guest_mode_work_pending() says:

<quote>
 * Has to be invoked with interrupts disabled before the transition to
 * guest mode.
</quote>

At the point where you call this, we already are in guest mode, at
least in the KVM sense.
I believe the comment is suggestive of guest mode in the hardware
sense, not KVM's vcpu->mode designation. I got this from
arch/x86/kvm/x86.c:vcpu_enter_guest() to infer the author's
intentions.
quoted
+}
+
 /**
  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
  * @vcpu:    The VCPU pointer
@@ -757,7 +765,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
              /*
               * Check conditions before entering the guest
               */
-             cond_resched();
+             if (__xfer_to_guest_mode_work_pending()) {
+                     ret = xfer_to_guest_mode_handle_work(vcpu);
xfer_to_guest_mode_handle_work() already does the exact equivalent of
__xfer_to_guest_mode_work_pending(). Why do we need to do it twice?
Right, there's no need to do the check twice.
quoted
+                     if (!ret)
+                             ret = 1;
+             }

              update_vmid(&vcpu->arch.hw_mmu->vmid);
@@ -776,16 +788,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)

              kvm_vgic_flush_hwstate(vcpu);

-             /*
-              * Exit if we have a signal pending so that we can deliver the
-              * signal to user space.
-              */
-             if (signal_pending(current)) {
-                     ret = -EINTR;
-                     run->exit_reason = KVM_EXIT_INTR;
-                     ++vcpu->stat.signal_exits;
-             }
-
              /*
               * If we're using a userspace irqchip, then check if we need
               * to tell a userspace irqchip about timer or PMU level
@@ -809,8 +811,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
               */
              smp_store_mb(vcpu->mode, IN_GUEST_MODE);

-             if (ret <= 0 || need_new_vmid_gen(&vcpu->arch.hw_mmu->vmid) ||
-                 kvm_request_pending(vcpu)) {
+             if (ret <= 0 || kvm_vcpu_exit_request(vcpu)) {
If you are doing this, please move the userspace irqchip handling into
the helper as well, so that we have a single function dealing with
collecting exit reasons.
Sure thing.

Thanks for the quick review, Marc!

--
Best,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help