From: Thomas Gleixner <redacted> Sent: Thursday, August 1, 2024 7:21 AM

On Tue, Feb 07 2023 at 09:14, lirongqing@baidu.com wrote:

quoted

@@ -117,11 +110,6 @@ static int pit_shutdown(struct clock_event_device *evt)

 	outb_p(0x30, PIT_MODE);

-	if (i8253_clear_counter_on_shutdown) {
-		outb_p(0, PIT_CH0);
-		outb_p(0, PIT_CH0);
-	}
-

The stop sequence is wrong:

    When there is a count in progress, writing a new LSB before the
    counter has counted down to 0 and rolled over to FFFFh, WILL stop
    the counter.  However, if the LSB is loaded AFTER the counter has
    rolled over to FFFFh, so that an MSB now exists in the counter, then
    the counter WILL NOT stop.

The original i8253 datasheet says:

    1) Write 1st byte stops the current counting
    2) Write 2nd byte starts the new count

The above does not make sure it actually has not rolled over and it
obviously is initiating the new count by writing the MSB too. So it does
not work on real hardware either.

The proper sequence is:

         // Switch to mode 0
         outb_p(0x30, PIT_MODE);
         // Load the maximum value to ensure there is no rollover
         outb_p(0xff, PIT_CH0);
         // Writing MSB starts the counter from 0xFFFF and clears rollover
         outb_p(0xff, PIT_CH0);
         // Stop the counter by writing only LSB
         outb_p(0xff, PIT_CH0);

That works on real hardware and fails on KVM... So much for the claim
that KVM follows the spec :)

So yes, the code is buggy, but instead of deleting it, we rather fix it,
no?

FWIW, in Hyper-V guests with the Hyper-V quirk removed, tglx's new
sequence does *not* stop the PIT. But this sequence does:

outb_p(0x30, PIT_MODE);
outb_p(0xff, PIT_CH0);
outb_p(0xff, PIT_CH0);

outb_p(0x30, PIT_MODE);
outb_p(0xff, PIT_CH0);

I don't have a convenient way to test my sequence on KVM.

Michael