From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2][RFC] Kernel changes for HPET legacy mode (v7) Date: Thu, 18 Jun 2009 21:04:32 +0200 Message-ID: <4A3A8FC0.1000606@web.de> References: <1245329246-17526-1-git-send-email-eak@us.ibm.com> <1245329246-17526-3-git-send-email-eak@us.ibm.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig58E50EAC8F6707AD38CAAC05" Cc: avi@redhat.com, kvm@vger.kernel.org To: Beth Kon Return-path: Received: from fmmailgate03.web.de ([217.72.192.234]:36117 "EHLO fmmailgate03.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755897AbZFRTEd (ORCPT ); Thu, 18 Jun 2009 15:04:33 -0400 In-Reply-To: <1245329246-17526-3-git-send-email-eak@us.ibm.com> Sender: kvm-owner@vger.kernel.org List-ID: This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig58E50EAC8F6707AD38CAAC05 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Beth Kon wrote: > When kvm is in hpet_legacy_mode, the hpet is providing the=20 > timer interrupt and the pit should not be. So in legacy mode, the pit t= imer is > destroyed, but the *state* of the pit is maintained. So if kvm or the g= uest > tries to modify the state of the pit, this modification is accepted, *e= xcept*=20 > that the timer isn't actually started. When we exit hpet_legacy_mode,=20 > the current state of the pit (which is up to date since we've been=20 > accepting modifications) is used to restart the pit timer. >=20 > The saved_mode code in kvm_pit_load_count temporarily changes mode to=20 > 0xff in order to destroy the timer, but then restores the actual value,= > again maintaining "current" state of the pit for possible later reenabl= ement. >=20 > changes from v6: >=20 > - Added ioctl interface for legacy mode in order not to break the abi. >=20 >=20 > Signed-off-by: Beth Kon =2E.. > @@ -1986,7 +1987,24 @@ static int kvm_vm_ioctl_set_pit(struct kvm *kvm,= struct kvm_pit_state *ps) > int r =3D 0; > =20 > memcpy(&kvm->arch.vpit->pit_state, ps, sizeof(struct kvm_pit_state));= > - kvm_pit_load_count(kvm, 0, ps->channels[0].count); > + kvm_pit_load_count(kvm, 0, ps->channels[0].count, 0); > + return r; > +} > + > +static int kvm_vm_ioctl_get_hpet_legacy_mode(struct kvm *kvm, u8 *mode= ) > +{ > + int r =3D 0; > + *mode =3D kvm->arch.vpit->pit_state.hpet_legacy_mode; > + return r; > +} This only applies if we go for a separate mode IOCTL: The legacy mode is not directly modifiable by the guest. Is it planned to add in-kernel hpet support? Otherwise get_hpet_legacy_mode looks a bit like overkill given that user space could easily track the state. > + > +static int kvm_vm_ioctl_set_hpet_legacy_mode(struct kvm *kvm, u8 *mode= ) > +{ > + int r =3D 0, start =3D 0; > + if (kvm->arch.vpit->pit_state.hpet_legacy_mode =3D=3D 0 && *mode =3D=3D= 1) Here you check more mode =3D=3D 1, but legacy_mode is only checked for !=3D= 0. I would make this consistent. > + start =3D 1; > + kvm->arch.vpit->pit_state.hpet_legacy_mode =3D *mode; > + kvm_pit_load_count(kvm, 0, kvm->arch.vpit->pit_state.channels[0].coun= t, start); > return r; > } > =20 > @@ -2047,6 +2065,7 @@ long kvm_arch_vm_ioctl(struct file *filp, > struct kvm_pit_state ps; > struct kvm_memory_alias alias; > struct kvm_pit_config pit_config; > + u8 hpet_legacy_mode; Hmm, stead of introducing a new pair of singe-purpose IOCTLs, why not add KVM_GET/SET_PIT2 which exchanges an extended kvm_pit_state2. And that struct should also include some flags field and enough padding to be potentially extended yet again in the future. In that case I see no problem having also a mode read-back interface. Jan --------------enig58E50EAC8F6707AD38CAAC05 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 iEYEARECAAYFAko6j8MACgkQniDOoMHTA+lTMgCfSVnb64kHS8aI0DVnRS30u/bT ZsQAn1YDnkHihMPTS6AAts2mUpB1y7s/ =Kvwk -----END PGP SIGNATURE----- --------------enig58E50EAC8F6707AD38CAAC05--