On Fri, Apr 3, 2026 at 2:06 AM John Naylor <[email protected]> wrote: > > On Fri, Apr 3, 2026 at 6:16 AM Lukas Fittl <[email protected]> wrote: > > v16 > > Just some minor quibbles on 0002:
Thanks for the review! > > --- a/src/include/port/pg_cpu.h > +++ b/src/include/port/pg_cpu.h > @@ -23,6 +23,12 @@ typedef enum X86FeatureId > /* scalar registers and 128-bit XMM registers */ > PG_SSE4_2, > PG_POPCNT, > + PG_HYPERVISOR, > > The hypervisor flag doesn't really belong with an instruction family. > Maybe a separate category like "identification"? Yeah, I've pondered different names here, but "identification" seems good for now - I agree it doesn't belong with the earlier flags. > > + /* TSC flags */ > + PG_RDTSCP, > + PG_TSC_INVARIANT, > + PG_TSC_ADJUST, > > Maybe spell out time stamp counter in the comment, since this will be > the first time the reader encounters that in this file. Makes sense, done. FWIW, TSC can be spelled out either as "Time Stamp Counter" or "Time-Stamp Counter". I've gone with the latter for now since that's what was done elsewhere, and is how the Intel manuals reference it as well. > > + * For some other Hypervisors that have an invariant TSC, e.g. HyperV, we > would > + * need to access an MSR to get the frequency (which is typically not > available > > Maybe spell out MSR too, because I for one don't know what that is. Done. I've also removed the note re: TSC calibration in that same comment, since that's in a later commit and I don't think its necessary to talk about it in that comment anyway. > > + X86Features[PG_HYPERVISOR] = reg[ECX] >> 31 & 1; > + have_osxsave = reg[ECX] & (1 << 27); > + > + pg_cpuid_subleaf(0x07, 0, reg); > + > + X86Features[PG_TSC_ADJUST] = (reg[EBX] & (1 << 1)) != 0; > > Some inconsistency in shift style. Fixed. See attached v17. Thanks, Lukas -- Lukas Fittl
v17-0005-instrumentation-ARM-support-for-fast-time-measur.patch
Description: Binary data
v17-0002-Allow-retrieving-x86-TSC-frequency-flags-from-CP.patch
Description: Binary data
v17-0001-instrumentation-Streamline-ticks-to-nanosecond-c.patch
Description: Binary data
v17-0004-pg_test_timing-Also-test-RDTSC-RDTSCP-timing-and.patch
Description: Binary data
v17-0003-instrumentation-Use-Time-Stamp-Counter-TSC-on-x8.patch
Description: Binary data
