Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751577AbdLLIE6 (ORCPT ); Tue, 12 Dec 2017 03:04:58 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60352 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750715AbdLLIEy (ORCPT ); Tue, 12 Dec 2017 03:04:54 -0500 From: Vitaly Kuznetsov To: Roman Kagan Cc: kvm@vger.kernel.org, x86@kernel.org, Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , Thomas Gleixner , Ingo Molnar , "H. Peter Anvin" , "K. Y. Srinivasan" , Haiyang Zhang , Stephen Hemminger , "Michael Kelley \(EOSG\)" , Andy Lutomirski , Mohammed Gamal , Cathy Avery , linux-kernel@vger.kernel.org, devel@linuxdriverproject.org Subject: Re: [PATCH 3/6] x86/hyper-v: reenlightenment notifications support References: <20171208105000.25116-1-vkuznets@redhat.com> <20171208105000.25116-4-vkuznets@redhat.com> <20171208181059.GB4777@rkaganb.sw.ru> <87bmj5k2pa.fsf@vitty.brq.redhat.com> <20171211171023.GA2304@rkaganb.sw.ru> Date: Tue, 12 Dec 2017 09:04:49 +0100 In-Reply-To: <20171211171023.GA2304@rkaganb.sw.ru> (Roman Kagan's message of "Mon, 11 Dec 2017 20:10:24 +0300") Message-ID: <874low5q3i.fsf@vitty.brq.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 12 Dec 2017 08:04:54 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2023 Lines: 60 Roman Kagan writes: > On Mon, Dec 11, 2017 at 10:56:33AM +0100, Vitaly Kuznetsov wrote: >> Roman Kagan writes: >> > On Fri, Dec 08, 2017 at 11:49:57AM +0100, Vitaly Kuznetsov wrote: >> >> +void register_hv_tsc_update(void (*cb)(void)) >> >> +{ >> > >> > The function name seems unfortunate. IMHO such a name suggests >> > registering a callback on a notifier chain (rather than unconditionally >> > replacing the old callback), and having no other side effects. >> >> I see, any suggestion? register_hv_reenlightenment_cb? register_hv_tscchange_cb? > > IMHO arm_hv_reenlightenment_cb or arm_hv_tscchange_cb would be better, > but I'm not very good at giving descriptive names. > I would probably try to avoid using 'arm' word in x86 code to assist poor git-greppers :-) And we actually need a pair of functions (enable/disable). I will probably go with set_hv_tscchange_cb() clear_hv_tscchange_cb() in v2 unless there's a better suggestion. >> >> > >> >> + struct hv_reenlightenment_control re_ctrl = { >> >> + .vector = HYPERV_REENLIGHTENMENT_VECTOR, >> >> + .enabled = 1, >> >> + .target_vp = hv_vp_index[smp_processor_id()] >> >> + }; >> >> + struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1}; >> >> + >> >> + if (!(ms_hyperv.features & HV_X64_ACCESS_REENLIGHTENMENT)) >> >> + return; >> > >> > What happens then? L2 guests keep running with their clocks ticking at >> > a different speed? >> > >> >> In reallity this never happens -- in case nested virtualization is >> supported reenlightenment is also available. In theory, L0 can emulate >> TSC acceess for forever after migration. > > I would think that Hyper-V only started rdtsc emulation if > TSC_EMULATION_CONTROL was turned on, which wouldn't happen here. > Yes, this is the de-facto behavior I observe with WS2016. > But indeed, normally this shouldn't be a problem. It may make sense > just to issue a warning if the feature is unsupported, though. Will do in v2, thanks. -- Vitaly