Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756979AbdGLIDL (ORCPT ); Wed, 12 Jul 2017 04:03:11 -0400 Received: from cn.fujitsu.com ([59.151.112.132]:63103 "EHLO heian.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1756954AbdGLIDH (ORCPT ); Wed, 12 Jul 2017 04:03:07 -0400 X-IronPort-AV: E=Sophos;i="5.22,518,1449504000"; d="scan'208";a="21182566" Subject: Re: [PATCH v8 1/5] x86: add simple udelay calibration To: Boris Ostrovsky , Lu Baolu , Greg Kroah-Hartman , Ingo Molnar , Mathias Nyman , References: <1490083293-3792-1-git-send-email-baolu.lu@linux.intel.com> <1490083293-3792-2-git-send-email-baolu.lu@linux.intel.com> <590C1084.7010302@linux.intel.com> CC: , , , , Juergen Gross , xen-devel From: Dou Liyang Message-ID: Date: Wed, 12 Jul 2017 16:02:59 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.167.226.106] X-yoursite-MailScanner-ID: 596C946B4C60.AD681 X-yoursite-MailScanner: Found to be clean X-yoursite-MailScanner-From: douly.fnst@cn.fujitsu.com Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3194 Lines: 109 Hi, Lu At 05/05/2017 08:50 PM, Boris Ostrovsky wrote: > On 05/05/2017 01:41 AM, Lu Baolu wrote: >> Hi, >> >> On 05/03/2017 06:38 AM, Boris Ostrovsky wrote: >>> On 03/21/2017 04:01 AM, Lu Baolu wrote: >>>> Add a simple udelay calibration in x86 architecture-specific >>>> boot-time initializations. This will get a workable estimate >>>> for loops_per_jiffy. Hence, udelay() could be used after this >>>> initialization. >>> This breaks Xen PV guests since at this point, and until >>> x86_init.paging.pagetable_init() which is when pvclock_vcpu_time_info is >>> mapped, they cannot access pvclock. >>> >>> Is it reasonable to do this before tsc_init() is called? (The failure >>> has nothing to do with tsc_init(), really --- it's just that it is >>> called late enough that Xen PV guests get properly initialized.) If it >>> is, would it be possible to move simple_udelay_calibration() after >>> x86_init.paging.pagetable_init()? >> This is currently only used for bare metal. How about by-pass it >> for Xen PV guests? > > It is fixed this for Xen PV guests now (in the sense that we don't crash > anymore) but my question is still whether this is not too early. Besides > tsc_init() (which might not be important here), at the time when > simple_udelay_calibration() is invoked we haven't yet called: > * kvmclock_init(), which sets calibration routines for KVM > * init_hypervisor_platform(), which sets calibration routines for vmware > and Xen HVM > * x86_init.paging.pagetable_init(), which sets calibration routines for > Xen PV > I guess these may have been missed. Do you have any comments about these? > -boris > > >> >> Best regards, >> Lu Baolu >> >>> -boris >>> >>> >>>> Cc: Ingo Molnar >>>> Cc: x86@kernel.org >>>> Signed-off-by: Lu Baolu >>>> --- >>>> arch/x86/kernel/setup.c | 22 ++++++++++++++++++++++ >>>> 1 file changed, 22 insertions(+) >>>> >>>> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >>>> index 4bf0c89..e70204e 100644 >>>> --- a/arch/x86/kernel/setup.c >>>> +++ b/arch/x86/kernel/setup.c >>>> @@ -837,6 +837,26 @@ dump_kernel_offset(struct notifier_block *self, unsigned long v, void *p) >>>> return 0; >>>> } >>>> >>>> +static void __init simple_udelay_calibration(void) >>>> +{ >>>> + unsigned int tsc_khz, cpu_khz; >>>> + unsigned long lpj; >>>> + >>>> + if (!boot_cpu_has(X86_FEATURE_TSC)) >>>> + return; if it returns here, can we use udelay() correctly like before? Thanks, dou. >>>> + >>>> + cpu_khz = x86_platform.calibrate_cpu(); >>>> + tsc_khz = x86_platform.calibrate_tsc(); >>>> + >>>> + tsc_khz = tsc_khz ? : cpu_khz; >>>> + if (!tsc_khz) >>>> + return; >>>> + >>>> + lpj = tsc_khz * 1000; >>>> + do_div(lpj, HZ); >>>> + loops_per_jiffy = lpj; >>>> +} >>>> + >>>> /* >>>> * Determine if we were loaded by an EFI loader. If so, then we have also been >>>> * passed the efi memmap, systab, etc., so we should use these data structures >>>> @@ -985,6 +1005,8 @@ void __init setup_arch(char **cmdline_p) >>>> */ >>>> x86_configure_nx(); >>>> >>>> + simple_udelay_calibration(); >>>> + >>>> parse_early_param(); >>>> >>>> #ifdef CONFIG_MEMORY_HOTPLUG > > > >