* [PATCH v5 01/11] KVM: improve arch vcpu request defining
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 02/11] KVM: add kvm_request_pending Andrew Jones
` (10 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: cdall, marc.zyngier, pbonzini, rkrcmar
Marc Zyngier suggested that we define the arch specific VCPU request
base, rather than requiring each arch to remember to start from 8.
That suggestion, along with Radim Krcmar's recent VCPU request flag
addition, snowballed into defining something of an arch VCPU request
defining API.
No functional change.
(Looks like x86 is running out of arch VCPU request bits. Maybe
someday we'll need to extend to 64.)
Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Christoffer Dall <cdall@linaro.org>
---
arch/arm/include/asm/kvm_host.h | 3 ++-
arch/arm64/include/asm/kvm_host.h | 3 ++-
arch/powerpc/include/asm/kvm_host.h | 4 ++--
arch/s390/include/asm/kvm_host.h | 6 ++---
arch/x86/include/asm/kvm_host.h | 47 ++++++++++++++++++++-----------------
include/linux/kvm_host.h | 7 ++++++
6 files changed, 41 insertions(+), 29 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index 12274d46df70..c556babe467c 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -44,7 +44,8 @@
#define KVM_MAX_VCPUS VGIC_V2_MAX_CPUS
#endif
-#define KVM_REQ_VCPU_EXIT (8 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_VCPU_EXIT \
+ KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
int __attribute_const__ kvm_target_cpu(void);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 32cbe8a3bb0d..0ff991c9c66e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -41,7 +41,8 @@
#define KVM_VCPU_MAX_FEATURES 4
-#define KVM_REQ_VCPU_EXIT (8 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_VCPU_EXIT \
+ KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
int __attribute_const__ kvm_target_cpu(void);
int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 9c51ac4b8f36..50e0bc9723cc 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -52,8 +52,8 @@
#define KVM_IRQCHIP_NUM_PINS 256
/* PPC-specific vcpu->requests bit members */
-#define KVM_REQ_WATCHDOG 8
-#define KVM_REQ_EPR_EXIT 9
+#define KVM_REQ_WATCHDOG KVM_ARCH_REQ(0)
+#define KVM_REQ_EPR_EXIT KVM_ARCH_REQ(1)
#include <linux/mmu_notifier.h>
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index 426614a882a9..9c3bd94204ac 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -42,9 +42,9 @@
#define KVM_HALT_POLL_NS_DEFAULT 80000
/* s390-specific vcpu->requests bit members */
-#define KVM_REQ_ENABLE_IBS 8
-#define KVM_REQ_DISABLE_IBS 9
-#define KVM_REQ_ICPT_OPEREXC 10
+#define KVM_REQ_ENABLE_IBS KVM_ARCH_REQ(0)
+#define KVM_REQ_DISABLE_IBS KVM_ARCH_REQ(1)
+#define KVM_REQ_ICPT_OPEREXC KVM_ARCH_REQ(2)
#define SIGP_CTRL_C 0x80
#define SIGP_CTRL_SCN_MASK 0x3f
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 9c761fea0c98..563979976fab 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -48,28 +48,31 @@
#define KVM_IRQCHIP_NUM_PINS KVM_IOAPIC_NUM_PINS
/* x86-specific vcpu->requests bit members */
-#define KVM_REQ_MIGRATE_TIMER 8
-#define KVM_REQ_REPORT_TPR_ACCESS 9
-#define KVM_REQ_TRIPLE_FAULT 10
-#define KVM_REQ_MMU_SYNC 11
-#define KVM_REQ_CLOCK_UPDATE 12
-#define KVM_REQ_EVENT 14
-#define KVM_REQ_APF_HALT 15
-#define KVM_REQ_STEAL_UPDATE 16
-#define KVM_REQ_NMI 17
-#define KVM_REQ_PMU 18
-#define KVM_REQ_PMI 19
-#define KVM_REQ_SMI 20
-#define KVM_REQ_MASTERCLOCK_UPDATE 21
-#define KVM_REQ_MCLOCK_INPROGRESS (22 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
-#define KVM_REQ_SCAN_IOAPIC (23 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
-#define KVM_REQ_GLOBAL_CLOCK_UPDATE 24
-#define KVM_REQ_APIC_PAGE_RELOAD (25 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
-#define KVM_REQ_HV_CRASH 26
-#define KVM_REQ_IOAPIC_EOI_EXIT 27
-#define KVM_REQ_HV_RESET 28
-#define KVM_REQ_HV_EXIT 29
-#define KVM_REQ_HV_STIMER 30
+#define KVM_REQ_MIGRATE_TIMER KVM_ARCH_REQ(0)
+#define KVM_REQ_REPORT_TPR_ACCESS KVM_ARCH_REQ(1)
+#define KVM_REQ_TRIPLE_FAULT KVM_ARCH_REQ(2)
+#define KVM_REQ_MMU_SYNC KVM_ARCH_REQ(3)
+#define KVM_REQ_CLOCK_UPDATE KVM_ARCH_REQ(4)
+#define KVM_REQ_EVENT KVM_ARCH_REQ(6)
+#define KVM_REQ_APF_HALT KVM_ARCH_REQ(7)
+#define KVM_REQ_STEAL_UPDATE KVM_ARCH_REQ(8)
+#define KVM_REQ_NMI KVM_ARCH_REQ(9)
+#define KVM_REQ_PMU KVM_ARCH_REQ(10)
+#define KVM_REQ_PMI KVM_ARCH_REQ(11)
+#define KVM_REQ_SMI KVM_ARCH_REQ(12)
+#define KVM_REQ_MASTERCLOCK_UPDATE KVM_ARCH_REQ(13)
+#define KVM_REQ_MCLOCK_INPROGRESS \
+ KVM_ARCH_REQ_FLAGS(14, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_SCAN_IOAPIC \
+ KVM_ARCH_REQ_FLAGS(15, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_GLOBAL_CLOCK_UPDATE KVM_ARCH_REQ(16)
+#define KVM_REQ_APIC_PAGE_RELOAD \
+ KVM_ARCH_REQ_FLAGS(17, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_HV_CRASH KVM_ARCH_REQ(18)
+#define KVM_REQ_IOAPIC_EOI_EXIT KVM_ARCH_REQ(19)
+#define KVM_REQ_HV_RESET KVM_ARCH_REQ(20)
+#define KVM_REQ_HV_EXIT KVM_ARCH_REQ(21)
+#define KVM_REQ_HV_STIMER KVM_ARCH_REQ(22)
#define CR0_RESERVED_BITS \
(~(unsigned long)(X86_CR0_PE | X86_CR0_MP | X86_CR0_EM | X86_CR0_TS \
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8c0664309815..3724b51aab64 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -126,6 +126,13 @@ static inline bool is_error_page(struct page *page)
#define KVM_REQ_MMU_RELOAD (1 | KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_PENDING_TIMER 2
#define KVM_REQ_UNHALT 3
+#define KVM_REQUEST_ARCH_BASE 8
+
+#define KVM_ARCH_REQ_FLAGS(nr, flags) ({ \
+ BUILD_BUG_ON((unsigned)(nr) >= 32 - KVM_REQUEST_ARCH_BASE); \
+ (unsigned)(((nr) + KVM_REQUEST_ARCH_BASE) | (flags)); \
+})
+#define KVM_ARCH_REQ(nr) KVM_ARCH_REQ_FLAGS(nr, 0)
#define KVM_USERSPACE_IRQ_SOURCE_ID 0
#define KVM_IRQFD_RESAMPLE_IRQ_SOURCE_ID 1
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 02/11] KVM: add kvm_request_pending
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
2017-06-04 12:43 ` [PATCH v5 01/11] KVM: improve arch vcpu request defining Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 03/11] KVM: Add documentation for VCPU requests Andrew Jones
` (9 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: cdall, marc.zyngier, pbonzini, rkrcmar
From: Radim Krčmář <rkrcmar@redhat.com>
A first step in vcpu->requests encapsulation. Additionally, we now
use READ_ONCE() when accessing vcpu->requests, which ensures we
always load vcpu->requests when it's accessed. This is important as
other threads can change it any time. Also, READ_ONCE() documents
that vcpu->requests is used with other threads, likely requiring
memory barriers, which it does.
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
[ Documented the new use of READ_ONCE() and converted another check
in arch/mips/kvm/vz.c ]
Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Christoffer Dall <cdall@linaro.org>
---
arch/mips/kvm/trap_emul.c | 2 +-
arch/mips/kvm/vz.c | 2 +-
arch/powerpc/kvm/booke.c | 2 +-
arch/powerpc/kvm/powerpc.c | 5 ++---
arch/s390/kvm/kvm-s390.c | 2 +-
arch/x86/kvm/x86.c | 4 ++--
include/linux/kvm_host.h | 5 +++++
7 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/mips/kvm/trap_emul.c b/arch/mips/kvm/trap_emul.c
index a563759fd142..6a0d7040d882 100644
--- a/arch/mips/kvm/trap_emul.c
+++ b/arch/mips/kvm/trap_emul.c
@@ -1094,7 +1094,7 @@ static void kvm_trap_emul_check_requests(struct kvm_vcpu *vcpu, int cpu,
struct mm_struct *mm;
int i;
- if (likely(!vcpu->requests))
+ if (likely(!kvm_request_pending(vcpu)))
return;
if (kvm_check_request(KVM_REQ_TLB_FLUSH, vcpu)) {
diff --git a/arch/mips/kvm/vz.c b/arch/mips/kvm/vz.c
index 71d8856ade64..74805035edc8 100644
--- a/arch/mips/kvm/vz.c
+++ b/arch/mips/kvm/vz.c
@@ -2337,7 +2337,7 @@ static int kvm_vz_check_requests(struct kvm_vcpu *vcpu, int cpu)
int ret = 0;
int i;
- if (!vcpu->requests)
+ if (!kvm_request_pending(vcpu))
return 0;
if (kvm_check_request(KVM_REQ_TLB_FLUSH, vcpu)) {
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
index 3eaac3809977..071b87ee682f 100644
--- a/arch/powerpc/kvm/booke.c
+++ b/arch/powerpc/kvm/booke.c
@@ -687,7 +687,7 @@ int kvmppc_core_prepare_to_enter(struct kvm_vcpu *vcpu)
kvmppc_core_check_exceptions(vcpu);
- if (vcpu->requests) {
+ if (kvm_request_pending(vcpu)) {
/* Exception delivery raised request; start over */
return 1;
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index f7cf2cd564ef..fd64f087737c 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -55,8 +55,7 @@ EXPORT_SYMBOL_GPL(kvmppc_pr_ops);
int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
{
- return !!(v->arch.pending_exceptions) ||
- v->requests;
+ return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
}
int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
@@ -108,7 +107,7 @@ int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
*/
smp_mb();
- if (vcpu->requests) {
+ if (kvm_request_pending(vcpu)) {
/* Make sure we process requests preemptable */
local_irq_enable();
trace_kvm_check_requests(vcpu);
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 689ac48361c6..ad41e0fa3a21 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2440,7 +2440,7 @@ static int kvm_s390_handle_requests(struct kvm_vcpu *vcpu)
{
retry:
kvm_s390_vcpu_request_handled(vcpu);
- if (!vcpu->requests)
+ if (!kvm_request_pending(vcpu))
return 0;
/*
* We use MMU_RELOAD just to re-arm the ipte notifier for the
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 464da936c53d..f81060518635 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6710,7 +6710,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
bool req_immediate_exit = false;
- if (vcpu->requests) {
+ if (kvm_request_pending(vcpu)) {
if (kvm_check_request(KVM_REQ_MMU_RELOAD, vcpu))
kvm_mmu_unload(vcpu);
if (kvm_check_request(KVM_REQ_MIGRATE_TIMER, vcpu))
@@ -6874,7 +6874,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
kvm_x86_ops->sync_pir_to_irr(vcpu);
}
- if (vcpu->mode == EXITING_GUEST_MODE || vcpu->requests
+ if (vcpu->mode == EXITING_GUEST_MODE || kvm_request_pending(vcpu)
|| need_resched() || signal_pending(current)) {
vcpu->mode = OUTSIDE_GUEST_MODE;
smp_wmb();
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3724b51aab64..0b50e7b35ed4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1105,6 +1105,11 @@ static inline void kvm_make_request(int req, struct kvm_vcpu *vcpu)
set_bit(req & KVM_REQUEST_MASK, &vcpu->requests);
}
+static inline bool kvm_request_pending(struct kvm_vcpu *vcpu)
+{
+ return READ_ONCE(vcpu->requests);
+}
+
static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
{
return test_bit(req & KVM_REQUEST_MASK, &vcpu->requests);
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 03/11] KVM: Add documentation for VCPU requests
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
2017-06-04 12:43 ` [PATCH v5 01/11] KVM: improve arch vcpu request defining Andrew Jones
2017-06-04 12:43 ` [PATCH v5 02/11] KVM: add kvm_request_pending Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 04/11] KVM: arm/arm64: properly use vcpu requests Andrew Jones
` (8 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: cdall, marc.zyngier, pbonzini, rkrcmar
Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Christoffer Dall <cdall@linaro.org>
---
Documentation/virtual/kvm/vcpu-requests.rst | 307 ++++++++++++++++++++++++++++
1 file changed, 307 insertions(+)
create mode 100644 Documentation/virtual/kvm/vcpu-requests.rst
diff --git a/Documentation/virtual/kvm/vcpu-requests.rst b/Documentation/virtual/kvm/vcpu-requests.rst
new file mode 100644
index 000000000000..5feb3706a7ae
--- /dev/null
+++ b/Documentation/virtual/kvm/vcpu-requests.rst
@@ -0,0 +1,307 @@
+=================
+KVM VCPU Requests
+=================
+
+Overview
+========
+
+KVM supports an internal API enabling threads to request a VCPU thread to
+perform some activity. For example, a thread may request a VCPU to flush
+its TLB with a VCPU request. The API consists of the following functions::
+
+ /* Check if any requests are pending for VCPU @vcpu. */
+ bool kvm_request_pending(struct kvm_vcpu *vcpu);
+
+ /* Check if VCPU @vcpu has request @req pending. */
+ bool kvm_test_request(int req, struct kvm_vcpu *vcpu);
+
+ /* Clear request @req for VCPU @vcpu. */
+ void kvm_clear_request(int req, struct kvm_vcpu *vcpu);
+
+ /*
+ * Check if VCPU @vcpu has request @req pending. When the request is
+ * pending it will be cleared and a memory barrier, which pairs with
+ * another in kvm_make_request(), will be issued.
+ */
+ bool kvm_check_request(int req, struct kvm_vcpu *vcpu);
+
+ /*
+ * Make request @req of VCPU @vcpu. Issues a memory barrier, which pairs
+ * with another in kvm_check_request(), prior to setting the request.
+ */
+ void kvm_make_request(int req, struct kvm_vcpu *vcpu);
+
+ /* Make request @req of all VCPUs of the VM with struct kvm @kvm. */
+ bool kvm_make_all_cpus_request(struct kvm *kvm, unsigned int req);
+
+Typically a requester wants the VCPU to perform the activity as soon
+as possible after making the request. This means most requests
+(kvm_make_request() calls) are followed by a call to kvm_vcpu_kick(),
+and kvm_make_all_cpus_request() has the kicking of all VCPUs built
+into it.
+
+VCPU Kicks
+----------
+
+The goal of a VCPU kick is to bring a VCPU thread out of guest mode in
+order to perform some KVM maintenance. To do so, an IPI is sent, forcing
+a guest mode exit. However, a VCPU thread may not be in guest mode at the
+time of the kick. Therefore, depending on the mode and state of the VCPU
+thread, there are two other actions a kick may take. All three actions
+are listed below:
+
+1) Send an IPI. This forces a guest mode exit.
+2) Waking a sleeping VCPU. Sleeping VCPUs are VCPU threads outside guest
+ mode that wait on waitqueues. Waking them removes the threads from
+ the waitqueues, allowing the threads to run again. This behavior
+ may be suppressed, see KVM_REQUEST_NO_WAKEUP below.
+3) Nothing. When the VCPU is not in guest mode and the VCPU thread is not
+ sleeping, then there is nothing to do.
+
+VCPU Mode
+---------
+
+VCPUs have a mode state, ``vcpu->mode``, that is used to track whether the
+guest is running in guest mode or not, as well as some specific
+outside guest mode states. The architecture may use ``vcpu->mode`` to
+ensure VCPU requests are seen by VCPUs (see "Ensuring Requests Are Seen"),
+as well as to avoid sending unnecessary IPIs (see "IPI Reduction"), and
+even to ensure IPI acknowledgements are waited upon (see "Waiting for
+Acknowledgements"). The following modes are defined:
+
+OUTSIDE_GUEST_MODE
+
+ The VCPU thread is outside guest mode.
+
+IN_GUEST_MODE
+
+ The VCPU thread is in guest mode.
+
+EXITING_GUEST_MODE
+
+ The VCPU thread is transitioning from IN_GUEST_MODE to
+ OUTSIDE_GUEST_MODE.
+
+READING_SHADOW_PAGE_TABLES
+
+ The VCPU thread is outside guest mode, but it wants the sender of
+ certain VCPU requests, namely KVM_REQ_TLB_FLUSH, to wait until the VCPU
+ thread is done reading the page tables.
+
+VCPU Request Internals
+======================
+
+VCPU requests are simply bit indices of the ``vcpu->requests`` bitmap.
+This means general bitops, like those documented in [atomic-ops]_ could
+also be used, e.g. ::
+
+ clear_bit(KVM_REQ_UNHALT & KVM_REQUEST_MASK, &vcpu->requests);
+
+However, VCPU request users should refrain from doing so, as it would
+break the abstraction. The first 8 bits are reserved for architecture
+independent requests, all additional bits are available for architecture
+dependent requests.
+
+Architecture Independent Requests
+---------------------------------
+
+KVM_REQ_TLB_FLUSH
+
+ KVM's common MMU notifier may need to flush all of a guest's TLB
+ entries, calling kvm_flush_remote_tlbs() to do so. Architectures that
+ choose to use the common kvm_flush_remote_tlbs() implementation will
+ need to handle this VCPU request.
+
+KVM_REQ_MMU_RELOAD
+
+ When shadow page tables are used and memory slots are removed it's
+ necessary to inform each VCPU to completely refresh the tables. This
+ request is used for that.
+
+KVM_REQ_PENDING_TIMER
+
+ This request may be made from a timer handler run on the host on behalf
+ of a VCPU. It informs the VCPU thread to inject a timer interrupt.
+
+KVM_REQ_UNHALT
+
+ This request may be made from the KVM common function kvm_vcpu_block(),
+ which is used to emulate an instruction that causes a CPU to halt until
+ one of an architectural specific set of events and/or interrupts is
+ received (determined by checking kvm_arch_vcpu_runnable()). When that
+ event or interrupt arrives kvm_vcpu_block() makes the request. This is
+ in contrast to when kvm_vcpu_block() returns due to any other reason,
+ such as a pending signal, which does not indicate the VCPU's halt
+ emulation should stop, and therefore does not make the request.
+
+KVM_REQUEST_MASK
+----------------
+
+VCPU requests should be masked by KVM_REQUEST_MASK before using them with
+bitops. This is because only the lower 8 bits are used to represent the
+request's number. The upper bits are used as flags. Currently only two
+flags are defined.
+
+VCPU Request Flags
+------------------
+
+KVM_REQUEST_NO_WAKEUP
+
+ This flag is applied to requests that only need immediate attention
+ from VCPUs running in guest mode. That is, sleeping VCPUs do not need
+ to be awaken for these requests. Sleeping VCPUs will handle the
+ requests when they are awaken later for some other reason.
+
+KVM_REQUEST_WAIT
+
+ When requests with this flag are made with kvm_make_all_cpus_request(),
+ then the caller will wait for each VCPU to acknowledge its IPI before
+ proceeding. This flag only applies to VCPUs that would receive IPIs.
+ If, for example, the VCPU is sleeping, so no IPI is necessary, then
+ the requesting thread does not wait. This means that this flag may be
+ safely combined with KVM_REQUEST_NO_WAKEUP. See "Waiting for
+ Acknowledgements" for more information about requests with
+ KVM_REQUEST_WAIT.
+
+VCPU Requests with Associated State
+===================================
+
+Requesters that want the receiving VCPU to handle new state need to ensure
+the newly written state is observable to the receiving VCPU thread's CPU
+by the time it observes the request. This means a write memory barrier
+must be inserted after writing the new state and before setting the VCPU
+request bit. Additionally, on the receiving VCPU thread's side, a
+corresponding read barrier must be inserted after reading the request bit
+and before proceeding to read the new state associated with it. See
+scenario 3, Message and Flag, of [lwn-mb]_ and the kernel documentation
+[memory-barriers]_.
+
+The pair of functions, kvm_check_request() and kvm_make_request(), provide
+the memory barriers, allowing this requirement to be handled internally by
+the API.
+
+Ensuring Requests Are Seen
+==========================
+
+When making requests to VCPUs, we want to avoid the receiving VCPU
+executing in guest mode for an arbitrary long time without handling the
+request. We can be sure this won't happen as long as we ensure the VCPU
+thread checks kvm_request_pending() before entering guest mode and that a
+kick will send an IPI to force an exit from guest mode when necessary.
+Extra care must be taken to cover the period after the VCPU thread's last
+kvm_request_pending() check and before it has entered guest mode, as kick
+IPIs will only trigger guest mode exits for VCPU threads that are in guest
+mode or at least have already disabled interrupts in order to prepare to
+enter guest mode. This means that an optimized implementation (see "IPI
+Reduction") must be certain when it's safe to not send the IPI. One
+solution, which all architectures except s390 apply, is to:
+
+- set ``vcpu->mode`` to IN_GUEST_MODE between disabling the interrupts and
+ the last kvm_request_pending() check;
+- enable interrupts atomically when entering the guest.
+
+This solution also requires memory barriers to be placed carefully in both
+the requesting thread and the receiving VCPU. With the memory barriers we
+can exclude the possibility of a VCPU thread observing
+!kvm_request_pending() on its last check and then not receiving an IPI for
+the next request made of it, even if the request is made immediately after
+the check. This is done by way of the Dekker memory barrier pattern
+(scenario 10 of [lwn-mb]_). As the Dekker pattern requires two variables,
+this solution pairs ``vcpu->mode`` with ``vcpu->requests``. Substituting
+them into the pattern gives::
+
+ CPU1 CPU2
+ ================= =================
+ local_irq_disable();
+ WRITE_ONCE(vcpu->mode, IN_GUEST_MODE); kvm_make_request(REQ, vcpu);
+ smp_mb(); smp_mb();
+ if (kvm_request_pending(vcpu)) { if (READ_ONCE(vcpu->mode) ==
+ IN_GUEST_MODE) {
+ ...abort guest entry... ...send IPI...
+ } }
+
+As stated above, the IPI is only useful for VCPU threads in guest mode or
+that have already disabled interrupts. This is why this specific case of
+the Dekker pattern has been extended to disable interrupts before setting
+``vcpu->mode`` to IN_GUEST_MODE. WRITE_ONCE() and READ_ONCE() are used to
+pedantically implement the memory barrier pattern, guaranteeing the
+compiler doesn't interfere with ``vcpu->mode``'s carefully planned
+accesses.
+
+IPI Reduction
+-------------
+
+As only one IPI is needed to get a VCPU to check for any/all requests,
+then they may be coalesced. This is easily done by having the first IPI
+sending kick also change the VCPU mode to something !IN_GUEST_MODE. The
+transitional state, EXITING_GUEST_MODE, is used for this purpose.
+
+Waiting for Acknowledgements
+----------------------------
+
+Some requests, those with the KVM_REQUEST_WAIT flag set, require IPIs to
+be sent, and the acknowledgements to be waited upon, even when the target
+VCPU threads are in modes other than IN_GUEST_MODE. For example, one case
+is when a target VCPU thread is in READING_SHADOW_PAGE_TABLES mode, which
+is set after disabling interrupts. To support these cases, the
+KVM_REQUEST_WAIT flag changes the condition for sending an IPI from
+checking that the VCPU is IN_GUEST_MODE to checking that it is not
+OUTSIDE_GUEST_MODE.
+
+Request-less VCPU Kicks
+-----------------------
+
+As the determination of whether or not to send an IPI depends on the
+two-variable Dekker memory barrier pattern, then it's clear that
+request-less VCPU kicks are almost never correct. Without the assurance
+that a non-IPI generating kick will still result in an action by the
+receiving VCPU, as the final kvm_request_pending() check does for
+request-accompanying kicks, then the kick may not do anything useful at
+all. If, for instance, a request-less kick was made to a VCPU that was
+just about to set its mode to IN_GUEST_MODE, meaning no IPI is sent, then
+the VCPU thread may continue its entry without actually having done
+whatever it was the kick was meant to initiate.
+
+One exception is x86's posted interrupt mechanism. In this case, however,
+even the request-less VCPU kick is coupled with the same
+local_irq_disable() + smp_mb() pattern described above; the ON bit
+(Outstanding Notification) in the posted interrupt descriptor takes the
+role of ``vcpu->requests``. When sending a posted interrupt, PIR.ON is
+set before reading ``vcpu->mode``; dually, in the VCPU thread,
+vmx_sync_pir_to_irr() reads PIR after setting ``vcpu->mode`` to
+IN_GUEST_MODE.
+
+Additional Considerations
+=========================
+
+Sleeping VCPUs
+--------------
+
+VCPU threads may need to consider requests before and/or after calling
+functions that may put them to sleep, e.g. kvm_vcpu_block(). Whether they
+do or not, and, if they do, which requests need consideration, is
+architecture dependent. kvm_vcpu_block() calls kvm_arch_vcpu_runnable()
+to check if it should awaken. One reason to do so is to provide
+architectures a function where requests may be checked if necessary.
+
+Clearing Requests
+-----------------
+
+Generally it only makes sense for the receiving VCPU thread to clear a
+request. However, in some circumstances, such as when the requesting
+thread and the receiving VCPU thread are executed serially, such as when
+they are the same thread, or when they are using some form of concurrency
+control to temporarily execute synchronously, then it's possible to know
+that the request may be cleared immediately, rather than waiting for the
+receiving VCPU thread to handle the request in VCPU RUN. The only current
+examples of this are kvm_vcpu_block() calls made by VCPUs to block
+themselves. A possible side-effect of that call is to make the
+KVM_REQ_UNHALT request, which may then be cleared immediately when the
+VCPU returns from the call.
+
+References
+==========
+
+.. [atomic-ops] Documentation/core-api/atomic_ops.rst
+.. [memory-barriers] Documentation/memory-barriers.txt
+.. [lwn-mb] https://lwn.net/Articles/573436/
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 04/11] KVM: arm/arm64: properly use vcpu requests
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (2 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 03/11] KVM: Add documentation for VCPU requests Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 14:51 ` Christoffer Dall
2017-06-04 12:43 ` [PATCH v5 05/11] KVM: arm/arm64: replace pause checks with vcpu request checks Andrew Jones
` (7 subsequent siblings)
11 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
arm/arm64 already has one VCPU request used when setting pause,
but it doesn't properly check requests in VCPU RUN. Check it
and also make sure we set vcpu->mode at the appropriate time
(before the check) and with the appropriate barriers. See
Documentation/virtual/kvm/vcpu-requests.rst. Also make sure we
don't leave any vcpu requests we don't intend to handle later
set in the request bitmap. If we don't clear them, then
kvm_request_pending() may return true when it shouldn't.
Using VCPU requests properly fixes a small race where pause
could get set just as a VCPU was entering guest mode.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
arch/arm/kvm/handle_exit.c | 1 +
arch/arm64/kvm/handle_exit.c | 1 +
virt/kvm/arm/arm.c | 14 ++++++++++++--
virt/kvm/arm/psci.c | 1 +
4 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
index 5fd7968cdae9..a2b4f7b82356 100644
--- a/arch/arm/kvm/handle_exit.c
+++ b/arch/arm/kvm/handle_exit.c
@@ -72,6 +72,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
trace_kvm_wfx(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
kvm_vcpu_block(vcpu);
+ kvm_clear_request(KVM_REQ_UNHALT, vcpu);
}
kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index fa1b18e364fc..17d8a1677a0b 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -89,6 +89,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
vcpu->stat.wfi_exit_stat++;
kvm_vcpu_block(vcpu);
+ kvm_clear_request(KVM_REQ_UNHALT, vcpu);
}
kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 3c387fdc4a9e..138212605ad9 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -546,6 +546,7 @@ void kvm_arm_resume_guest(struct kvm *kvm)
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.pause = false;
+ kvm_clear_request(KVM_REQ_VCPU_EXIT, vcpu);
swake_up(kvm_arch_vcpu_wq(vcpu));
}
}
@@ -638,8 +639,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
run->exit_reason = KVM_EXIT_INTR;
}
+ /*
+ * Ensure we set mode to IN_GUEST_MODE after we disable
+ * interrupts and before the final VCPU requests check.
+ * See the comment in kvm_vcpu_exiting_guest_mode() and
+ * Documentation/virtual/kvm/vcpu-requests.rst
+ */
+ smp_store_mb(vcpu->mode, IN_GUEST_MODE);
+
if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
- vcpu->arch.power_off || vcpu->arch.pause) {
+ kvm_request_pending(vcpu) ||
+ vcpu->arch.power_off || vcpu->arch.pause) {
+ vcpu->mode = OUTSIDE_GUEST_MODE;
local_irq_enable();
kvm_pmu_sync_hwstate(vcpu);
kvm_timer_sync_hwstate(vcpu);
@@ -655,7 +666,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
*/
trace_kvm_entry(*vcpu_pc(vcpu));
guest_enter_irqoff();
- vcpu->mode = IN_GUEST_MODE;
ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index a08d7a93aebb..f68be2cc6256 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -57,6 +57,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
* for KVM will preserve the register state.
*/
kvm_vcpu_block(vcpu);
+ kvm_clear_request(KVM_REQ_UNHALT, vcpu);
return PSCI_RET_SUCCESS;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 04/11] KVM: arm/arm64: properly use vcpu requests
2017-06-04 12:43 ` [PATCH v5 04/11] KVM: arm/arm64: properly use vcpu requests Andrew Jones
@ 2017-06-04 14:51 ` Christoffer Dall
0 siblings, 0 replies; 15+ messages in thread
From: Christoffer Dall @ 2017-06-04 14:51 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvmarm, kvm, marc.zyngier, pbonzini, rkrcmar
On Sun, Jun 04, 2017 at 02:43:54PM +0200, Andrew Jones wrote:
> arm/arm64 already has one VCPU request used when setting pause,
> but it doesn't properly check requests in VCPU RUN. Check it
> and also make sure we set vcpu->mode at the appropriate time
> (before the check) and with the appropriate barriers. See
> Documentation/virtual/kvm/vcpu-requests.rst. Also make sure we
> don't leave any vcpu requests we don't intend to handle later
> set in the request bitmap. If we don't clear them, then
> kvm_request_pending() may return true when it shouldn't.
>
> Using VCPU requests properly fixes a small race where pause
> could get set just as a VCPU was entering guest mode.
Whoops, i forgot to say that I reviewed this:
Reviewed-by: Christoffer Dall <cdall@linaro.org>
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> arch/arm/kvm/handle_exit.c | 1 +
> arch/arm64/kvm/handle_exit.c | 1 +
> virt/kvm/arm/arm.c | 14 ++++++++++++--
> virt/kvm/arm/psci.c | 1 +
> 4 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kvm/handle_exit.c b/arch/arm/kvm/handle_exit.c
> index 5fd7968cdae9..a2b4f7b82356 100644
> --- a/arch/arm/kvm/handle_exit.c
> +++ b/arch/arm/kvm/handle_exit.c
> @@ -72,6 +72,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
> trace_kvm_wfx(*vcpu_pc(vcpu), false);
> vcpu->stat.wfi_exit_stat++;
> kvm_vcpu_block(vcpu);
> + kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> }
>
> kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index fa1b18e364fc..17d8a1677a0b 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -89,6 +89,7 @@ static int kvm_handle_wfx(struct kvm_vcpu *vcpu, struct kvm_run *run)
> trace_kvm_wfx_arm64(*vcpu_pc(vcpu), false);
> vcpu->stat.wfi_exit_stat++;
> kvm_vcpu_block(vcpu);
> + kvm_clear_request(KVM_REQ_UNHALT, vcpu);
> }
>
> kvm_skip_instr(vcpu, kvm_vcpu_trap_il_is32bit(vcpu));
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index 3c387fdc4a9e..138212605ad9 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -546,6 +546,7 @@ void kvm_arm_resume_guest(struct kvm *kvm)
>
> kvm_for_each_vcpu(i, vcpu, kvm) {
> vcpu->arch.pause = false;
> + kvm_clear_request(KVM_REQ_VCPU_EXIT, vcpu);
> swake_up(kvm_arch_vcpu_wq(vcpu));
> }
> }
> @@ -638,8 +639,18 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> run->exit_reason = KVM_EXIT_INTR;
> }
>
> + /*
> + * Ensure we set mode to IN_GUEST_MODE after we disable
> + * interrupts and before the final VCPU requests check.
> + * See the comment in kvm_vcpu_exiting_guest_mode() and
> + * Documentation/virtual/kvm/vcpu-requests.rst
> + */
> + smp_store_mb(vcpu->mode, IN_GUEST_MODE);
> +
> if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
> - vcpu->arch.power_off || vcpu->arch.pause) {
> + kvm_request_pending(vcpu) ||
> + vcpu->arch.power_off || vcpu->arch.pause) {
> + vcpu->mode = OUTSIDE_GUEST_MODE;
> local_irq_enable();
> kvm_pmu_sync_hwstate(vcpu);
> kvm_timer_sync_hwstate(vcpu);
> @@ -655,7 +666,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
> */
> trace_kvm_entry(*vcpu_pc(vcpu));
> guest_enter_irqoff();
> - vcpu->mode = IN_GUEST_MODE;
>
> ret = kvm_call_hyp(__kvm_vcpu_run, vcpu);
>
> diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
> index a08d7a93aebb..f68be2cc6256 100644
> --- a/virt/kvm/arm/psci.c
> +++ b/virt/kvm/arm/psci.c
> @@ -57,6 +57,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
> * for KVM will preserve the register state.
> */
> kvm_vcpu_block(vcpu);
> + kvm_clear_request(KVM_REQ_UNHALT, vcpu);
>
> return PSCI_RET_SUCCESS;
> }
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v5 05/11] KVM: arm/arm64: replace pause checks with vcpu request checks
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (3 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 04/11] KVM: arm/arm64: properly use vcpu requests Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 06/11] KVM: arm/arm64: use vcpu requests for power_off Andrew Jones
` (6 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
The current use of KVM_REQ_VCPU_EXIT for pause is fine. Even the
requester clearing the request is OK, as this is the special case
where the sole requesting thread and receiving VCPU are executing
synchronously (see "Clearing Requests" in
Documentation/virtual/kvm/vcpu-requests.rst) However, that's about
to change, so let's ensure only the receiving VCPU clears the
request. Additionally, by guaranteeing KVM_REQ_VCPU_EXIT is always
set when pause is, we can avoid checking pause directly in VCPU RUN.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
---
virt/kvm/arm/arm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 138212605ad9..21a4db90073f 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -546,7 +546,6 @@ void kvm_arm_resume_guest(struct kvm *kvm)
kvm_for_each_vcpu(i, vcpu, kvm) {
vcpu->arch.pause = false;
- kvm_clear_request(KVM_REQ_VCPU_EXIT, vcpu);
swake_up(kvm_arch_vcpu_wq(vcpu));
}
}
@@ -557,6 +556,11 @@ static void vcpu_sleep(struct kvm_vcpu *vcpu)
swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
(!vcpu->arch.pause)));
+
+ if (vcpu->arch.pause) {
+ /* Awaken to handle a signal, request we sleep again later. */
+ kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ }
}
static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
@@ -564,6 +568,14 @@ static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
return vcpu->arch.target >= 0;
}
+static void check_vcpu_requests(struct kvm_vcpu *vcpu)
+{
+ if (kvm_request_pending(vcpu)) {
+ if (kvm_check_request(KVM_REQ_VCPU_EXIT, vcpu))
+ vcpu_sleep(vcpu);
+ }
+}
+
/**
* kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
* @vcpu: The VCPU pointer
@@ -609,7 +621,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
update_vttbr(vcpu->kvm);
- if (vcpu->arch.power_off || vcpu->arch.pause)
+ check_vcpu_requests(vcpu);
+
+ if (vcpu->arch.power_off)
vcpu_sleep(vcpu);
/*
@@ -649,7 +663,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
kvm_request_pending(vcpu) ||
- vcpu->arch.power_off || vcpu->arch.pause) {
+ vcpu->arch.power_off) {
vcpu->mode = OUTSIDE_GUEST_MODE;
local_irq_enable();
kvm_pmu_sync_hwstate(vcpu);
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 06/11] KVM: arm/arm64: use vcpu requests for power_off
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (4 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 05/11] KVM: arm/arm64: replace pause checks with vcpu request checks Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 07/11] KVM: arm/arm64: optimize VCPU RUN Andrew Jones
` (5 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
System shutdown is currently using request-less VCPU kicks. This
leaves open a tiny race window, as it doesn't ensure the state
change to power_off is seen by a VCPU just about to enter guest
mode. VCPU requests, OTOH, are guaranteed to be seen (see "Ensuring
Requests Are Seen" of Documentation/virtual/kvm/vcpu-requests.rst)
This patch applies the EXIT request used by pause to power_off,
fixing the race.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
---
virt/kvm/arm/psci.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index f68be2cc6256..f189d0ad30d5 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -179,10 +179,9 @@ static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
* after this call is handled and before the VCPUs have been
* re-initialized.
*/
- kvm_for_each_vcpu(i, tmp, vcpu->kvm) {
+ kvm_for_each_vcpu(i, tmp, vcpu->kvm)
tmp->arch.power_off = true;
- kvm_vcpu_kick(tmp);
- }
+ kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_VCPU_EXIT);
memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
vcpu->run->system_event.type = type;
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 07/11] KVM: arm/arm64: optimize VCPU RUN
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (5 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 06/11] KVM: arm/arm64: use vcpu requests for power_off Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 08/11] KVM: arm/arm64: change exit request to sleep request Andrew Jones
` (4 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
We can make a small optimization by not checking the state of
the power_off field on each run. This is done by treating
power_off like pause, only checking it when we get the EXIT
VCPU request. When a VCPU powers off another VCPU the EXIT
request is already made, so we just need to make sure the
request is also made on self power off. kvm_vcpu_kick() isn't
necessary for these cases, as the VCPU would just be kicking
itself, but we add it anyway as a self kick doesn't cost much,
and it makes the code more future-proof.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
---
virt/kvm/arm/arm.c | 19 +++++++++++--------
virt/kvm/arm/psci.c | 2 ++
2 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 21a4db90073f..9379b1d75ad3 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -368,6 +368,13 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
kvm_timer_vcpu_put(vcpu);
}
+static void vcpu_power_off(struct kvm_vcpu *vcpu)
+{
+ vcpu->arch.power_off = true;
+ kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ kvm_vcpu_kick(vcpu);
+}
+
int kvm_arch_vcpu_ioctl_get_mpstate(struct kvm_vcpu *vcpu,
struct kvm_mp_state *mp_state)
{
@@ -387,7 +394,7 @@ int kvm_arch_vcpu_ioctl_set_mpstate(struct kvm_vcpu *vcpu,
vcpu->arch.power_off = false;
break;
case KVM_MP_STATE_STOPPED:
- vcpu->arch.power_off = true;
+ vcpu_power_off(vcpu);
break;
default:
return -EINVAL;
@@ -557,7 +564,7 @@ static void vcpu_sleep(struct kvm_vcpu *vcpu)
swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
(!vcpu->arch.pause)));
- if (vcpu->arch.pause) {
+ if (vcpu->arch.power_off || vcpu->arch.pause) {
/* Awaken to handle a signal, request we sleep again later. */
kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
}
@@ -623,9 +630,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
check_vcpu_requests(vcpu);
- if (vcpu->arch.power_off)
- vcpu_sleep(vcpu);
-
/*
* Preparing the interrupts to be injected also
* involves poking the GIC, which must be done in a
@@ -662,8 +666,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
smp_store_mb(vcpu->mode, IN_GUEST_MODE);
if (ret <= 0 || need_new_vmid_gen(vcpu->kvm) ||
- kvm_request_pending(vcpu) ||
- vcpu->arch.power_off) {
+ kvm_request_pending(vcpu)) {
vcpu->mode = OUTSIDE_GUEST_MODE;
local_irq_enable();
kvm_pmu_sync_hwstate(vcpu);
@@ -896,7 +899,7 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
* Handle the "start in power-off" case.
*/
if (test_bit(KVM_ARM_VCPU_POWER_OFF, vcpu->arch.features))
- vcpu->arch.power_off = true;
+ vcpu_power_off(vcpu);
else
vcpu->arch.power_off = false;
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index f189d0ad30d5..4a436685c552 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -65,6 +65,8 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
{
vcpu->arch.power_off = true;
+ kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ kvm_vcpu_kick(vcpu);
}
static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 08/11] KVM: arm/arm64: change exit request to sleep request
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (6 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 07/11] KVM: arm/arm64: optimize VCPU RUN Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:43 ` [PATCH v5 09/11] KVM: arm/arm64: use vcpu requests for irq injection Andrew Jones
` (3 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
A request called EXIT is too generic. All requests are meant to cause
exits, but different requests have different flags. Let's not make
it difficult to decide if the EXIT request is correct for some case
by just always providing unique requests for each case. This patch
changes EXIT to SLEEP, because that's what the request is asking the
VCPU to do.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Acked-by: Christoffer Dall <cdall@linaro.org>
---
arch/arm/include/asm/kvm_host.h | 2 +-
arch/arm64/include/asm/kvm_host.h | 2 +-
virt/kvm/arm/arm.c | 12 ++++++------
virt/kvm/arm/psci.c | 4 ++--
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index c556babe467c..fdd644c01c89 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -44,7 +44,7 @@
#define KVM_MAX_VCPUS VGIC_V2_MAX_CPUS
#endif
-#define KVM_REQ_VCPU_EXIT \
+#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 0ff991c9c66e..9bd0d1040de9 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -41,7 +41,7 @@
#define KVM_VCPU_MAX_FEATURES 4
-#define KVM_REQ_VCPU_EXIT \
+#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
int __attribute_const__ kvm_target_cpu(void);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index 9379b1d75ad3..ddc833987dfb 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -371,7 +371,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
static void vcpu_power_off(struct kvm_vcpu *vcpu)
{
vcpu->arch.power_off = true;
- kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ kvm_make_request(KVM_REQ_SLEEP, vcpu);
kvm_vcpu_kick(vcpu);
}
@@ -543,7 +543,7 @@ void kvm_arm_halt_guest(struct kvm *kvm)
kvm_for_each_vcpu(i, vcpu, kvm)
vcpu->arch.pause = true;
- kvm_make_all_cpus_request(kvm, KVM_REQ_VCPU_EXIT);
+ kvm_make_all_cpus_request(kvm, KVM_REQ_SLEEP);
}
void kvm_arm_resume_guest(struct kvm *kvm)
@@ -557,7 +557,7 @@ void kvm_arm_resume_guest(struct kvm *kvm)
}
}
-static void vcpu_sleep(struct kvm_vcpu *vcpu)
+static void vcpu_req_sleep(struct kvm_vcpu *vcpu)
{
struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
@@ -566,7 +566,7 @@ static void vcpu_sleep(struct kvm_vcpu *vcpu)
if (vcpu->arch.power_off || vcpu->arch.pause) {
/* Awaken to handle a signal, request we sleep again later. */
- kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ kvm_make_request(KVM_REQ_SLEEP, vcpu);
}
}
@@ -578,8 +578,8 @@ static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
static void check_vcpu_requests(struct kvm_vcpu *vcpu)
{
if (kvm_request_pending(vcpu)) {
- if (kvm_check_request(KVM_REQ_VCPU_EXIT, vcpu))
- vcpu_sleep(vcpu);
+ if (kvm_check_request(KVM_REQ_SLEEP, vcpu))
+ vcpu_req_sleep(vcpu);
}
}
diff --git a/virt/kvm/arm/psci.c b/virt/kvm/arm/psci.c
index 4a436685c552..f1e363bab5e8 100644
--- a/virt/kvm/arm/psci.c
+++ b/virt/kvm/arm/psci.c
@@ -65,7 +65,7 @@ static unsigned long kvm_psci_vcpu_suspend(struct kvm_vcpu *vcpu)
static void kvm_psci_vcpu_off(struct kvm_vcpu *vcpu)
{
vcpu->arch.power_off = true;
- kvm_make_request(KVM_REQ_VCPU_EXIT, vcpu);
+ kvm_make_request(KVM_REQ_SLEEP, vcpu);
kvm_vcpu_kick(vcpu);
}
@@ -183,7 +183,7 @@ static void kvm_prepare_system_event(struct kvm_vcpu *vcpu, u32 type)
*/
kvm_for_each_vcpu(i, tmp, vcpu->kvm)
tmp->arch.power_off = true;
- kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_VCPU_EXIT);
+ kvm_make_all_cpus_request(vcpu->kvm, KVM_REQ_SLEEP);
memset(&vcpu->run->system_event, 0, sizeof(vcpu->run->system_event));
vcpu->run->system_event.type = type;
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 09/11] KVM: arm/arm64: use vcpu requests for irq injection
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (7 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 08/11] KVM: arm/arm64: change exit request to sleep request Andrew Jones
@ 2017-06-04 12:43 ` Andrew Jones
2017-06-04 12:44 ` [PATCH v5 10/11] KVM: arm/arm64: PMU: remove request-less vcpu kick Andrew Jones
` (2 subsequent siblings)
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:43 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
Don't use request-less VCPU kicks when injecting IRQs, as a VCPU
kick meant to trigger the interrupt injection could be sent while
the VCPU is outside guest mode, which means no IPI is sent, and
after it has called kvm_vgic_flush_hwstate(), meaning it won't see
the updated GIC state until its next exit some time later for some
other reason. The receiving VCPU only needs to check this request
in VCPU RUN to handle it. By checking it, if it's pending, a
memory barrier will be issued that ensures all state is visible.
See "Ensuring Requests Are Seen" of
Documentation/virtual/kvm/vcpu-requests.rst
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
---
arch/arm/include/asm/kvm_host.h | 1 +
arch/arm64/include/asm/kvm_host.h | 1 +
virt/kvm/arm/arm.c | 7 +++++++
virt/kvm/arm/vgic/vgic.c | 9 +++++++--
4 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index fdd644c01c89..00ad56ee6455 100644
--- a/arch/arm/include/asm/kvm_host.h
+++ b/arch/arm/include/asm/kvm_host.h
@@ -46,6 +46,7 @@
#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
u32 *kvm_vcpu_reg(struct kvm_vcpu *vcpu, u8 reg_num, u32 mode);
int __attribute_const__ kvm_target_cpu(void);
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 9bd0d1040de9..0c4fd1f46e10 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -43,6 +43,7 @@
#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
int __attribute_const__ kvm_target_cpu(void);
int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
index ddc833987dfb..cac5c2f2ddba 100644
--- a/virt/kvm/arm/arm.c
+++ b/virt/kvm/arm/arm.c
@@ -580,6 +580,12 @@ static void check_vcpu_requests(struct kvm_vcpu *vcpu)
if (kvm_request_pending(vcpu)) {
if (kvm_check_request(KVM_REQ_SLEEP, vcpu))
vcpu_req_sleep(vcpu);
+
+ /*
+ * Clear IRQ_PENDING requests that were made to guarantee
+ * that a VCPU sees new virtual interrupts.
+ */
+ kvm_check_request(KVM_REQ_IRQ_PENDING, vcpu);
}
}
@@ -771,6 +777,7 @@ static int vcpu_interrupt_line(struct kvm_vcpu *vcpu, int number, bool level)
* trigger a world-switch round on the running physical CPU to set the
* virtual IRQ/FIQ fields in the HCR appropriately.
*/
+ kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
kvm_vcpu_kick(vcpu);
return 0;
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index aea080a2c443..c66feaca2a5d 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -286,8 +286,10 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq)
* won't see this one until it exits for some other
* reason.
*/
- if (vcpu)
+ if (vcpu) {
+ kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
kvm_vcpu_kick(vcpu);
+ }
return false;
}
@@ -333,6 +335,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq)
spin_unlock(&irq->irq_lock);
spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
+ kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
kvm_vcpu_kick(vcpu);
return true;
@@ -722,8 +725,10 @@ void vgic_kick_vcpus(struct kvm *kvm)
* a good kick...
*/
kvm_for_each_vcpu(c, vcpu, kvm) {
- if (kvm_vgic_vcpu_pending_irq(vcpu))
+ if (kvm_vgic_vcpu_pending_irq(vcpu)) {
+ kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
kvm_vcpu_kick(vcpu);
+ }
}
}
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 10/11] KVM: arm/arm64: PMU: remove request-less vcpu kick
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (8 preceding siblings ...)
2017-06-04 12:43 ` [PATCH v5 09/11] KVM: arm/arm64: use vcpu requests for irq injection Andrew Jones
@ 2017-06-04 12:44 ` Andrew Jones
2017-06-04 12:44 ` [PATCH v5 11/11] KVM: arm/arm64: timer: " Andrew Jones
2017-06-04 14:56 ` [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Christoffer Dall
11 siblings, 0 replies; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:44 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
Refactor PMU overflow handling in order to remove the request-less
vcpu kick. Now, since kvm_vgic_inject_irq() uses vcpu requests,
there should be no chance that a kick sent at just the wrong time
(between the VCPU's call to kvm_pmu_flush_hwstate() and before it
enters guest mode) results in a failure for the guest to see updated
GIC state until its next exit some time later for some other reason.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
---
virt/kvm/arm/pmu.c | 40 +++++++++++++++++++---------------------
1 file changed, 19 insertions(+), 21 deletions(-)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 4b43e7f3b158..2451607dc25e 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -203,6 +203,23 @@ static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
return reg;
}
+static void kvm_pmu_check_overflow(struct kvm_vcpu *vcpu)
+{
+ struct kvm_pmu *pmu = &vcpu->arch.pmu;
+ bool overflow = !!kvm_pmu_overflow_status(vcpu);
+
+ if (pmu->irq_level == overflow)
+ return;
+
+ pmu->irq_level = overflow;
+
+ if (likely(irqchip_in_kernel(vcpu->kvm))) {
+ int ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
+ pmu->irq_num, overflow);
+ WARN_ON(ret);
+ }
+}
+
/**
* kvm_pmu_overflow_set - set PMU overflow interrupt
* @vcpu: The vcpu pointer
@@ -210,37 +227,18 @@ static u64 kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
*/
void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val)
{
- u64 reg;
-
if (val == 0)
return;
vcpu_sys_reg(vcpu, PMOVSSET_EL0) |= val;
- reg = kvm_pmu_overflow_status(vcpu);
- if (reg != 0)
- kvm_vcpu_kick(vcpu);
+ kvm_pmu_check_overflow(vcpu);
}
static void kvm_pmu_update_state(struct kvm_vcpu *vcpu)
{
- struct kvm_pmu *pmu = &vcpu->arch.pmu;
- bool overflow;
-
if (!kvm_arm_pmu_v3_ready(vcpu))
return;
-
- overflow = !!kvm_pmu_overflow_status(vcpu);
- if (pmu->irq_level == overflow)
- return;
-
- pmu->irq_level = overflow;
-
- if (likely(irqchip_in_kernel(vcpu->kvm))) {
- int ret;
- ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
- pmu->irq_num, overflow);
- WARN_ON(ret);
- }
+ kvm_pmu_check_overflow(vcpu);
}
bool kvm_pmu_should_notify_user(struct kvm_vcpu *vcpu)
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v5 11/11] KVM: arm/arm64: timer: remove request-less vcpu kick
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (9 preceding siblings ...)
2017-06-04 12:44 ` [PATCH v5 10/11] KVM: arm/arm64: PMU: remove request-less vcpu kick Andrew Jones
@ 2017-06-04 12:44 ` Andrew Jones
2017-06-04 14:52 ` Christoffer Dall
2017-06-04 14:56 ` [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Christoffer Dall
11 siblings, 1 reply; 15+ messages in thread
From: Andrew Jones @ 2017-06-04 12:44 UTC (permalink / raw)
To: kvmarm, kvm; +Cc: marc.zyngier, cdall, pbonzini
The timer work is only scheduled for a VCPU when that VCPU is
blocked. This means we only need to wake it up, not kick (IPI)
it. While calling kvm_vcpu_kick() would just do the wake up,
and not kick, anyway, let's change this to avoid request-less
vcpu kicks, as they're generally not a good idea (see
"Request-less VCPU Kicks" in
Documentation/virtual/kvm/vcpu-requests.rst)
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
virt/kvm/arm/arch_timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
index 5976609ef27c..7933b1f8f7b7 100644
--- a/virt/kvm/arm/arch_timer.c
+++ b/virt/kvm/arm/arch_timer.c
@@ -95,7 +95,7 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
* If the vcpu is blocked we want to wake it up so that it will see
* the timer has expired when entering the guest.
*/
- kvm_vcpu_kick(vcpu);
+ kvm_vcpu_wake_up(vcpu);
}
static u64 kvm_timer_compute_delta(struct arch_timer_context *timer_ctx)
--
2.9.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v5 11/11] KVM: arm/arm64: timer: remove request-less vcpu kick
2017-06-04 12:44 ` [PATCH v5 11/11] KVM: arm/arm64: timer: " Andrew Jones
@ 2017-06-04 14:52 ` Christoffer Dall
0 siblings, 0 replies; 15+ messages in thread
From: Christoffer Dall @ 2017-06-04 14:52 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvmarm, kvm, marc.zyngier, pbonzini, rkrcmar
On Sun, Jun 04, 2017 at 02:44:01PM +0200, Andrew Jones wrote:
> The timer work is only scheduled for a VCPU when that VCPU is
> blocked. This means we only need to wake it up, not kick (IPI)
> it. While calling kvm_vcpu_kick() would just do the wake up,
> and not kick, anyway, let's change this to avoid request-less
> vcpu kicks, as they're generally not a good idea (see
> "Request-less VCPU Kicks" in
> Documentation/virtual/kvm/vcpu-requests.rst)
>
Reviewed-by: Christoffer Dall <cdall@linaro.org>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> virt/kvm/arm/arch_timer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c
> index 5976609ef27c..7933b1f8f7b7 100644
> --- a/virt/kvm/arm/arch_timer.c
> +++ b/virt/kvm/arm/arch_timer.c
> @@ -95,7 +95,7 @@ static void kvm_timer_inject_irq_work(struct work_struct *work)
> * If the vcpu is blocked we want to wake it up so that it will see
> * the timer has expired when entering the guest.
> */
> - kvm_vcpu_kick(vcpu);
> + kvm_vcpu_wake_up(vcpu);
> }
>
> static u64 kvm_timer_compute_delta(struct arch_timer_context *timer_ctx)
> --
> 2.9.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests
2017-06-04 12:43 [PATCH v5 00/11] KVM: arm/arm64: race fixes and vcpu requests Andrew Jones
` (10 preceding siblings ...)
2017-06-04 12:44 ` [PATCH v5 11/11] KVM: arm/arm64: timer: " Andrew Jones
@ 2017-06-04 14:56 ` Christoffer Dall
11 siblings, 0 replies; 15+ messages in thread
From: Christoffer Dall @ 2017-06-04 14:56 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvmarm, kvm, marc.zyngier, pbonzini, rkrcmar
Hi Drew,
On Sun, Jun 04, 2017 at 02:43:50PM +0200, Andrew Jones wrote:
> This series fixes some hard to produce races by introducing the use of
> vcpu requests. I've tested the series on a Mustang and ThunderX and
> compile-tested the ARM bits.
>
> Patch 3/11 adds documentation, as, at least for me, understanding vcpu
> request interplay with vcpu kicks and vcpu mode and the memory barriers
> that interplay implies, is exhausting.
I have applied these to kvmarm/queue.
Will give them a bit of testing over the next week (and hope others do
as well), and will then otherwise apply them to next.
Thanks for all the hard work!
-Christoffer
^ permalink raw reply [flat|nested] 15+ messages in thread