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

[Lu Baolu <baolu.lu@linux.intel.com>]

Hi,

On 07/12/2017 04:02 PM, Dou Liyang wrote:
> 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?
>

The patch will be available in 4.13-rc1.

Best regards,
Lu Baolu

>> -boris
>>
>>
>>>
>>> Best regards,
>>> Lu Baolu
>>>
>>>> -boris
>>>>
>>>>
>>>>> Cc: Ingo Molnar <mingo@redhat.com>
>>>>> Cc: x86@kernel.org
>>>>> Signed-off-by: Lu Baolu <baolu.lu@linux.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
>>
>>
>>
>>
>
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>