[16] RFR(S) 8249672: Include microcode revision in features_string on x86

[Vladimir Kozlov]

I updated subject to our formal review request format (JDK version, RFE's id and subject).

I moved RFE to runtime group as Thomas said:

https://bugs.openjdk.java.net/browse/JDK-8249672

Submitted tier1 testing to build on all our supported platforms. And debug builds on linux failed:

#  SIGSEGV (0xb) at pc=0x0000146fc6af4b0b, pid=9715, tid=9718
# V  [libjvm.so+0xc12b0b]  GuardedMemory::print_on(outputStream*) const+0xeb

V  [libjvm.so+0xc12b0b]  GuardedMemory::print_on(outputStream*) const+0xeb
V  [libjvm.so+0x13c898a]  verify_memory(void*)+0x26a
V  [libjvm.so+0x13cd30b]  os::free(void*)+0x5b
V  [libjvm.so+0x13e5598]  os::cpu_microcode_revision()+0xc8
V  [libjvm.so+0x17d314c]  VM_Version::get_processor_features()+0x76c
V  [libjvm.so+0x17d6ead]  VM_Version::initialize()+0x10d
V  [libjvm.so+0x17ce6c6]  VM_Version_init()+0x26
V  [libjvm.so+0xcb2895]   init_globals()+0x55
V  [libjvm.so+0x16dde63]  Threads::create_vm(JavaVMInitArgs*, bool*)+0x2d3


Regards,
Vladimir K

On 7/17/20 3:02 PM, Thomas Stüfe wrote:
> Hi Vladimir,
> 
> On Fri, Jul 17, 2020 at 11:57 PM Ivanov, Vladimir A <
> vladimir.a.ivanov at intel.com> wrote:
> 
>>>   +#if defined(IA32) || defined(AMD64)
>>>
>>> Is that not synonymous with x86?
>>
>> This patter was copied from the method “print_model_name_and_flags” (file
>> os/linux/os_linux.cpp).
>>
>> This method also read the “/proc/cpuinfo” file and I reuse it as
>> ‘template’ for the new method.
>>
>> It is better to use one pattern to work with exactly same file but in
>> general you are right.
>>
>> The X86 is defined in the file ./share/utilities/macros.hpp as:
>>
>> #if defined(IA32) || defined(AMD64)
>>
>> #define X86
>>
>> #define X86_ONLY(code) code
>>
>> #define NOT_X86(code)
>>
>>
>>
>> The question here: could I delete this “ifdefs” while this method should
>> work on x86 only?
>>
>>
>>
> 
> os_linux_x86.cpp is compiled for x86 platforms only, whereas os_linux.cpp
> is shared among all architectures.
> 
> So, in the former you do not need to exclude non-x86 architectures.
> 
> Cheers, Thomas
> 
> 
>> Thanks, Vladimir
>>
>>
>>
>> *From:* Thomas Stüfe <thomas.stuefe at gmail.com>
>> *Sent:* Friday, July 17, 2020 2:26 PM
>> *To:* Ivanov, Vladimir A <vladimir.a.ivanov at intel.com>; Hotspot dev
>> runtime <hotspot-runtime-dev at openjdk.java.net>
>> *Cc:* hotspot-compiler-dev at openjdk.java.net
>> *Subject:* Re: add microcode version to the hs_err files
>>
>>
>>
>>
>>
>>
>>
>> On Fri, Jul 17, 2020 at 11:19 PM Thomas Stüfe <thomas.stuefe at gmail.com>
>> wrote:
>>
>> Hi Vladimir,
>>
>>
>>
>> I think this would be more suited to hotspot-runtime.
>>
>>
>>
>>
>> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8249672/webrev.00/src/hotspot/os_cpu/linux_x86/os_linux_x86.cpp.udiff.html
>>
>>
>> +#if defined(IA32) || defined(AMD64)
>>
>> Is that not synonymous with x86?
>>
>>
>>
>> +    while ((read = getline(&line, &len, fp)) != -1) {
>> +      if (len > 10 && strstr(line, "microcode") != NULL) {
>> +        char* rev = strchr(line, ':');
>> +        if (rev != NULL) sscanf(rev + 1, "%x", &result);
>> +        break;
>> +      }
>> +    }
>> +    free(line);
>>
>>
>>
>> Not sure this works as intended. At the first call to getline() it will
>> allocate a line buffer for you and return it. That buffer will be as large
>> as the first line you happen to read. You then pass that same buffer into
>> getline to fetch the next lines, but what if those are longer than the
>> first?
>>
>>
>>
>>
>>
>> Forget that point, getline calls realloc() on the line buffer to resize
>> it, so this should be okay.
>>
>>
>>
>> Thanks, Thomas
>>
>>
>>
>> But anyway it would be better to pass a simple caller provided buffer in -
>> stack allocated. Since this function is called at crash time and the C heap
>> could be corrupted.
>>
>>
>>
>> Cheers, Thomas
>>
>>
>>
>>
>>
>> On Fri, Jul 17, 2020 at 10:22 PM Ivanov, Vladimir A <
>> vladimir.a.ivanov at intel.com> wrote:
>>
>> Hello,
>>
>> could you please review the patch
>> http://cr.openjdk.java.net/~sviswanathan/Vladimir/8249672/webrev.00/
>>
>> This patch add the microcode version for different OSes that may be useful
>> in the issue resolution process.
>>
>>
>>
>> The reported microcode version for different OSes loos as:
>>
>>
>>
>> Linux (RHEL7.7):
>>
>> # cat hs_err_pid251046.log |grep microc
>>
>> CPU: total 112 (initial active 112) (28 cores per cpu, 2 threads per core)
>> family 6 model 85 stepping 4 microcode 0x200005e, cmov, cx8, fxsr, mmx,
>> sse, sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, vzeroupper, avx, avx2, aes,
>> clmul, erms, rtm, 3dnowpref, lzcnt, ht, tsc, tscinvbit, bmi1, bmi2, adx,
>> fma, clflush, clflushopt, clwb
>>
>>
>>
>> Windows (Win10, v1809):
>>
>> CPU: total 4 (initial active 4) (2 cores per cpu, 2 threads per core)
>> family 6 model 142 stepping 9 microcode 0xb4, cmov, cx8, fxsr, mmx, sse,
>> sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, vzeroupper, avx, avx2, aes,
>> clmul, erms, rtm, 3dnowpref, lzcnt, ht, tsc, tscinvbit, bmi1, bmi2, adx,
>> fma, clflush, clflushopt
>>
>>
>>
>> MacOS (Darwin):
>>
>> $ cat hs_err_pid95187.log |grep microc
>>
>> CPU: total 8 (initial active 8) (4 cores per cpu, 2 threads per core)
>> family 6 model 126 stepping 5 microcode 0x78, cmov, cx8, fxsr, mmx, sse,
>> sse2, sse3, ssse3, sse4.1, sse4.2, popcnt, vzeroupper, avx, avx2, aes,
>> clmul, erms, 3dnowpref, lzcnt, ht, tsc, tscinvbit, bmi1, bmi2, adx, sha,
>> fma, clflush, clflushopt
>>
>>
>>
>> Thanks, Vladimir
>>
>>
>>    Thanks, Vladimir
>>
>>