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. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html