This is the mail archive of the
cygwin-patches
mailing list for the Cygwin project.
Re: [PATCH] fhandler_proc.cc(format_proc_cpuinfo): fix issues, add fields, flags
- From: Ken Brown <kbrown at cornell dot edu>
- To: "cygwin-patches at cygwin dot com" <cygwin-patches at cygwin dot com>
- Date: Sat, 5 Oct 2019 21:06:21 +0000
- Subject: Re: [PATCH] fhandler_proc.cc(format_proc_cpuinfo): fix issues, add fields, flags
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=cornell.edu; dmarc=pass action=none header.from=cornell.edu; dkim=pass header.d=cornell.edu; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=1fwN9nkVX1VHHjxfxc6D5NKfylzb7LdW3W08RR3iQZ0=; b=RN8qKKvixg3wenwWjM0R4F+osfV2kJxf1ypu75XwALcturKwwhkfsZXx9ePTpEs7T/7qguff1e8bPjab2wi68Su9KepzWYPKZQxBy2Sq8xnRb71Odz9UTg1I2ZKv+rdod0Qv/wNdnFkeq8ZzsrgohWMUURtNmnI8ohf7jbzHSqEME1VYchN07gsbzHiIQbrbngRMSgB64qmw5/ymLOCeYG1eBCNTLpyc+C8BirNAl9tyAfZT1UHrnalCfcwGaGLO/Q4fMMdFCDe+oZ2rrH7/ncy4UsFYKlOSR8DhvB9MK4W8tvnvC6+ywtyHydjcEJth5/WhxpiUdtomKAFzTAxd+A==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MmOLPmNYOcdojAyfa2rUmpGSN/VMO8cK/obUL61yhHRyzPmleiVwHHp5/2/KOYomvKaqtMNUAsxHs0rN3qF/rlkfnHVqgysJT5xDyH7IdvvIAKchQUTS1icfdbWpZ5K4GuLNZso2rXa1Nmh1ZPZSZ6JwFEWC5Vy3MywpH3n1QftuwGo52lDGBNSX2CoXQH7lg5fZGF5hMYiI/GWiXT7R5KKRAI/Vm4/XRk7BkOsBjKFp7mIe2s4G8fZ8ROo7Zx+Pz1AG8ofdH5lk/fHyoM5TU/y5BbI+QSQoFe9aWNjwqpxolEghh+UE9AX1Wpw8geBuFy+G9T8WtOkoZh3qWhaC5A==
- References: <20191004104457.33757-1-Brian.Inglis@SystematicSW.ab.ca>
Hi Brian,
On 10/4/2019 6:44 AM, Brian Inglis wrote:
> fix cache size return code handling and make AMD/Intel code common;
> fix cpuid level count as number of non-zero leafs excluding sub-leafs;
> fix AMD physical cores count to be documented nc + 1;
> round cpu MHz to correct Windows and match Linux cpuinfo;
> add microcode from Windows registry Update Revision REG_BINARY;
> add bogomips which has been cpu MHz*2 since Pentium MMX;
> handle as common former Intel only feature flags also supported on AMD;
> add 88 feature flags inc. AVX512 extensions, AES, SHA with 20 cpuid calls;
> commented out flags are mostly used but not currently reported in cpuinfo
> but some may not currently be used by Linux
Thanks! This must have been a lot of work.
It would be easier to review if you would split it up into smaller patches, each
doing one thing, to the extent that this makes sense. For example, the
simplification achieved by using the ftcprint macro could be done in a single
patch that's separate from the substantive changes.
A few nits:
> - DWORD cpu_mhz = 0;
> - RTL_QUERY_REGISTRY_TABLE tab[2] = {
> + DWORD cpu_mhz;
> + DWORD bogomips;
> + long long microcode = 0xffffffff; /* at least 8 bytes for AMD */
> + union {
> + LONG len;
> + char uc_microcode[16];
> + } uc;
> +
> + cpu_mhz = 0;
> + bogomips = 0;
> + microcode = 0;
Why change the existing intialization style? How about
DWORD cpu_mhz = 0;
DWORD bogomips = 0;
long long microcode = 0; /* at least 8 bytes for AMD */
> + memset(&uc, 0, sizeof(uc.uc_microcode));
^ ^
(Space before parenthesis, here and in several other places.)
> + cpu_mhz = ((cpu_mhz - 1)/10 + 1)*10; /* round up a digit */
Please surround '/' and '*' by spaces (and similarly in what follows). Also,
the comment is confusing. Maybe "round up to a multiple of 10"?
> + for (uint32_t l = maxf; 1 < l; --l) {
^
(Brace on next line, and also in one other place.)
Ken