From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933670AbdBPVbv (ORCPT ); Thu, 16 Feb 2017 16:31:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60106 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933087AbdBPVbt (ORCPT ); Thu, 16 Feb 2017 16:31:49 -0500 Date: Thu, 16 Feb 2017 22:31:45 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: David Hildenbrand Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Andrew Jones , Marc Zyngier , Christian Borntraeger , Cornelia Huck , James Hogan , Paul Mackerras , Christoffer Dall Subject: Re: [PATCH 4/5] KVM: add __kvm_request_needs_mb Message-ID: <20170216213144.GH6633@potion> References: <20170216160449.13094-1-rkrcmar@redhat.com> <20170216160449.13094-5-rkrcmar@redhat.com> <865e0ec3-6918-5372-0c85-af2181209749@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <865e0ec3-6918-5372-0c85-af2181209749@redhat.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 16 Feb 2017 21:31:50 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-02-16 20:49+0100, David Hildenbrand: > Am 16.02.2017 um 17:04 schrieb Radim Krčmář: >> A macro to optimize requests that do not need a memory barrier because >> they have no dependencies. An architecture can implement a function >> that says which requests do not need memory barriers when handling them. >> >> Signed-off-by: Radim Krčmář >> --- >> include/linux/kvm_host.h | 41 +++++++++++++++++++++++++++++++++++++---- >> virt/kvm/kvm_main.c | 3 ++- >> 2 files changed, 39 insertions(+), 5 deletions(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index d899473859d3..2cc438685af8 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -1097,8 +1097,8 @@ static inline int kvm_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) >> * 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. >> + * TODO: the API does not distinguish local and remote requests -- remote >> + * should contain kvm_vcpu_kick(). >> */ > > Just for your info, kvm_vcpu_kick() and kvm_make_all_cpus_request() do > not work on s390x (and in its current form never will). I tried to make > it work once, but I gave up. > > s390x uses kvm_s390_sync_request()->kvm_s390_vcpu_request() to kick a > guest out of guest mode. A special bit in the SIE control block is used > to perform the kick (exit_sie(), STOP request), and another bit to > prevent the guest from reentering the SIE, until the request has been > handled (to avoid races). Hm, kvm_s390_vcpu_wakeup() looks more suitable as the s390 implementation of kvm_vcpu_kick() (which is what we want to be connected with kvm_request). I think that kvm_s390_sync_request() is a different idea as it does not call swait_active(), so the request is delayed if the VCPU is halted. And kvm_s390_sync_request() it also waits for the VCPU to actually exit, which pushes it even further away from what other requests do. :) I would rather use bitops/barriers/kicks directly if the use of kvm_request helpers is too diverse. > This is really complicated stuff, and the basic reason for it (if I > remember correctly) is that s390x does reenable all interrupts when > entering the sie (see kvm-s390.c:__vcpu_run()). So the fancy smp-based > kicks don't work (as it is otherwise just racy), and if I remember > correctly, SMP reschedule signals (s390x external calls) would be > slower. (Christian, please correct me if I'm wrong) Having a different mechanism for the exit itself is ok, just the general behavior has to stay the same -- kvm_vcpu_kick() does whatever is needed to let the VCPU notice possible changes in state as soon as possible and doesn't care about the VCPU any further. If other architectures had a fast mechanism that forced an immediate VCPU exit, they would gladly use it as well. > So this statement, is at least from a s390x point of view wrong. The > kvm_vcpu_kick() function would have to be rerouted to an appropriate > s390x implementation (or that whole smp and OUTSIDE_GUEST_MODE stuff > would have to be factored out). I agree. What about starting by adding __weak on functions that are currently "#ifndef CONFIG_S390" and letting s390 completely reimplement them? Thanks for the info!