From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH] Don't call cpu_synchronize_state() in apic_init_reset() Date: Wed, 23 Sep 2009 17:45:37 +0200 Message-ID: <4ABA42A1.4010904@web.de> References: <1253631112-26124-1-git-send-email-gleb@redhat.com> <1253631112-26124-3-git-send-email-gleb@redhat.com> <4AB9E3BC.7060304@redhat.com> <4ABA399F.6030003@web.de> <4ABA3C14.7080803@redhat.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig3045CA494E614BC17FF55659" Cc: Gleb Natapov , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from fmmailgate01.web.de ([217.72.192.221]:47341 "EHLO fmmailgate01.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751333AbZIWPqE (ORCPT ); Wed, 23 Sep 2009 11:46:04 -0400 In-Reply-To: <4ABA3C14.7080803@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig3045CA494E614BC17FF55659 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Avi Kivity wrote: > On 09/23/2009 06:07 PM, Jan Kiszka wrote: >> Avi Kivity wrote: >> =20 >>> On 09/22/2009 05:51 PM, Gleb Natapov wrote: >>> =20 >>>> Each caller of the function already calls it. >>>> >>>> Signed-off-by: Gleb Natapov >>>> --- >>>> hw/apic.c | 1 - >>>> 1 files changed, 0 insertions(+), 1 deletions(-) >>>> >>>> diff --git a/hw/apic.c b/hw/apic.c >>>> index 3a2e128..a9d1fb8 100644 >>>> --- a/hw/apic.c >>>> +++ b/hw/apic.c >>>> @@ -488,7 +488,6 @@ void apic_init_reset(CPUState *env) >>>> if (!s) >>>> return; >>>> >>>> - cpu_synchronize_state(env); >>>> s->tpr =3D 0; >>>> s->spurious_vec =3D 0xff; >>>> s->log_dest =3D 0; >>>> >>>> =20 >>> Still, it's safer to live this in. >>> >>> =20 >> Yet another diff to upstream... >> >> It's really time to stabilize this still a bit fuzzy register sync >> model, also in qemu-kvm. If you think we need it, let us push it >> upstream - with a sound explanation, and maybe even with more sync >> points for the sake of consistency. >> >> =20 >=20 > Functions calling each other in the same subsystem can rely on callers > calling cpu_synchronize_state(). Across subsystems, that's another > matter, exported functions should try not to rely on implementation > details of their callers. >=20 > (You might argue that the apic is not separate subsystem wrt an x86 cpu= , > and I'm not sure I have a counterargument) >=20 I do accept this argument. It's just that my feeling is that we are lacking proper review of the required call sites of cpu_sychronize_state and rather put it where some regression popped up (and that only in qemu-kvm...). The new rule is: Synchronize the states before accessing registers (or in-kernel devices) the first time after a vmexit to user space. But, e.g., I do not see where we do this on CPU reset. Jan --------------enig3045CA494E614BC17FF55659 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.9 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org iEYEARECAAYFAkq6QqUACgkQitSsb3rl5xTufQCfY7NikWaGB7wxsLbaLooO1qAU ehgAniu5aYRaelZ5SKmZUe/+mLFoJphn =YJMW -----END PGP SIGNATURE----- --------------enig3045CA494E614BC17FF55659--