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  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]
Date:   Thu, 16 Feb 2017 22:31:45 +0100
From:   Radim Krčmář <>
To:     David Hildenbrand <>
        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

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

Thanks for the info!

Powered by blists - more mailing lists