linux-kernel - Re: [PATCH 4/5] KVM: add __kvm_request_needs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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

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@...hat.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