On Thu, 16 Feb 2017 17:04:45 +0100
Radim Krčmář <rkrcmar@xxxxxxxxxx> wrote:
> kvm_make_request was a wrapper that added barriers to bit_set and
> kvm_check_request did the same for bit_test and bit_check, but the name
> was not very obvious and we were also lacking operations that cover
> bit_test and bit_clear, which resulted in an inconsistent use.
>
> The renaming:
> kvm_request_set <- kvm_make_request
> kvm_request_test_and_clear <- kvm_check_request
>
> Automated with coccinelle script:
> @@
> expression VCPU, REQ;
> @@
> -kvm_make_request(REQ, VCPU)
> +kvm_request_set(REQ, VCPU)
>
> @@
> expression VCPU, REQ;
> @@
> -kvm_check_request(REQ, VCPU)
> +kvm_request_test_and_clear(REQ, VCPU)
Forgot your s-o-b?
> ---
> arch/mips/kvm/emulate.c | 2 +-
> arch/mips/kvm/trap_emul.c | 2 +-
> arch/powerpc/kvm/book3s_pr.c | 2 +-
> arch/powerpc/kvm/booke.c | 16 +++---
> arch/powerpc/kvm/powerpc.c | 2 +-
> arch/s390/kvm/kvm-s390.c | 22 ++++----
> arch/s390/kvm/kvm-s390.h | 4 +-
> arch/s390/kvm/priv.c | 4 +-
> arch/x86/kvm/hyperv.c | 14 ++---
> arch/x86/kvm/i8259.c | 2 +-
> arch/x86/kvm/lapic.c | 22 ++++----
> arch/x86/kvm/mmu.c | 14 ++---
> arch/x86/kvm/pmu.c | 6 +-
> arch/x86/kvm/svm.c | 12 ++--
> arch/x86/kvm/vmx.c | 30 +++++-----
> arch/x86/kvm/x86.c | 128 +++++++++++++++++++++----------------------
> include/linux/kvm_host.h | 30 ++++++++--
> virt/kvm/kvm_main.c | 4 +-
> 18 files changed, 167 insertions(+), 149 deletions(-)
(...lots of coccinelle changes...)
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 8d69d5150748..21f91de3098b 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -1084,24 +1084,42 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
>
> #endif /* CONFIG_HAVE_KVM_EVENTFD */
>
> -static inline void kvm_make_request(int req, struct kvm_vcpu *vcpu)
> +/*
> + * An API for setting KVM requests.
> + * The general API design is inspired by bit_* API.
> + *
> + * A request can be set either to itself or to a remote VCPU. If the request
> + * is set to a remote VCPU, then the VCPU needs to be notified, which is
> + * usually done with kvm_vcpu_kick().
> + * The request can also mean that some data is ready, so a remote requests
> + * needs a smp_wmb(). i.e. there are three types of requests:
> + * 1) local request
> + * 2) remote request with no data (= kick)
> + * 3) remote request with data (= kick + mb)
> + *
> + * TODO: the API is inconsistent -- a request doesn't call kvm_vcpu_kick(), but
> + * forces smp_wmb() for all requests.
> + */
> +static inline void kvm_request_set(unsigned req, struct kvm_vcpu *vcpu)
Should we make req unsigned long as well, so that it matches the bit
api even more?
> {
> /*
> - * Ensure the rest of the request is published to kvm_check_request's
> - * caller. Paired with the smp_mb__after_atomic in kvm_check_request.
> + * Ensure the rest of the request is published to
> + * kvm_request_test_and_clear's caller.
> + * Paired with the smp_mb__after_atomic in kvm_request_test_and_clear.
> */
> smp_wmb();
> set_bit(req, &vcpu->requests);
> }
>
> -static inline bool kvm_check_request(int req, struct kvm_vcpu *vcpu)
> +static inline bool kvm_request_test_and_clear(unsigned req, struct kvm_vcpu *vcpu)
> {
> if (test_bit(req, &vcpu->requests)) {
> clear_bit(req, &vcpu->requests);
>
> /*
> - * Ensure the rest of the request is visible to kvm_check_request's
> - * caller. Paired with the smp_wmb in kvm_make_request.
> + * Ensure the rest of the request is visible to
> + * kvm_request_test_and_clear's caller.
> + * Paired with the smp_wmb in kvm_request_set.
> */
> smp_mb__after_atomic();
> return true;
