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

Tue Aug 18 14:39:20 UTC 2020

Hi Yasumasa,

thanks a lot for measuring and improving the overhead. This version looks much better.

I still wonder what the reason was for using a stub. There was probably a reason, but I don't remember.

Hopefully, Matthias can find time for another review when he's back next week.

Thanks and best regards,
Martin

> -----Original Message-----
> From: Yasumasa Suenaga <suenaga at oss.nttdata.com>
> Sent: Dienstag, 18. August 2020 05:03
> To: David Holmes <david.holmes at oracle.com>; Baesken, Matthias
> <matthias.baesken at sap.com>; hotspot-runtime-dev at openjdk.java.net;
> build-dev at openjdk.java.net; Doerr, Martin <martin.doerr at sap.com>
> Subject: Re: PING: RFR: 8250598: Hyper-V is detected in spite of running on
> host OS
> 
> On 2020/08/18 10:42, David Holmes wrote:
> > Hi Yasumasa,
> >
> > On 14/08/2020 12:36 pm, Yasumasa Suenaga wrote:
> >> Hi David, Matthias, Martin,
> >>
> >> I uploaded new webrev. Could you review again?
> >>
> >>    http://cr.openjdk.java.net/~ysuenaga/JDK-8250598/webrev.02/
> >>
> >> It generates runtime stub for hypervisor detection, and it can distinguish
> between Hyper-V host (root partition) and guest. And also it works on 32bit
> platforms.
> >>
> >> I measured startup performance (java --version) with Measure-Command
> on PowerShell, this change is faster than current implementation.
> >>
> >> TotalMilliseconds : 57.8196
> >> TotalMilliseconds : 66.8379
> >> TotalMilliseconds : 64.7693
> >> TotalMilliseconds : 55.6546
> >> TotalMilliseconds : 63.848
> >> average : 61.78588
> >
> > This seems good to me, but again I can't review the details. My main
> concern is how you are testing this across a range of virtualized hosts? But if it
> fixes your Hyper-V issue and doesn't break anything else then I am fine with
> it.
> 
> Thanks David!
> 
> I don't have any other virtualized hosts excepting Hyper-V, so I cannot test
> any more.
> However detection logic is equivalent from current implementation, so I
> believe it does not break anything else if it works fine on Hyper-V.
> 
> And also Matthias has queued this change to internal tests in SAP. I guess he
> will share the result next week.
> 
> 
> > I will leave the technical review to Martin and Matthias.
> >
> > FYI I'm heading out on a weeks vacation after today so won't be able to
> follow up further.
> 
> Have a nice vacation!
> 
> 
> Thanks,
> 
> Yasumasa
> 
> 
> > Thanks,
> > David
> >
> >> This change has passed tests on submit repo (mach5-one-ysuenaga-JDK-
> 8250598-20200814-0119-13424118), and also it works fine on following
> environments:
> >>
> >>    - Hyper-V host (Windows 10 x64)
> >>    - Hyper-V guest (Windows 10 x64)
> >>    - Hyper-V guest (Linux x64)
> >>    - Hyper-V guest (32bit JDK on Linux x64)
> >>
> >>
> >> Thanks,
> >>
> >> Yasumasa
> >>
> >>
> >> On 2020/08/13 20:39, David Holmes wrote:
> >>> On 13/08/2020 5:39 pm, Yasumasa Suenaga wrote:
> >>>> Hi Matthias, David,
> >>>>
> >>>> On 2020/08/13 15:52, Baesken, Matthias wrote:
> >>>>>
> >>>>>>
> >>>>>> Should we make the change to determine just before it is needed
> (e.g. VM.info or hs_err log) at first?
> >>>>>> It is out of scope of this change, so I want to work as another issue if
> it is needed.
> >>>>>>
> >>>>>
> >>>>> Hi ,
> >>>>> I think we better do not call into WMI , when  writing an hs_err file   .
> >>>>
> >>>> I found out from Hypervisor Top Level Functional Specification [1]
> section 2.4.8.
> >>>> That says CPUID leaf 0x40000007 is available to the root partition only.
> >>>>
> >>>> I changed VM_Version::is_in_VM() as below, and it works fine.
> >>>> (I use __cpuid intrinsic in here because this code would be compiled by
> cl.exe only.)
> >>>>
> >>>> ```
> >>>> #include <intrin.h>
> >>>>
> >>>> #define EAX 0
> >>>> #define EBX 1
> >>>> #define ECX 2
> >>>> #define EDX 3
> >>>>
> >>>> bool VM_Version::is_in_VM() {
> >>>>    int regs[4];
> >>>>    __cpuid(regs, 0x40000007);
> >>>>
> >>>>    // CPUID leaf 0x40000007 is available to the root partition only.
> >>>>    return (regs[EAX] == 0x0) && (regs[EBX] == 0x0) && (regs[ECX] ==
> 0x0) && (regs[EDX] == 0x0);
> >>>> }
> >>>> ```
> >>>>
> >>>> Also startup performance is better than WMI call.
> >>>>
> >>>> TotalMilliseconds : 66.7863
> >>>> TotalMilliseconds : 56.8123
> >>>> TotalMilliseconds : 57.0015
> >>>> TotalMilliseconds : 62.4755
> >>>> TotalMilliseconds : 67.7632
> >>>> average : 62.16776
> >>>>
> >>>> If you are ok this change, I will update webrev, and will send review
> request.
> >>>> (new webrev will include both loop for CPUID leaf and renaming
> is_in_VM)
> >>>
> >>> If it works and addresses the startup hit then I'm fine with it - but still
> can't actually review the code.
> >>>
> >>> Thanks,
> >>> David
> >>>
> >>>>
> >>>> Thanks,
> >>>>
> >>>> Yasumasa
> >>>>
> >>>>
> >>>> [1] https://github.com/MicrosoftDocs/Virtualization-
> Documentation/raw/master/tlfs/Hypervisor%20Top%20Level%20Functional
> %20Specification%20v6.0b.pdf
> >>>>
> >>>>
> >>>>> Best regards, Matthias
> >>>>>
> >>>>>
> >>>>>
> >>>>> -----Original Message-----
> >>>>> From: Yasumasa Suenaga <suenaga at oss.nttdata.com>
> >>>>> Sent: Donnerstag, 13. August 2020 06:15
> >>>>> To: David Holmes <david.holmes at oracle.com>; Baesken, Matthias
> <matthias.baesken at sap.com>; hotspot-runtime-dev at openjdk.java.net;
> build-dev at openjdk.java.net
> >>>>> Cc: Doerr, Martin <martin.doerr at sap.com>
> >>>>> Subject: Re: PING: RFR: 8250598: Hyper-V is detected in spite of
> running on host OS
> >>>>>
> >>>>> On 2020/08/13 11:54, David Holmes wrote:
> >>>>>> On 13/08/2020 11:12 am, Yasumasa Suenaga wrote:
> >>>>>>> Hi Matthias, David,
> >>>>>>>
> >>>>>>> I measured startup benchmarks with `Measure-Command
> {.\jdk\build\windows-x86_64-server-release\images\jdk\bin\java.exe --
> version}` on PowerShell.
> >>>>>>>
> >>>>>>> * PC: Ryzen 3 3300X, 16GB memory
> >>>>>>> * OS: Windows 10 x64 (May 2020 Update)
> >>>>>>> * Java: jdk/jdk revision 60537
> >>>>>>>       (Compiled by VS 2019 (16.7.0))
> >>>>>>>
> >>>>>>> * without patch
> >>>>>>> TotalMilliseconds : 70.2124
> >>>>>>> TotalMilliseconds : 64.4465
> >>>>>>> TotalMilliseconds : 59.0854
> >>>>>>> TotalMilliseconds : 68.0255
> >>>>>>> TotalMilliseconds : 72.6467
> >>>>>>> average : 66.8833
> >>>>>>>
> >>>>>>> * with webrev.01
> >>>>>>> TotalMilliseconds : 81.7185
> >>>>>>> TotalMilliseconds : 68.539
> >>>>>>> TotalMilliseconds : 85.7226
> >>>>>>> TotalMilliseconds : 72.6584
> >>>>>>> TotalMilliseconds : 75.6091
> >>>>>>> average : 76.84952
> >>>>>>>
> >>>>>>> Overhead of WMI seems to be +10ms in this case.
> >>>>>>
> >>>>>> Which is nearly 15%! Sorry but I just know Claes will be very unhappy
> if this were to go in as-is.
> >>>>>
> >>>>> Should we make the change to determine just before it is needed
> (e.g. VM.info or hs_err log) at first?
> >>>>> It is out of scope of this change, so I want to work as another issue if it
> is needed.
> >>>>>
> >>>>>
> >>>>> Yasumasa
> >>>>>
> >>>>>
> >>>>>> David
> >>>>>> -----
> >>>>>>
> >>>>>>>
> >>>>>>> Yasumasa
> >>>>>>>
> >>>>>>>
> >>>>>>> On 2020/08/13 0:05, Baesken, Matthias wrote:
> >>>>>>>>> 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.
> >>>>>>>>
> >>>>>>>> Hi Yasumasa  , thanks !
> >>>>>>>>
> >>>>>>>> Regarding the WMI overhead , if you could  get some more info
> on this it would be better to judge if it is perfectly okay or concerning .
> >>>>>>>>
> >>>>>>>> (I remember that I looked into it a couple of years ago , but
> decided not to use it in early VM startup ;  but have to confess this was just
> based on a "gut feeling",
> >>>>>>>>    No micro benchmarks  )
> >>>>>>>>
> >>>>>>>> Best regards, Matthias
> >>>>>>>>
> >>>>>>>>