linux-kernel - Re: [PATCH] clockevents/drivers/i8253: Do not zero timer counter in shutdown

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: <e83f8294a27d8b76d0c65ab7c105b2d4b8a53474.camel@infradead.org>
Date: Fri, 02 Aug 2024 10:55:33 +0100
From: David Woodhouse <dwmw2@...radead.org>
To: Thomas Gleixner <tglx@...utronix.de>, lirongqing@...du.com, 
 seanjc@...gle.com, kys@...rosoft.com, haiyangz@...rosoft.com,
 wei.liu@...nel.org,  decui@...rosoft.com, mingo@...hat.com, bp@...en8.de,
 dave.hansen@...ux.intel.com,  x86@...nel.org, linux-hyperv@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] clockevents/drivers/i8253: Do not zero timer counter in
 shutdown

On Thu, 2024-08-01 at 22:31 +0100, David Woodhouse wrote:
> On 1 August 2024 22:22:56 BST, Thomas Gleixner <tglx@...utronix.de> wrote:
> > On Thu, Aug 01 2024 at 21:49, David Woodhouse wrote:
> > > On Thu, 2024-08-01 at 22:00 +0200, Thomas Gleixner wrote:
> > > > > I justify my cowardice on the basis that it doesn't *matter* if a
> > > > > hardware implementation is still toggling the IRQ pin; in that case
> > > > > it's only a few irrelevant transistors which are busy, and it doesn't
> > > > > translate to steal time.
> > > > 
> > > > On real hardware it translates to power...
> > > 
> > > Perhaps, although I'd guess it's a negligible amount. Still, happy to
> > > be brave and make it unconditional. Want a new version of the patch?
> > 
> > Let'ss fix the shutdown sequence first (See Michaels latest mail) and
> > then do the clockevents_i8253_init() change on top.
> 
> Makes sense.

On second thoughts, let's add clockevent_i8253_disable() first and call
it when the PIT isn't being used, then we can bikeshed the precise
behaviour of that function to our hearts' content.

I think it should look like this. Revised version of your test program
attached.

void clockevent_i8253_disable(void)
{
	raw_spin_lock(&i8253_lock);

	/*
	 * Writing the MODE register should stop the counter, according to
	 * the datasheet. This appears to work on real hardware (well, on
	 * modern Intel and AMD boxes; I didn't dig the Pegasos out of the
	 * shed).
	 *
	 * However, some virtual implementations differ, and the MODE change
	 * doesn't have any effect until either the counter is written (KVM
	 * in-kernel PIT) or the next interrupt (QEMU). And in those cases,
	 * it may not stop the *count*, only the interrupts. Although in
	 * the virt case, that probably doesn't matter, as the value of the
	 * counter will only be calculated on demand if the guest reads it;
	 * it's the interrupts which cause steal time.
	 *
	 * Hyper-V apparently has a bug where even in mode 0, the IRQ keeps
	 * firing repeatedly if the counter is running. But it *does* do the
	 * right thing when the MODE register is written.
	 *
	 * So: write the MODE and then load the counter, which ensures that
	 * the IRQ is stopped on those buggy virt implementations. And then
	 * write the MODE again, which is the right way to stop it.
	 */
	outb_p(0x30, PIT_MODE);
	outb_p(0, PIT_CH0);
	outb_p(0, PIT_CH0);

	outb_p(0x30, PIT_MODE);

	raw_spin_unlock(&i8253_lock);
}


View attachment "i8254.c" of type "text/x-csrc" (1914 bytes)

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)