Thanks, I had problems getting anything to run under my test DLLs, so when I finally got things running, I ran some tests, and everything seemed to work okay, so I shipped the patch. As usual, after sending, brain kicked in and I realized why it did not appear to be picking up some reg keys due to my over-thinko, and nervousness messing around with the microcode rev reg keys during testing. Resending retested, refixed patch, and microcode reg keys restored and looking normal!
On 2020-07-07 12:36, Lavrentiev, Anton (NIH/NLM/NCBI) [C] wrote: > Hi, > > This is shifting up, IMO: > > + microcode <<= 32; /* shift them down */ > > Thanks, > Anton > >> -----Original Message----- >> From: Brian Inglis <[email protected]> >> Sent: Tuesday, July 07, 2020 1:34 PM >> To: [email protected] >> Subject: [PATCH] format_proc_cpuinfo: add microcode registry lookup values >> >> Re: CPU microcode reported wrong in /proc/cpuinfo >> https://sourceware.org/pipermail/cygwin/2020-May/245063.html >> earlier Windows releases used different registry values to store microcode >> revisions depending on the MSR name being used to get microcode revisions: >> add these alternative registry values to the cpuinfo registry value lookup; >> iterate thru the registry data until a valid microcode revision is found; >> some revision values are in the high bits, so if the low bits are all clear, >> shift the revision value down into the low bits >> --- >> winsup/cygwin/fhandler_proc.cc | 44 +++++++++++++++++++++++++++------- >> 1 file changed, 35 insertions(+), 9 deletions(-) >> >> diff --git a/winsup/cygwin/fhandler_proc.cc b/winsup/cygwin/fhandler_proc.cc >> index f1bc1c7405..f637dfd8e4 100644 >> --- a/winsup/cygwin/fhandler_proc.cc >> +++ b/winsup/cygwin/fhandler_proc.cc >> @@ -692,26 +692,52 @@ format_proc_cpuinfo (void *, char *&destbuf) >> union >> { >> LONG uc_len; /* -max size of buffer before call */ >> - char uc_microcode[16]; >> - } uc; >> + char uc_microcode[16]; /* at least 8 bytes */ >> + } uc[4]; /* microcode values changed historically */ >> >> - RTL_QUERY_REGISTRY_TABLE tab[3] = >> + RTL_QUERY_REGISTRY_TABLE tab[6] = >> { >> { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING, >> - L"~Mhz", &cpu_mhz, REG_NONE, NULL, 0 }, >> + L"~Mhz", &cpu_mhz, REG_NONE, NULL, 0 }, >> { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING, >> - L"Update Revision", &uc, REG_NONE, NULL, 0 }, >> + L"Update Revision", &uc[0], REG_NONE, NULL, 0 }, >> + /* latest MSR */ >> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING, >> + L"Update Signature", &uc[1], REG_NONE, NULL, 0 }, >> + /* previous MSR */ >> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING, >> + L"CurrentPatchLevel", &uc[2], REG_NONE, NULL, 0 }, >> + /* earlier MSR */ >> + { NULL, RTL_QUERY_REGISTRY_DIRECT | RTL_QUERY_REGISTRY_NOSTRING, >> + L"Platform Specific Field1", &uc[3], REG_NONE, NULL, 0 }, >> + /* alternative */ >> { NULL, 0, NULL, NULL, 0, NULL, 0 } >> }; >> >> - memset (&uc, 0, sizeof (uc.uc_microcode)); >> - uc.uc_len = -16; /* -max size of microcode buffer */ >> + for (size_t uci = 0; uci < sizeof (uc)/sizeof (*uc); ++uci) >> + { >> + memset (&uc[uci], 0, sizeof (uc[uci])); >> + uc[uci].uc_len = -(LONG)sizeof (uc[0].uc_microcode); >> + /* neg buffer size */ >> + } >> + >> RtlQueryRegistryValues (RTL_REGISTRY_ABSOLUTE, cpu_key, tab, >> NULL, NULL); >> cpu_mhz = ((cpu_mhz - 1) / 10 + 1) * 10; /* round up to multiple >> of 10 */ >> DWORD bogomips = cpu_mhz * 2; /* bogomips is double cpu MHz since MMX >> */ >> - long long microcode = 0; /* at least 8 bytes for AMD */ >> - memcpy (µcode, &uc, sizeof (microcode)); >> + >> + unsigned long long microcode = 0; /* needs 8 bytes */ >> + for (size_t uci = 0; uci < sizeof (uc)/sizeof (*uc) && !microcode; >> ++uci) >> + { >> + /* still neg buffer size => no data */ >> + if (-(LONG)sizeof (uc[uci].uc_microcode) != uc[uci].uc_len) >> + { >> + memcpy (µcode, uc[uci].uc_microcode, sizeof (microcode)); >> + >> + if (!(microcode & 0xFFFFFFFFLL)) /* some values in high bits */ >> + microcode <<= 32; /* shift them down */ >> + } >> + } >> >> bufptr += __small_sprintf (bufptr, "processor\t: %d\n", cpu_number); >> uint32_t maxf, vendor_id[4], unused; >> -- >> 2.27.0 > -- Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada This email may be disturbing to some readers as it contains too much technical detail. Reader discretion is advised. [Data in IEC units and prefixes, physical quantities in SI.]
