Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2/16/22 13:26, Pierre Morel wrote:


On 2/16/22 09:13, Janosch Frank wrote:
On 2/15/22 18:30, Pierre Morel wrote:


On 2/15/22 16:21, Claudio Imbrenda wrote:
On Tue, 15 Feb 2022 16:08:16 +0100
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:

On 2/15/22 13:06, Claudio Imbrenda wrote:
On Tue, 15 Feb 2022 11:46:32 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:
Several tests are in need of a way to check on which hypervisor
and virtualization level they are running on to be able to fence
certain tests. This patch adds functions that return true if a
vm is running under KVM, LPAR or generally as a level 2 guest.

To check if we're running under KVM we use the STSI 3.2.2
instruction, let's define it's response structure in a central
header.

sorry, I had replied to the old series, let me reply here too


I think it would look cleaner if there was only one
"detect_environment" function, that would call stsi once and detect
the
environment, then the various vm_is_* would become something like

bool vm_is_*(void)
{
     return detect_environment() == VM_IS_*;
}

of course detect_environment would also cache the result with static
variables.

bonus, we could make that function public, so a testcase could just
switch over the type of hypervisor it's being run on, instead of
having
to use a series of ifs.

and then maybe the various vm_is_* could become static inlines to
be put
in the header.

please note that "detect_environment" is just the first thing that
came
to my mind, I have no preference regarding the name.

I'd like to keep this patch as simple as possible because there are
multiple patch sets which are gated by it.

The vm.h code and the skey.c z/VM 6 check is a thorn in my side anyway
and I'd rather have it fixed properly which will likely result in a lot
of opinions being voiced.

So I'd propose to rename vm_is_vm() to vm_is_guest2() and pick this
patch.

ok for me

I'll rename the function and queue the patch


Not OK for me, in the POP PTF do not do any difference between guest 2
and guest 3.

If we're running with HW virtualization then every guest >= 2 is a guest
2 at the end. And most of the time we don't want to know the HW level
anyway, we want to know who our hypervisor is and vm_is_vm() doesn't
tell you one bit about that.

It tells us that we are running under a VM, the POP defines 1 as the
basic machine, 2 as the LPAR and 3 as the Virtual Machine

I find this definition clear, much more clear than guest 2 that is why I
used it.


At this point I would be happier if we remove the function and use
stsi_get_fc() == 3 directly. There's no arguing about what we're
checking when using that.

Speaking of function name, I do not understand the name of this function
stsi_get_fc() : returning the function code ?

I guess it should be stsi_get_current_level() or something like that.
Let me add that to the list of vm.h rework items.



We're currently arguing about a function that's only used in this patch,
no?

It is absolutely unimportant for me, if you prefer this we do this.
I send the changes.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux