PING: RFR: 8250598: Hyper-V is detected in spite of running on host OS

[David Holmes]

Hi Yasumasa,

On 12/08/2020 10:56 pm, Yasumasa Suenaga wrote:
> Hi Matthias, David,
> 
> On 2020/08/12 21:12, David Holmes wrote:
>> On 12/08/2020 8:51 pm, Baesken, Matthias wrote:
>>> Hi Yasumasa ,  I'm more or less fine with the change .
>>> But still not fully convinced  that removing  the iteration is a good 
>>> thing .
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8250598/webrev.01/src/hotspot/cpu/x86/vm_version_x86.cpp.frames.html 
>>>
>>>
>>> 1827   for (base = 0x40000000; base < 0x40010000; base += 0x100) {
>>> 1828     check_virt_cpuid(base, registers);
>>>
>>> I think just checking  "0x40000000" should work in most cases but if 
>>> I remember correctly sometimes it was not enough .
>>> See also  some references about Xen/KVM :
>>>
>>> https://lists.linuxfoundation.org/pipermail/virtualization/2012-May/019974.html 
>>>
>>>
>>> "If compat mode for another h/v is enabled then those leaves will appear
>>> at 0x40000000 and Xen's will be bumped up, so a fully Xen aware set of
>>> drivers (or detection routine, etc) should check at 0x100 intervals
>>> until 0x40010000 "
>>>
>>> and
>>>
>>> https://lore.kernel.org/patchwork/patch/394371/
> 
> I understand that if the process runs on Xen on other hypervisor (e.g. 
> KVM), information for Xen would be set between 0x40000100 and 0x40010000.
> Ok, I will not remove the loop in new webrev, and will add comment about 
> it.
> 
> 
>>> And not so happy about the WMI  usage (called in early JVM startup) :
>>>
>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8250598/webrev.01/src/hotspot/os_cpu/windows_x86/vm_version_windows_x86.cpp.frames.html 
>>>
>>>
>>> bool VM_Version::is_in_VM() { ... }
>>>   ...
>>> }
>>>
>>>   But if noone else complains about it, I guess it's okay .
>>
>> I haven't reviewed this because I don't understand any of the code. 
>> But all that extra stuff during VM initialization doesn't look good to 
>> me. We do not want to pay a startup penalty just because we want a 
>> more accurate hypervisor listing in an error report!
>>
>> Can we not determine this only if needed? (Granted doing all this 
>> stuff during crash reporting may also be an issue.)
> 
> This value would be used in hs_err and VM.info dcmd, so we can determine 
> it when it is needed.
> However the change would be bigger, and it is not the original scope. 
> Can we fix them at the same time?

Are you able to run startup benchmarks to see if there is any regression?

David
-----

> 
> Thanks,
> 
> Yasumasa
> 
> 
>> David
>> -----
>>
>>>
>>> Best regards, Matthias
>>>
>>>
>>>> PING: Could you review this change?
>>>>
>>>>    JBS: https://bugs.openjdk.java.net/browse/JDK-8250598
>>>>    webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8250598/webrev.01/
>>>>
>>>> Build change has been reviewed by Erik.
>>>>
>>>