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]

Message-ID: <43F901BD926A4E43B106BF17856F0755A74B2827@orsmsx508.amr.corp.intel.com>
Date:	Tue, 11 May 2010 12:42:51 -0700
From:	"Pan, Jacob jun" <jacob.jun.pan@...el.com>
To:	Thomas Gleixner <tglx@...utronix.de>,
	Jacob Pan <jacob.jun.pan@...ux.intel.com>
CC:	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>,
	"Du, Alek" <alek.du@...el.com>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	"Tang, Feng" <feng.tang@...el.com>,
	LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 3/8] x86/apic: allow use of lapic timer early
 calibration result

Thanks for the review.

> >
> > 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.
> 
I am trying to avoid the risks of completely remove the current lapic 
calibration code since there are so many platforms with different timer
options. And I don't understand things like why pm timer is preferred.
Why not use the rating in clocksource?

> > 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.
> 
perhaps, the calibration data can directly be assigned to lapic timer
clock_event_device.mult? There is no need for the device specific result
scale (e.g. bus clocks per tick)

> >  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/