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?
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.
