From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933107AbdBQJa2 convert rfc822-to-8bit (ORCPT ); Fri, 17 Feb 2017 04:30:28 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:42696 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751646AbdBQJaY (ORCPT ); Fri, 17 Feb 2017 04:30:24 -0500 Date: Fri, 17 Feb 2017 10:30:14 +0100 From: Cornelia Huck To: Radim =?UTF-8?B?S3LEjW3DocWZ?= Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Andrew Jones , Marc Zyngier , Christian Borntraeger , James Hogan , Paul Mackerras , Christoffer Dall Subject: Re: [PATCH 1/5] KVM: change API for requests to match bit operations In-Reply-To: <20170216160449.13094-2-rkrcmar@redhat.com> References: <20170216160449.13094-1-rkrcmar@redhat.com> <20170216160449.13094-2-rkrcmar@redhat.com> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17021709-0024-0000-0000-000002B7E32B X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17021709-0025-0000-0000-000022704532 Message-Id: <20170217103014.5ada1f1c.cornelia.huck@de.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-17_07:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702170090 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 16 Feb 2017 17:04:45 +0100 Radim Krčmář 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;