grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 1/1] i386: Make pmtimer tsc calibration not take 51 second


From: Robbie Harwood
Subject: Re: [PATCH v3 1/1] i386: Make pmtimer tsc calibration not take 51 seconds to fail
Date: Tue, 19 Jul 2022 13:38:51 -0400

Daniel Kiper <dkiper@net-space.pl> writes:

> On Thu, Jul 14, 2022 at 05:42:51PM -0400, Robbie Harwood wrote:
>
>> This outcome was tested using grub compiled with
>> GRUB_PMTIMER_IGNORE_BAD_READS defined (so as not to trip the bad read
>> test) using qemu+kvm with UEFI (OVMF) firmware, and these options:
>> -machine pc-q35-2.10 -cpu Broadwell-noTSX

>> +/*
>> + * Define GRUB_PMTIMER_IGNORE_BAD_READS if you're trying to test a timer 
>> that's
>> + * present but doesn't keep time well.
>> + */
>> +// #define GRUB_PMTIMER_IGNORE_BAD_READS

>> +#ifndef GRUB_PMTIMER_IGNORE_BAD_READS
>
> I still do not understand why we need compile this code conditionally.
> Why cannot we build it always and enable only when debug is set to
> pmtimer/all? Or use another variable to enable this test?

It's mentioned in the commit message and the comment I've quoted above
that this is for testing - there isn't a use case for defining it
otherwise.  (I also touched on this some in reply to Paul Menzel.)

As it changes more behavior than just debug prints, it doesn't make
sense to me to tie it to debug.  More generally, I'd rather remove it
than making the behavior runtime-configurable.  Would you prefer
removing the #define entirely to the patch as written now?

(Rest of comments make sense to me and I'll cut a v4 once we resolve
this.)

Be well,
--Robbie

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]