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]

Date:   Fri, 17 Feb 2017 09:46:50 +0100
From:   Christian Borntraeger <borntraeger@...ibm.com>
To:     David Hildenbrand <david@...hat.com>,
        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>,
        Cornelia Huck <cornelia.huck@...ibm.com>,
        James Hogan <james.hogan@...tec.com>,
        Paul Mackerras <paulus@...abs.org>,
        Christoffer Dall <christoffer.dall@...aro.org>
Subject: Re: [PATCH 4/5] KVM: add __kvm_request_needs_mb

On 02/16/2017 08:49 PM, David Hildenbrand wrote:
> 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)

No the reason was that there are some requests that need to be handled
outside run SIE. For example one reason was the guest prefix page.
This must be mapped read/write ALL THE TIME when a guest is running,
otherwise the host might crash. So we have to exit SIE and make sure that
it does not reenter, therefore we use the RELOAD_MMU request from a notifier
that is called from page table functions, whenever memory management decides
to unmap/write protect (dirty pages tracking, reference tracking, page migration
or compaction...)

SMP-based request wills kick out the guest, but for some thing like the
one above it will be too late.

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