Re: [PATCH] clockevents/drivers/i8253: Do not zero timer counter in shutdown
From: David Woodhouse
Date: Fri Aug 02 2024 - 05:56:17 EST
On Thu, 2024-08-01 at 22:31 +0100, David Woodhouse wrote:
> On 1 August 2024 22:22:56 BST, Thomas Gleixner <tglx@xxxxxxxxxxxxx> 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);
}
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <stdint.h>
#include <sys/io.h>
typedef unsigned char uint8_t;
typedef unsigned short uint16_t;
#define BUILDIO(bwl, bw, type) \
static __always_inline void __out##bwl(type value, uint16_t port) \
{ \
asm volatile("out" #bwl " %" #bw "0, %w1" \
: : "a"(value), "Nd"(port)); \
} \
\
static __always_inline type __in##bwl(uint16_t port) \
{ \
type value; \
asm volatile("in" #bwl " %w1, %" #bw "0" \
: "=a"(value) : "Nd"(port)); \
return value; \
}
BUILDIO(b, b, uint8_t)
#define inb __inb
#define outb __outb
#define PIT_MODE 0x43
#define PIT_CH0 0x40
#define PIT_CH2 0x42
static int is8254;
static void dump_pit(void)
{
if (is8254) {
// Latch and output counter and status
outb(0xC2, PIT_MODE);
printf("%02x %02x %02x\n", inb(PIT_CH0), inb(PIT_CH0), inb(PIT_CH0));
} else {
// Latch and output counter
outb(0x0, PIT_MODE);
printf("%02x %02x\n", inb(PIT_CH0), inb(PIT_CH0));
}
}
int main(int argc, char* argv[])
{
int nr_counts = 2;
if (argc > 1)
nr_counts = atoi(argv[1]);
if (argc > 2)
is8254 = 1;
if (ioperm(0x40, 4, 1) != 0)
return 1;
dump_pit();
printf("Set oneshot\n");
outb(0x38, PIT_MODE);
outb(0x00, PIT_CH0);
outb(0x0F, PIT_CH0);
dump_pit();
usleep(1000);
dump_pit();
printf("Set periodic\n");
outb(0x34, PIT_MODE);
outb(0x00, PIT_CH0);
outb(0x0F, PIT_CH0);
dump_pit();
usleep(1000);
dump_pit();
dump_pit();
usleep(100000);
dump_pit();
usleep(100000);
dump_pit();
printf("Set stop (%d counter writes)\n", nr_counts);
outb(0x30, PIT_MODE);
while (nr_counts--)
outb(0xFF, PIT_CH0);
dump_pit();
usleep(100000);
dump_pit();
usleep(100000);
dump_pit();
printf("Set MODE 0\n");
outb(0x30, PIT_MODE);
dump_pit();
usleep(100000);
dump_pit();
usleep(100000);
dump_pit();
return 0;
}
Attachment:
smime.p7s
Description: S/MIME cryptographic signature