From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vadim Rozenfeld Subject: Re: [RFC PATCH 1/2] Hyper-H reference counter Date: Mon, 20 May 2013 20:21:42 +1000 Message-ID: <1369045302.31632.19.camel@localhost> References: <1368695621.18400.9.camel@localhost> <20130516092128.GP26453@redhat.com> <1368696535.18400.10.camel@localhost> <5194E2D3.3080300@redhat.com> <1368714390.18400.13.camel@localhost> <5194F0F8.9070205@redhat.com> <1368945475.1859.2.camel@localhost> <5199D952.2020808@redhat.com> <20130520083648.GP4725@redhat.com> <5199E20C.1030004@redhat.com> <20130520084912.GQ4725@redhat.com> <5199E536.6070809@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Gleb Natapov , kvm@vger.kernel.org, mtosatti@redhat.com, pl@dlh.net To: Paolo Bonzini Return-path: Received: from mx1.redhat.com ([209.132.183.28]:65224 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752024Ab3ETKVx (ORCPT ); Mon, 20 May 2013 06:21:53 -0400 In-Reply-To: <5199E536.6070809@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, 2013-05-20 at 10:56 +0200, Paolo Bonzini wrote: > Il 20/05/2013 10:49, Gleb Natapov ha scritto: > > On Mon, May 20, 2013 at 10:42:52AM +0200, Paolo Bonzini wrote: > >> Il 20/05/2013 10:36, Gleb Natapov ha scritto: > >>> On Mon, May 20, 2013 at 10:05:38AM +0200, Paolo Bonzini wrote: > >>>> Il 19/05/2013 08:37, Vadim Rozenfeld ha scritto: > >>>>> On Thu, 2013-05-16 at 16:45 +0200, Paolo Bonzini wrote: > >>>>>> Il 16/05/2013 16:26, Vadim Rozenfeld ha scritto: > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> Yes, I have this check added in the second patch. > >>>>>>>>>>>>>>> > >>>>>>>>>>>>> Move it here please. > >>>>>>>>>>> OK, will do it. > >>>>>>>>> > >>>>>>>>> Or better, remove all the handling of HV_X64_MSR_REFERENCE_TSC from this > >>>>>>>>> patch, and leave it all to the second. > >>>>>>>>> > >>>>>>> What for? Could you please elaborate? > >>>>> > >>>>> To make code reviewable. Add one MSR here, the other in the second patch. > >>>>> removing HV_X64_MSR_REFERENCE_TSC will make this particular patch > >>>>> completely non-functional. > >>>> > >>>> Do you mean Windows guest will BSOD or just that they won't use the > >>>> reference TSC? If the latter, it's not a problem. > >>>> > >>> I think it is. If reference counter works without TSC we have a bisect > >>> point for the case when something will going wrong with TSC. > >> > >> Isn't that exactly what might happen with this patch only? Windows will > >> not use the TSC because it finds invalid values in the TSC page. > > > > Yes, it will use reference counter instead. Exactly what we want for a bisect point. > > > >> If it > >> still uses the reference counter, we have the situation you describe. > >> > >> refcount TSC page > >> Y Y <= after patch 2 > >> Y N <= after patch 1 > >> N Y <= impossible > >> N N <= removing TSC page from this patch? > >> > >> Of course if the guest BSODs, it's not possible to split the patches > >> that way. Perhaps in that case it's simply better to do a single patch. > >> > > I am not sure what you are trying to say. Your option list above shows > > that there is a value to split patches like they are split now. > > Hmm, we're talking past each other. :) > > I put the "?" because that's what Vadim implied ("it would make this > particular patch non-functional"), but I don't see why it should be like > this. To me, the obvious way of getting the desired bisect point is > implementing one MSR per patch. So, moving the REFERENCE_TSC handling > entirely to patch 2 would still be in the "refcount=Y, TSC page=N" case. > > In any case, this patch needs more comments and a better commit message. > Microsoft docs are decent, but there are several non-obvious points in > how the patches were done, and they need to be documented. We need specify two partition privileges to activate reference time enlightenment in HYPERV_CPUID_FEATURES (0x40000003) AccessPartitionReferenceCounter and AccessPartitionReferenceTsc otherwise VM will use HPET or PMTimer as a timestamp source. If we specify AccessPartitionReferenceTsc but don't handle write request to HV_X64_MSR_REFERENCE_TSC - the system will fail with 0x78 (PHASE0_EXCEPTION) bugcheck code. If we provide HV_X64_MSR_REFERENCE_TSC handler but don't initialize sequence to 0 - guest will probably newer start or will be extremely slow, because in this case scale should also be initialized. Sequence 0 is a special case, it means use reference counter, but not TSC, as a time source. It is also a fallback solution in a case when a VM, which using TSC has been migrated to a host, which is not equipped with invariant TSC.