Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1634659 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v8 1/5] x86: add simple udelay calibration Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-05-03 00:50 +0200
    Re: [PATCH v8 1/5] x86: add simple udelay calibration Lu Baolu <baolu.lu@linux.intel.com> - 2017-05-05 07:50 +0200
      Re: [PATCH v8 1/5] x86: add simple udelay calibration Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-05-05 15:00 +0200

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

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()?

-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;
> +
> +	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

[toc] | [next] | [standalone]

#1636172

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?

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;
>> +
>> +	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
>

[toc] | [prev] | [next] | [standalone]

#1636363

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

-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;
>>> +
>>> +	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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web