Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1618329 > unrolled thread

[PATCH RFC 5/6] KVM: mark requests that do not need a wakeup

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH RFC 5/6] KVM: mark requests that do not need a wakeup Radim Krčmář <rkrcmar@redhat.com> - 2017-04-06 22:30 +0200
    Re: [PATCH RFC 5/6] KVM: mark requests that do not need a wakeup Marc Zyngier <marc.zyngier@arm.com> - 2017-04-07 10:30 +0200
      Re: [PATCH RFC 5/6] KVM: mark requests that do not need a wakeup Radim Krčmář <rkrcmar@redhat.com> - 2017-04-07 14:30 +0200

#1618329 — [PATCH RFC 5/6] KVM: mark requests that do not need a wakeup

Some operations must ensure that the guest is not running with stale
data, but if the guest is halted, then the update can wait until another
event happens.  kvm_make_all_requests() currently doesn't wake up, so we
can mark all requests used with it.

First 8 bits were arbitrarily reserved for request numbers.

Most uses of requests have the request type as a constant, so a compiler
will optimize the '&'.

An alternative would be to have an inline function that would return
whether the request needs a wake-up or not, but I like this one better
even though it might produce worse assembly.

Suggested-by: Christoffer Dall <cdall@linaro.org>
Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
  Btw. do you recall which macro allowed to define bitmasks?  (It has
  two arguments, FROM and TO.)

 arch/arm/include/asm/kvm_host.h   |  2 +-
 arch/arm64/include/asm/kvm_host.h |  2 +-
 arch/x86/include/asm/kvm_host.h   |  6 +++---
 include/linux/kvm_host.h          | 12 +++++++-----
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h
index de67ce647501..49358f20d36f 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	8
+#define KVM_REQ_VCPU_EXIT	(8 | 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 522e4f60976e..1c9458a7ec92 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	8
+#define KVM_REQ_VCPU_EXIT	(8 | KVM_REQUEST_NO_WAKEUP)
 
 int __attribute_const__ kvm_target_cpu(void);
 int kvm_reset_vcpu(struct kvm_vcpu *vcpu);
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d962fa998a6f..901fc1ab1206 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -61,10 +61,10 @@
 #define KVM_REQ_PMI               19
 #define KVM_REQ_SMI               20
 #define KVM_REQ_MASTERCLOCK_UPDATE 21
-#define KVM_REQ_MCLOCK_INPROGRESS 22
-#define KVM_REQ_SCAN_IOAPIC       23
+#define KVM_REQ_MCLOCK_INPROGRESS (22 | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_SCAN_IOAPIC       (23 | KVM_REQUEST_NO_WAKEUP)
 #define KVM_REQ_GLOBAL_CLOCK_UPDATE 24
-#define KVM_REQ_APIC_PAGE_RELOAD  25
+#define KVM_REQ_APIC_PAGE_RELOAD  (25 | KVM_REQUEST_NO_WAKEUP)
 #define KVM_REQ_HV_CRASH          26
 #define KVM_REQ_IOAPIC_EOI_EXIT   27
 #define KVM_REQ_HV_RESET          28
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 51737d6401ae..2b706704bd92 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -115,12 +115,14 @@ static inline bool is_error_page(struct page *page)
 	return IS_ERR(page);
 }
 
+#define KVM_REQUEST_MASK           0xff
+#define KVM_REQUEST_NO_WAKEUP      BIT(8)
 /*
  * Architecture-independent vcpu->requests bit members
  * Bits 4-7 are reserved for more arch-independent bits.
  */
-#define KVM_REQ_TLB_FLUSH          0
-#define KVM_REQ_MMU_RELOAD         1
+#define KVM_REQ_TLB_FLUSH          (0 | KVM_REQUEST_NO_WAKEUP)
+#define KVM_REQ_MMU_RELOAD         (1 | KVM_REQUEST_NO_WAKEUP)
 #define KVM_REQ_PENDING_TIMER      2
 #define KVM_REQ_UNHALT             3
 
@@ -1076,17 +1078,17 @@ static inline void kvm_make_request(int req, struct kvm_vcpu *vcpu)
 	 * caller.  Paired with the smp_mb__after_atomic in kvm_check_request.
 	 */
 	smp_wmb();
-	set_bit(req, &vcpu->requests);
+	set_bit(req & KVM_REQUEST_MASK, &vcpu->requests);
 }
 
 static inline bool kvm_test_request(int req, struct kvm_vcpu *vcpu)
 {
-	return test_bit(req, &vcpu->requests);
+	return test_bit(req & KVM_REQUEST_MASK, &vcpu->requests);
 }
 
 static inline void kvm_clear_request(int req, struct kvm_vcpu *vcpu)
 {
-	clear_bit(req, &vcpu->requests);
+	clear_bit(req & KVM_REQUEST_MASK, &vcpu->requests);
 }
 
 static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
-- 
2.12.0

[toc] | [next] | [standalone]

#1618583

On 06/04/17 21:20, Radim Krčmář wrote:
> Some operations must ensure that the guest is not running with stale
> data, but if the guest is halted, then the update can wait until another
> event happens.  kvm_make_all_requests() currently doesn't wake up, so we
> can mark all requests used with it.
> 
> First 8 bits were arbitrarily reserved for request numbers.
> 
> Most uses of requests have the request type as a constant, so a compiler
> will optimize the '&'.
> 
> An alternative would be to have an inline function that would return
> whether the request needs a wake-up or not, but I like this one better
> even though it might produce worse assembly.
> 
> Suggested-by: Christoffer Dall <cdall@linaro.org>
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>   Btw. do you recall which macro allowed to define bitmasks?  (It has
>   two arguments, FROM and TO.)

GENMASK (and its _ULL variant), defined in include/linux/bitops.h.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...

[toc] | [prev] | [next] | [standalone]

#1618725

2017-04-07 09:27+0100, Marc Zyngier:
> On 06/04/17 21:20, Radim Krčmář wrote:
>> Some operations must ensure that the guest is not running with stale
>> data, but if the guest is halted, then the update can wait until another
>> event happens.  kvm_make_all_requests() currently doesn't wake up, so we
>> can mark all requests used with it.
>> 
>> First 8 bits were arbitrarily reserved for request numbers.
>> 
>> Most uses of requests have the request type as a constant, so a compiler
>> will optimize the '&'.
>> 
>> An alternative would be to have an inline function that would return
>> whether the request needs a wake-up or not, but I like this one better
>> even though it might produce worse assembly.
>> 
>> Suggested-by: Christoffer Dall <cdall@linaro.org>
>> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
>> ---
>>   Btw. do you recall which macro allowed to define bitmasks?  (It has
>>   two arguments, FROM and TO.)
> 
> GENMASK (and its _ULL variant), defined in include/linux/bitops.h.

Thank you, it is under BIT() ... I am blind.

> +#define KVM_REQUEST_MASK           0xff

The 0xff should be "GENMASK(7,0)".

First 8 bits is plenty and should be fast even if the compiler doesn't
optimize the masking because request is not constant.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web