Re: [PATCH 4/5] KVM: add __kvm_request_needs_mb

From: David Hildenbrand <david@redhat.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Andrew Jones <drjones@redhat.com>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Cornelia Huck <cornelia.huck@de.ibm.com>,
	James Hogan <james.hogan@imgtec.com>,
	Paul Mackerras <paulus@ozlabs.org>,
	Christoffer Dall <christoffer.dall@linaro.org>,
	Christian Borntraeger <borntraeger@de.ibm.com>
Subject: Re: [PATCH 4/5] KVM: add __kvm_request_needs_mb
Date: Thu, 16 Feb 2017 20:49:22 +0100	[thread overview]
Message-ID: <865e0ec3-6918-5372-0c85-af2181209749@redhat.com> (raw)
In-Reply-To: <20170216160449.13094-5-rkrcmar@redhat.com>

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ář <rkrcmar@redhat.com>
> ---
>  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).

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)

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).

-- 
Thanks,

David