linux-kernel - Re: [PATCH 3/8] x86/apic: allow use of lapic timer early calibration result

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]

Date:	Tue, 11 May 2010 15:46:15 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Jacob Pan <jacob.jun.pan@...ux.intel.com>
cc:	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>,
	Alek Du <alek.du@...el.com>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Feng Tang <feng.tang@...el.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Jacob Pan <jacob.jun.pan@...el.com>
Subject: Re: [PATCH 3/8] x86/apic: allow use of lapic timer early calibration
 result

On Fri, 7 May 2010, Jacob Pan wrote:

> From: Jacob Pan <jacob.jun.pan@...el.com>
> 
> lapic timer calibration can be combined with tsc in platform specific
> calibration functions. if such calibration result is obtained early,
> we can skip the redundent calibration loops.

I'd rather move lapic calibration into TSC calibration in general as
we do the same thing twice for no good reason.

That needs some code restructuring, but that's worth it.

> Signed-off-by: Jacob Pan <jacob.jun.pan@...el.com>
> Signed-off-by: Jacob Pan <jacob.jun.pan@...ux.intel.com>

Hehe. So you handed the patch to yourself :)

> ---
>  arch/x86/kernel/apic/apic.c |   21 ++++++++++++++++++++-
>  1 files changed, 20 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index e5a4a1e..8ef56ac 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -175,7 +175,7 @@ static struct resource lapic_resource = {
>  	.flags = IORESOURCE_MEM | IORESOURCE_BUSY,
>  };
>  
> -static unsigned int calibration_result;
> +unsigned int calibration_result;

Aside of my general objection it'd be not a good idea to make this
global w/o renaming it to something sensible like
lapic_timer_frequency.
  
>  static int lapic_next_event(unsigned long delta,
>  			    struct clock_event_device *evt);
> @@ -597,6 +597,25 @@ static int __init calibrate_APIC_clock(void)
>  	long delta, deltatsc;
>  	int pm_referenced = 0;
>  
> +	/**
> +	 * check if lapic timer has already been calibrated by platform
> +	 * specific routine, such as tsc calibration code. if so, we just fill
> +	 * in the clockevent structure and return.
> +	 */
> +	if (calibration_result) {
> +		apic_printk(APIC_VERBOSE, "lapic timer already calibrated %d\n",
> +				calibration_result);
> +		lapic_clockevent.mult = div_sc(calibration_result/APIC_DIVISOR,
> +					TICK_NSEC, lapic_clockevent.shift);
> +		lapic_clockevent.max_delta_ns =
> +			clockevent_delta2ns(0x7FFFFF, &lapic_clockevent);
> +		lapic_clockevent.min_delta_ns =
> +			clockevent_delta2ns(0xF, &lapic_clockevent);
> +		lapic_clockevent.features &= ~CLOCK_EVT_FEAT_DUMMY;
> +		return 0;

  Grr. I hate duplicated code.

> +	}
> +
>  	local_irq_disable();
>  
>  	/* Replace the global interrupt handler */
> -- 
> 1.6.3.3
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/