From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751562AbdBWPoD (ORCPT ); Thu, 23 Feb 2017 10:44:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:56648 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751195AbdBWPn7 (ORCPT ); Thu, 23 Feb 2017 10:43:59 -0500 Date: Thu, 23 Feb 2017 16:43:54 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Christian Borntraeger Cc: David Hildenbrand , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Paolo Bonzini , Andrew Jones , Marc Zyngier , Cornelia Huck , James Hogan , Paul Mackerras , Christoffer Dall Subject: Re: [PATCH 4/5] KVM: add __kvm_request_needs_mb Message-ID: <20170223154354.GB8342@potion> References: <20170216160449.13094-1-rkrcmar@redhat.com> <20170216160449.13094-5-rkrcmar@redhat.com> <865e0ec3-6918-5372-0c85-af2181209749@redhat.com> <35bdb122-3783-91b1-56b5-27bda4c923ad@de.ibm.com> <7f521412-1e8f-e519-1274-5db3ec7d36b8@redhat.com> <20170222151704.GA3856@potion> <58da709b-7000-2b21-a107-06e278dbc3aa@de.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <58da709b-7000-2b21-a107-06e278dbc3aa@de.ibm.com> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 23 Feb 2017 15:43:59 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 2017-02-22 20:23+0100, Christian Borntraeger: > On 02/22/2017 04:17 PM, Radim Krčmář wrote: >> [Oops, the end of this thread got dragged into a mark-as-read spree ...] >> >> 2017-02-17 11:13+0100, David Hildenbrand: >>>>> 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. >>> >>> While what you said is 100% correct, I had something else in mind that >>> hindered using vcpu_kick() and especially kvm_make_all_cpus_request(). >>> And I remember that being related to how preemption and >>> OUTSIDE_GUEST_MODE is handled. I think this boils down to what would >>> have to be implemented in kvm_arch_vcpu_should_kick(). >>> >>> x86 can track the guest state using vcpu->mode, because they can be sure >>> that the guest can't reschedule while in the critical guest entry/exit >>> section. This is not true for s390x, as preemption is enabled. That's >>> why vcpu->mode cannot be used in its current form to track if a VCPU is >>> in/oustide/exiting guest mode. And kvm_make_all_cpus_request() currently >>> relies on this setting. >>> >>> For now, calling vcpu_kick() on s390x will result in a BUG(). >>> >>> >>> On s390x, there are 3 use cases I see for requests: >>> >>> 1. Remote requests that need a sync >>> >>> Make a request, wait until SIE has been left and make sure the request >>> will be processed before re-entering the SIE. e.g. KVM_REQ_RELOAD_MMU >>> notifier in mmu notifier you mentioned. Also KVM_REQ_DISABLE_IBS is a >>> candidate. >> >> Btw. aren't those requests racy? >> >> void exit_sie(struct kvm_vcpu *vcpu) >> { >> atomic_or(CPUSTAT_STOP_INT, &vcpu->arch.sie_block->cpuflags); >> >> If you get stalled here and the target VCPU handles the request and >> reenters SIE in the meantime, then you'll wait until its next exit. >> (And miss an unbounded amount of exits in the worst case.) >> >> while (vcpu->arch.sie_block->prog0c & PROG_IN_SIE) >> cpu_relax(); >> } >> > > Its not racy for the purpose it was originally made for (get the vcpu > out of SIE before we unmap a guest prefix page) as the MMU_RELOAD handler > will wait for the pte lock which is held by the code that called > kvm_s390_sync_request(KVM_REQ_MMU_RELOAD, vcpu). > > We also have the guarantee that after returning from kvm_s390_sync_request > we will have that request be handled before we reenter the guest, which is > all we need for DISABLE_IBS. > > But yes, all non MMU_RELOAD users might wait longer, possibly several guest > exits. We never noticed that as requests are really a seldom event. Basically > unmapping of the guest prefix page due to paging and migration, switching > between 1 and more guest cpus and some other seldom events. Ok, thanks for the info. I don't think that we'll find too many use-cases to demand inclusion into a generic kick/request API, so having a function that waits until a VCPU is out of guest mode would be more suited for generic code.