linux-kernel - Re: [PATCH v8 1/5] x86: add simple udelay calibration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c1edc9cb-3b28-c6e4-4fb0-7f20a477bcb7@cn.fujitsu.com>
Date:   Wed, 12 Jul 2017 16:02:59 +0800
From:   Dou Liyang <douly.fnst@...fujitsu.com>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ingo Molnar <mingo@...hat.com>,
        Mathias Nyman <mathias.nyman@...ux.intel.com>,
        <tglx@...utronix.de>
CC:     <peterz@...radead.org>, <linux-usb@...r.kernel.org>,
        <x86@...nel.org>, <linux-kernel@...r.kernel.org>,
        Juergen Gross <jgross@...e.com>,
        xen-devel <xen-devel@...ts.xen.org>
Subject: Re: [PATCH v8 1/5] x86: add simple udelay calibration

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 <mingo@...hat.com>
>>>> Cc: x86@...nel.org
>>>> Signed-off-by: Lu Baolu <baolu.lu@...ux.intel.com>
>>>> ---
>>>>  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
>
>
>
>