RFR: 8219241: Provide basic virtualization related info in the hs_error file on linux/windows x86_64

[David Holmes]

Hi Matthias,

This is looking much better/simpler - thanks!

My remaining query is why we need the OS specific checks and code in 
os-cpu files? Isn't this just cpu specific? Even if a particular OS 
doesn't support virtualization, won't the cpuid query simply report "no 
virtualization"?

Thanks,
David

On 4/04/2019 12:43 am, Baesken, Matthias wrote:
>> 1. Why do we have/need os::<os>::print_virtualization_info when we also
>> have VM_Version::print_platform_virtualization_info?
>>
> 
> Hi David,  I have to agree  -  we do not really need both .
> So I removed   os::<os>::print_virtualization_info    and   call    VM_Version::print_platform_virtualization_info(st);     at  the places where the output is done.
> 
> Regarding point  2. :
> 
> I now  just use   print_platform_virtualization_info,   print_detected_virtualization  has been removed .
> print_platform_virtualization_info   is doing  the output   (differs  for  vm_version_  s390 /  ppc / x86_64 ) .
> 
> The  new webrev   is simpler, with less methods  :
> 
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8219241.4/
> 
> Best regards, Matthias
> 
> 
>> -----Original Message-----
>> From: David Holmes <david.holmes at oracle.com>
>> Sent: Freitag, 29. März 2019 01:16
>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
>> Cc: Doerr, Martin <martin.doerr at sap.com>
>> Subject: Re: RFR: 8219241: Provide basic virtualization related info in the
>> hs_error file on linux/windows x86_64
>>
>> Hi Matthias,
>>
>> I have two issues with this API:
>>
>> 1. Why do we have/need os::<os>::print_virtualization_info when we also
>> have VM_Version::print_platform_virtualization_info?
>>
>> 2. I don't like the fact that the there are two ways to define the
>> platform specific information:
>>     a) override VM_Version::print_platform_virtualization_info; or
>>     b) hook into the get_detected_virtualization switch
>>
>> And IIUC code that uses (a) relies on the switch doing nothing
>> (NoVirtualization) and code that uses (b) relies on
>> print_platform_virtualization_info doing nothing!
>>
>> Why doesn't the default implementation of
>> VM_Version::print_platform_virtualization_info define the
>> get_detected_virtualization() switch logic, and do away with
>> Abstract_VM_Version::print_detected_virtualization? Code that wants to
>> print this info can just call
>> VM_Version::print_platform_virtualization_info().
>>
>> Sorry but I just find the current proposal has too many methods and an
>> unclear structure.
>>
>> David
>