public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] format_proc_cpuinfo: add microcode registry lookup values
@ 2020-07-07 17:33 Brian Inglis
  2020-07-07 18:36 ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  0 siblings, 1 reply; 3+ messages in thread
From: Brian Inglis @ 2020-07-07 17:33 UTC (permalink / raw)
  To: cygwin-patches

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 (&microcode, &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 (&microcode, 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


^ permalink raw reply	[flat|nested] 3+ messages in thread
* RE: [PATCH] format_proc_cpuinfo: add microcode registry lookup values
  2020-07-07 17:33 [PATCH] format_proc_cpuinfo: add microcode registry lookup values Brian Inglis
@ 2020-07-07 18:36 ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
  2020-07-07 18:55   ` Brian Inglis
  0 siblings, 1 reply; 3+ messages in thread
From: Lavrentiev, Anton (NIH/NLM/NCBI) [C] @ 2020-07-07 18:36 UTC (permalink / raw)
  To: Brian Inglis; +Cc: cygwin-patches

Hi,

This is shifting up, IMO:

+		  microcode <<= 32;		/* shift them down */

Thanks,
Anton

> -----Original Message-----
> From: Brian Inglis <Brian.Inglis@SystematicSW.ab.ca>
> Sent: Tuesday, July 07, 2020 1:34 PM
> To: cygwin-patches@cygwin.com
> 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 (&microcode, &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 (&microcode, 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


^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH] format_proc_cpuinfo: add microcode registry lookup values
  2020-07-07 18:36 ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
@ 2020-07-07 18:55   ` Brian Inglis
  0 siblings, 0 replies; 3+ messages in thread
From: Brian Inglis @ 2020-07-07 18:55 UTC (permalink / raw)
  To: cygwin-patches

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 <Brian.Inglis@SystematicSW.ab.ca>
>> Sent: Tuesday, July 07, 2020 1:34 PM
>> To: cygwin-patches@cygwin.com
>> 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 (&microcode, &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 (&microcode, 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.]

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-07-07 18:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-07 17:33 [PATCH] format_proc_cpuinfo: add microcode registry lookup values Brian Inglis
2020-07-07 18:36 ` Lavrentiev, Anton (NIH/NLM/NCBI) [C]
2020-07-07 18:55   ` Brian Inglis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).