Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755229AbdDFVCU (ORCPT ); Thu, 6 Apr 2017 17:02:20 -0400 Received: from mailapp01.imgtec.com ([195.59.15.196]:36887 "EHLO imgpgp01.kl.imgtec.org" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752974AbdDFVCS (ORCPT ); Thu, 6 Apr 2017 17:02:18 -0400 X-PGP-Universal: processed; by imgpgp01.kl.imgtec.org on Thu, 06 Apr 2017 23:08:33 +0100 Date: Thu, 6 Apr 2017 22:02:15 +0100 From: James Hogan To: Radim =?utf-8?B?S3LEjW3DocWZ?= CC: , , Christoffer Dall , Andrew Jones , Marc Zyngier , Paolo Bonzini , Christian Borntraeger , Cornelia Huck , Paul Mackerras Subject: Re: [PATCH RFC 1/6] KVM: fix guest_mode optimization in kvm_make_all_cpus_request() Message-ID: <20170406210215.GV31606@jhogan-linux.le.imgtec.org> References: <20170406202056.18379-1-rkrcmar@redhat.com> <20170406202056.18379-2-rkrcmar@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="tphTSO44SEA4WSDH" Content-Disposition: inline In-Reply-To: <20170406202056.18379-2-rkrcmar@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Originating-IP: [192.168.154.110] X-ESG-ENCRYPT-TAG: 1b7d744b Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4288 Lines: 109 --tphTSO44SEA4WSDH Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Apr 06, 2017 at 10:20:51PM +0200, Radim Kr=C4=8Dm=C3=A1=C5=99 wrote: > We have kvm_arch_vcpu_should_kick() to decide whether the target cpu > needs to be kicked. The previous condition was wrong, because > architectures that don't use vcpu->mode would not get interrupts and > also suboptimal, because it sent IPI in cases where none was necessary. >=20 > The situation is even more convoluted. MIPS and POWERPC return 1 from > kvm_arch_vcpu_should_kick(), but implement vcpu->mode for some reason, > so now they might kick uselessly. This is not a huge problem. Whoops. I hadn't spotted kvm_arch_vcpu_should_kick() when I added vcpu->mode stuff in 4.11... I'm guessing I need to implement that similar to ARM / x86... though MIPS doesn't use kvm_vcpu_kick() yet. >=20 > s390, on the other hand, never changed vcpu->mode, so it would always be > OUTSIDE_GUEST_MODE before and therefore didn't send IPIs. > I don't see a reason why s390 had kvm_make_all_cpus_request() that did > nothing but set the bit in vcpu->request, so the new behavior seems > better. >=20 > Signed-off-by: Radim Kr=C4=8Dm=C3=A1=C5=99 > --- > arch/s390/kvm/kvm-s390.c | 4 +--- > virt/kvm/kvm_main.c | 2 +- > 2 files changed, 2 insertions(+), 4 deletions(-) >=20 > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index fd6cd05bb6a7..45b6d9ca5d24 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -2130,9 +2130,7 @@ static void kvm_gmap_notifier(struct gmap *gmap, un= signed long start, > =20 > int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu) > { > - /* kvm common code refers to this, but never calls it */ > - BUG(); > - return 0; > + return 1; > } > =20 > static int kvm_arch_vcpu_ioctl_get_one_reg(struct kvm_vcpu *vcpu, > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index f489167839c4..2389e9c41cd2 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -187,7 +187,7 @@ bool kvm_make_all_cpus_request(struct kvm *kvm, unsig= ned int req) > smp_mb__after_atomic(); > =20 > if (cpus !=3D NULL && cpu !=3D -1 && cpu !=3D me && > - kvm_vcpu_exiting_guest_mode(vcpu) !=3D OUTSIDE_GUEST_MODE) > + kvm_arch_vcpu_should_kick(vcpu)) This presumably changes the behaviour on x86, from !=3D OUTSIDE_GUEST_MODE to =3D=3D IN_GUEST_MODE. so: - you'll no longer get IPIs if its in READING_SHADOW_PAGE_TABLES (which MIPS also now uses when accessing mappings outside of guest mode and depends upon to wait until the old mappings are no longer in use). - you'll no longer get IPIs if its in EXITING_GUEST_MODE (i.e. if you get two of these in quick succession only the first will wait for the IPI, which might work as long as they're already serialised but it still feels wrong). But it doesn't seem right to change the kvm_arch_vcpu_should_kick() implementations to check kvm_vcpu_exiting_guest_mode(vcpu) !=3D OUTSIDE_GUEST_MODE to match condition either, since kvm_vcpu_kick() doesn't seem to need synchronisation, only to know that it won't be delayed in reaching hypervisor code. Cheers James > cpumask_set_cpu(cpu, cpus); > } > if (unlikely(cpus =3D=3D NULL)) > --=20 > 2.12.0 >=20 --tphTSO44SEA4WSDH Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- iQIcBAEBCAAGBQJY5qzGAAoJEGwLaZPeOHZ6MNkP/RsS2bvXybXzd2v9MMJkubcB WYevDLFkKc1GpuW4qlvvQMF/e6tDkAW+kY0LVu8Rxr3ZcZGGmpZSGgXiJB5eBKUj bCnYuMmBVfjnlzUkU9BDeNjEnQVgHiGfvw/VXYTc34FVG72Xbh0G5cNH7CHNgc/X 1KzDlAPjQ1eDZtFuTP2S3aT7HwaZrjbgGV3BeqJG5/UmOb6QrD9LzD46ejO8Kj96 hebSaUgk1bMlGVhraE/UvsfBZaa8OMIH4hqI6gXtTZfvJwaS1ymez0l6mf70EC/9 XHnKvtJXXqlEpu/JcwM6aN+GJnjk4csH8phBhi2vmQQ5OMgHXK5kZsjF3/1JNL3d jll+QtWoKgU3CKUtG1UMV9GRcqRwYX4CZy5vCGwC6p+KKYjRCwry/qBbz8UuCALo IastIwWpeV+5Wszdz+eDnHN5bBKxv5ro0J+ezJV6zT5lJtqZ3q5vGhlq9It5KNwa sem2gLXhSB1kRyVx07CNLQj0O4a8z0WZvVi9rjeuiobLO3hTIu7yj+UMqmZe4eBo YMr63hRejD1Ud6i286fEgK/8igpdfGQKUmmEZXxe5Xs7fX+sgMTn3Q4IEVJM19Jh p58g/8zNOfkozzSe2NkA+N1nAGJdl0zZs5+QAgNM+zTf6Std5mP7XOYnmM7R26LI nUWTiRThAUX5ceGoavNo =OcG5 -----END PGP SIGNATURE----- --tphTSO44SEA4WSDH--