* [kvm-unit-tests PATCH 0/1] s390x: stsi: Define vm_is_kvm to be used in different tests @ 2022-02-15 10:46 Pierre Morel 2022-02-15 10:46 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel 0 siblings, 1 reply; 9+ messages in thread From: Pierre Morel @ 2022-02-15 10:46 UTC (permalink / raw) To: linux-s390; +Cc: frankja, thuth, kvm, cohuck, imbrenda, david, nrb 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. Pierre Morel (1): s390x: stsi: Define vm_is_kvm to be used in different tests lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- lib/s390x/vm.h | 3 +++ s390x/stsi.c | 23 ++------------------ 4 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 lib/s390x/stsi.h -- 2.27.0 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 10:46 [kvm-unit-tests PATCH 0/1] s390x: stsi: Define vm_is_kvm to be used in different tests Pierre Morel @ 2022-02-15 10:46 ` Pierre Morel 2022-02-15 12:06 ` Claudio Imbrenda 0 siblings, 1 reply; 9+ messages in thread From: Pierre Morel @ 2022-02-15 10:46 UTC (permalink / raw) To: linux-s390; +Cc: frankja, thuth, kvm, cohuck, imbrenda, david, nrb 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. Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> --- lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- lib/s390x/vm.h | 3 +++ s390x/stsi.c | 23 ++------------------ 4 files changed, 90 insertions(+), 23 deletions(-) create mode 100644 lib/s390x/stsi.h diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h new file mode 100644 index 00000000..bebc492d --- /dev/null +++ b/lib/s390x/stsi.h @@ -0,0 +1,32 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Structures used to Store System Information + * + * Copyright IBM Corp. 2022 + */ + +#ifndef _S390X_STSI_H_ +#define _S390X_STSI_H_ + +struct sysinfo_3_2_2 { + uint8_t reserved[31]; + uint8_t count; + struct { + uint8_t reserved2[4]; + uint16_t total_cpus; + uint16_t conf_cpus; + uint16_t standby_cpus; + uint16_t reserved_cpus; + uint8_t name[8]; + uint32_t caf; + uint8_t cpi[16]; + uint8_t reserved5[3]; + uint8_t ext_name_encoding; + uint32_t reserved3; + uint8_t uuid[16]; + } vm[8]; + uint8_t reserved4[1504]; + uint8_t ext_names[8][256]; +}; + +#endif /* _S390X_STSI_H_ */ diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c index a5b92863..91acd05b 100644 --- a/lib/s390x/vm.c +++ b/lib/s390x/vm.c @@ -12,6 +12,7 @@ #include <alloc_page.h> #include <asm/arch_def.h> #include "vm.h" +#include "stsi.h" /** * Detect whether we are running with TCG (instead of KVM) @@ -26,9 +27,13 @@ bool vm_is_tcg(void) if (initialized) return is_tcg; + if (!vm_is_vm()) { + initialized = true; + return is_tcg; + } + buf = alloc_page(); - if (!buf) - return false; + assert(buf); if (stsi(buf, 1, 1, 1)) goto out; @@ -43,3 +48,49 @@ out: free_page(buf); return is_tcg; } + +/** + * Detect whether we are running with KVM + */ +bool vm_is_kvm(void) +{ + /* EBCDIC for "KVM/" */ + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; + static bool initialized; + static bool is_kvm; + struct sysinfo_3_2_2 *stsi_322; + + if (initialized) + return is_kvm; + + if (!vm_is_vm() || vm_is_tcg()) { + initialized = true; + return is_kvm; + } + + stsi_322 = alloc_page(); + assert(stsi_322); + + if (stsi(stsi_322, 3, 2, 2)) + goto out; + + /* + * If the manufacturer string is "KVM/" in EBCDIC, then we + * are on KVM. + */ + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); + initialized = true; +out: + free_page(stsi_322); + return is_kvm; +} + +bool vm_is_lpar(void) +{ + return stsi_get_fc() == 2; +} + +bool vm_is_vm(void) +{ + return stsi_get_fc() == 3; +} diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h index 7abba0cc..3aaf76af 100644 --- a/lib/s390x/vm.h +++ b/lib/s390x/vm.h @@ -9,5 +9,8 @@ #define _S390X_VM_H_ bool vm_is_tcg(void); +bool vm_is_kvm(void); +bool vm_is_vm(void); +bool vm_is_lpar(void); #endif /* _S390X_VM_H_ */ diff --git a/s390x/stsi.c b/s390x/stsi.c index 391f8849..dccc53e7 100644 --- a/s390x/stsi.c +++ b/s390x/stsi.c @@ -13,27 +13,8 @@ #include <asm/asm-offsets.h> #include <asm/interrupt.h> #include <smp.h> +#include <stsi.h> -struct stsi_322 { - uint8_t reserved[31]; - uint8_t count; - struct { - uint8_t reserved2[4]; - uint16_t total_cpus; - uint16_t conf_cpus; - uint16_t standby_cpus; - uint16_t reserved_cpus; - uint8_t name[8]; - uint32_t caf; - uint8_t cpi[16]; - uint8_t reserved5[3]; - uint8_t ext_name_encoding; - uint32_t reserved3; - uint8_t uuid[16]; - } vm[8]; - uint8_t reserved4[1504]; - uint8_t ext_names[8][256]; -}; static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); static void test_specs(void) @@ -91,7 +72,7 @@ static void test_3_2_2(void) /* EBCDIC for "KVM/" */ const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; const char vm_name_ext[] = "kvm-unit-test"; - struct stsi_322 *data = (void *)pagebuf; + struct sysinfo_3_2_2 *data = (void *)pagebuf; report_prefix_push("3.2.2"); -- 2.27.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 10:46 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel @ 2022-02-15 12:06 ` Claudio Imbrenda 2022-02-15 15:08 ` Janosch Frank 0 siblings, 1 reply; 9+ messages in thread From: Claudio Imbrenda @ 2022-02-15 12:06 UTC (permalink / raw) To: Pierre Morel; +Cc: linux-s390, frankja, thuth, kvm, cohuck, david, nrb On Tue, 15 Feb 2022 11:46:32 +0100 Pierre Morel <pmorel@linux.ibm.com> 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. > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > --- > lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ > lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- > lib/s390x/vm.h | 3 +++ > s390x/stsi.c | 23 ++------------------ > 4 files changed, 90 insertions(+), 23 deletions(-) > create mode 100644 lib/s390x/stsi.h > > diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h > new file mode 100644 > index 00000000..bebc492d > --- /dev/null > +++ b/lib/s390x/stsi.h > @@ -0,0 +1,32 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Structures used to Store System Information > + * > + * Copyright IBM Corp. 2022 > + */ > + > +#ifndef _S390X_STSI_H_ > +#define _S390X_STSI_H_ > + > +struct sysinfo_3_2_2 { > + uint8_t reserved[31]; > + uint8_t count; > + struct { > + uint8_t reserved2[4]; > + uint16_t total_cpus; > + uint16_t conf_cpus; > + uint16_t standby_cpus; > + uint16_t reserved_cpus; > + uint8_t name[8]; > + uint32_t caf; > + uint8_t cpi[16]; > + uint8_t reserved5[3]; > + uint8_t ext_name_encoding; > + uint32_t reserved3; > + uint8_t uuid[16]; > + } vm[8]; > + uint8_t reserved4[1504]; > + uint8_t ext_names[8][256]; > +}; > + > +#endif /* _S390X_STSI_H_ */ > diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c > index a5b92863..91acd05b 100644 > --- a/lib/s390x/vm.c > +++ b/lib/s390x/vm.c > @@ -12,6 +12,7 @@ > #include <alloc_page.h> > #include <asm/arch_def.h> > #include "vm.h" > +#include "stsi.h" > > /** > * Detect whether we are running with TCG (instead of KVM) > @@ -26,9 +27,13 @@ bool vm_is_tcg(void) > if (initialized) > return is_tcg; > > + if (!vm_is_vm()) { > + initialized = true; > + return is_tcg; > + } > + > buf = alloc_page(); > - if (!buf) > - return false; > + assert(buf); > > if (stsi(buf, 1, 1, 1)) > goto out; > @@ -43,3 +48,49 @@ out: > free_page(buf); > return is_tcg; > } > + > +/** > + * Detect whether we are running with KVM > + */ > +bool vm_is_kvm(void) > +{ > + /* EBCDIC for "KVM/" */ > + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > + static bool initialized; > + static bool is_kvm; > + struct sysinfo_3_2_2 *stsi_322; > + > + if (initialized) > + return is_kvm; > + > + if (!vm_is_vm() || vm_is_tcg()) { > + initialized = true; > + return is_kvm; > + } > + > + stsi_322 = alloc_page(); > + assert(stsi_322); > + > + if (stsi(stsi_322, 3, 2, 2)) > + goto out; > + > + /* > + * If the manufacturer string is "KVM/" in EBCDIC, then we > + * are on KVM. > + */ > + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); > + initialized = true; > +out: > + free_page(stsi_322); > + return is_kvm; > +} > + > +bool vm_is_lpar(void) > +{ > + return stsi_get_fc() == 2; > +} > + > +bool vm_is_vm(void) > +{ > + return stsi_get_fc() == 3; > +} > diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h > index 7abba0cc..3aaf76af 100644 > --- a/lib/s390x/vm.h > +++ b/lib/s390x/vm.h > @@ -9,5 +9,8 @@ > #define _S390X_VM_H_ > > bool vm_is_tcg(void); > +bool vm_is_kvm(void); > +bool vm_is_vm(void); > +bool vm_is_lpar(void); > > #endif /* _S390X_VM_H_ */ > diff --git a/s390x/stsi.c b/s390x/stsi.c > index 391f8849..dccc53e7 100644 > --- a/s390x/stsi.c > +++ b/s390x/stsi.c > @@ -13,27 +13,8 @@ > #include <asm/asm-offsets.h> > #include <asm/interrupt.h> > #include <smp.h> > +#include <stsi.h> > > -struct stsi_322 { > - uint8_t reserved[31]; > - uint8_t count; > - struct { > - uint8_t reserved2[4]; > - uint16_t total_cpus; > - uint16_t conf_cpus; > - uint16_t standby_cpus; > - uint16_t reserved_cpus; > - uint8_t name[8]; > - uint32_t caf; > - uint8_t cpi[16]; > - uint8_t reserved5[3]; > - uint8_t ext_name_encoding; > - uint32_t reserved3; > - uint8_t uuid[16]; > - } vm[8]; > - uint8_t reserved4[1504]; > - uint8_t ext_names[8][256]; > -}; > static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); > > static void test_specs(void) > @@ -91,7 +72,7 @@ static void test_3_2_2(void) > /* EBCDIC for "KVM/" */ > const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > const char vm_name_ext[] = "kvm-unit-test"; > - struct stsi_322 *data = (void *)pagebuf; > + struct sysinfo_3_2_2 *data = (void *)pagebuf; > > report_prefix_push("3.2.2"); > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 12:06 ` Claudio Imbrenda @ 2022-02-15 15:08 ` Janosch Frank 2022-02-15 15:21 ` Claudio Imbrenda 0 siblings, 1 reply; 9+ messages in thread From: Janosch Frank @ 2022-02-15 15:08 UTC (permalink / raw) To: Claudio Imbrenda, Pierre Morel; +Cc: linux-s390, thuth, kvm, cohuck, david, nrb On 2/15/22 13:06, Claudio Imbrenda wrote: > On Tue, 15 Feb 2022 11:46:32 +0100 > Pierre Morel <pmorel@linux.ibm.com> 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. > >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >> --- >> lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ >> lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- >> lib/s390x/vm.h | 3 +++ >> s390x/stsi.c | 23 ++------------------ >> 4 files changed, 90 insertions(+), 23 deletions(-) >> create mode 100644 lib/s390x/stsi.h >> >> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h >> new file mode 100644 >> index 00000000..bebc492d >> --- /dev/null >> +++ b/lib/s390x/stsi.h >> @@ -0,0 +1,32 @@ >> +/* SPDX-License-Identifier: GPL-2.0-only */ >> +/* >> + * Structures used to Store System Information >> + * >> + * Copyright IBM Corp. 2022 >> + */ >> + >> +#ifndef _S390X_STSI_H_ >> +#define _S390X_STSI_H_ >> + >> +struct sysinfo_3_2_2 { >> + uint8_t reserved[31]; >> + uint8_t count; >> + struct { >> + uint8_t reserved2[4]; >> + uint16_t total_cpus; >> + uint16_t conf_cpus; >> + uint16_t standby_cpus; >> + uint16_t reserved_cpus; >> + uint8_t name[8]; >> + uint32_t caf; >> + uint8_t cpi[16]; >> + uint8_t reserved5[3]; >> + uint8_t ext_name_encoding; >> + uint32_t reserved3; >> + uint8_t uuid[16]; >> + } vm[8]; >> + uint8_t reserved4[1504]; >> + uint8_t ext_names[8][256]; >> +}; >> + >> +#endif /* _S390X_STSI_H_ */ >> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >> index a5b92863..91acd05b 100644 >> --- a/lib/s390x/vm.c >> +++ b/lib/s390x/vm.c >> @@ -12,6 +12,7 @@ >> #include <alloc_page.h> >> #include <asm/arch_def.h> >> #include "vm.h" >> +#include "stsi.h" >> >> /** >> * Detect whether we are running with TCG (instead of KVM) >> @@ -26,9 +27,13 @@ bool vm_is_tcg(void) >> if (initialized) >> return is_tcg; >> >> + if (!vm_is_vm()) { >> + initialized = true; >> + return is_tcg; >> + } >> + >> buf = alloc_page(); >> - if (!buf) >> - return false; >> + assert(buf); >> >> if (stsi(buf, 1, 1, 1)) >> goto out; >> @@ -43,3 +48,49 @@ out: >> free_page(buf); >> return is_tcg; >> } >> + >> +/** >> + * Detect whether we are running with KVM >> + */ >> +bool vm_is_kvm(void) >> +{ >> + /* EBCDIC for "KVM/" */ >> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >> + static bool initialized; >> + static bool is_kvm; >> + struct sysinfo_3_2_2 *stsi_322; >> + >> + if (initialized) >> + return is_kvm; >> + >> + if (!vm_is_vm() || vm_is_tcg()) { >> + initialized = true; >> + return is_kvm; >> + } >> + >> + stsi_322 = alloc_page(); >> + assert(stsi_322); >> + >> + if (stsi(stsi_322, 3, 2, 2)) >> + goto out; >> + >> + /* >> + * If the manufacturer string is "KVM/" in EBCDIC, then we >> + * are on KVM. >> + */ >> + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); >> + initialized = true; >> +out: >> + free_page(stsi_322); >> + return is_kvm; >> +} >> + >> +bool vm_is_lpar(void) >> +{ >> + return stsi_get_fc() == 2; >> +} >> + >> +bool vm_is_vm(void) >> +{ >> + return stsi_get_fc() == 3; >> +} >> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h >> index 7abba0cc..3aaf76af 100644 >> --- a/lib/s390x/vm.h >> +++ b/lib/s390x/vm.h >> @@ -9,5 +9,8 @@ >> #define _S390X_VM_H_ >> >> bool vm_is_tcg(void); >> +bool vm_is_kvm(void); >> +bool vm_is_vm(void); >> +bool vm_is_lpar(void); >> >> #endif /* _S390X_VM_H_ */ >> diff --git a/s390x/stsi.c b/s390x/stsi.c >> index 391f8849..dccc53e7 100644 >> --- a/s390x/stsi.c >> +++ b/s390x/stsi.c >> @@ -13,27 +13,8 @@ >> #include <asm/asm-offsets.h> >> #include <asm/interrupt.h> >> #include <smp.h> >> +#include <stsi.h> >> >> -struct stsi_322 { >> - uint8_t reserved[31]; >> - uint8_t count; >> - struct { >> - uint8_t reserved2[4]; >> - uint16_t total_cpus; >> - uint16_t conf_cpus; >> - uint16_t standby_cpus; >> - uint16_t reserved_cpus; >> - uint8_t name[8]; >> - uint32_t caf; >> - uint8_t cpi[16]; >> - uint8_t reserved5[3]; >> - uint8_t ext_name_encoding; >> - uint32_t reserved3; >> - uint8_t uuid[16]; >> - } vm[8]; >> - uint8_t reserved4[1504]; >> - uint8_t ext_names[8][256]; >> -}; >> static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); >> >> static void test_specs(void) >> @@ -91,7 +72,7 @@ static void test_3_2_2(void) >> /* EBCDIC for "KVM/" */ >> const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >> const char vm_name_ext[] = "kvm-unit-test"; >> - struct stsi_322 *data = (void *)pagebuf; >> + struct sysinfo_3_2_2 *data = (void *)pagebuf; >> >> report_prefix_push("3.2.2"); >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 15:08 ` Janosch Frank @ 2022-02-15 15:21 ` Claudio Imbrenda 2022-02-15 17:30 ` Pierre Morel 0 siblings, 1 reply; 9+ messages in thread From: Claudio Imbrenda @ 2022-02-15 15:21 UTC (permalink / raw) To: Janosch Frank; +Cc: Pierre Morel, linux-s390, thuth, kvm, cohuck, david, nrb On Tue, 15 Feb 2022 16:08:16 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 2/15/22 13:06, Claudio Imbrenda wrote: > > On Tue, 15 Feb 2022 11:46:32 +0100 > > Pierre Morel <pmorel@linux.ibm.com> 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 > > > > >> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > >> --- > >> lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ > >> lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- > >> lib/s390x/vm.h | 3 +++ > >> s390x/stsi.c | 23 ++------------------ > >> 4 files changed, 90 insertions(+), 23 deletions(-) > >> create mode 100644 lib/s390x/stsi.h > >> > >> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h > >> new file mode 100644 > >> index 00000000..bebc492d > >> --- /dev/null > >> +++ b/lib/s390x/stsi.h > >> @@ -0,0 +1,32 @@ > >> +/* SPDX-License-Identifier: GPL-2.0-only */ > >> +/* > >> + * Structures used to Store System Information > >> + * > >> + * Copyright IBM Corp. 2022 > >> + */ > >> + > >> +#ifndef _S390X_STSI_H_ > >> +#define _S390X_STSI_H_ > >> + > >> +struct sysinfo_3_2_2 { > >> + uint8_t reserved[31]; > >> + uint8_t count; > >> + struct { > >> + uint8_t reserved2[4]; > >> + uint16_t total_cpus; > >> + uint16_t conf_cpus; > >> + uint16_t standby_cpus; > >> + uint16_t reserved_cpus; > >> + uint8_t name[8]; > >> + uint32_t caf; > >> + uint8_t cpi[16]; > >> + uint8_t reserved5[3]; > >> + uint8_t ext_name_encoding; > >> + uint32_t reserved3; > >> + uint8_t uuid[16]; > >> + } vm[8]; > >> + uint8_t reserved4[1504]; > >> + uint8_t ext_names[8][256]; > >> +}; > >> + > >> +#endif /* _S390X_STSI_H_ */ > >> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c > >> index a5b92863..91acd05b 100644 > >> --- a/lib/s390x/vm.c > >> +++ b/lib/s390x/vm.c > >> @@ -12,6 +12,7 @@ > >> #include <alloc_page.h> > >> #include <asm/arch_def.h> > >> #include "vm.h" > >> +#include "stsi.h" > >> > >> /** > >> * Detect whether we are running with TCG (instead of KVM) > >> @@ -26,9 +27,13 @@ bool vm_is_tcg(void) > >> if (initialized) > >> return is_tcg; > >> > >> + if (!vm_is_vm()) { > >> + initialized = true; > >> + return is_tcg; > >> + } > >> + > >> buf = alloc_page(); > >> - if (!buf) > >> - return false; > >> + assert(buf); > >> > >> if (stsi(buf, 1, 1, 1)) > >> goto out; > >> @@ -43,3 +48,49 @@ out: > >> free_page(buf); > >> return is_tcg; > >> } > >> + > >> +/** > >> + * Detect whether we are running with KVM > >> + */ > >> +bool vm_is_kvm(void) > >> +{ > >> + /* EBCDIC for "KVM/" */ > >> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > >> + static bool initialized; > >> + static bool is_kvm; > >> + struct sysinfo_3_2_2 *stsi_322; > >> + > >> + if (initialized) > >> + return is_kvm; > >> + > >> + if (!vm_is_vm() || vm_is_tcg()) { > >> + initialized = true; > >> + return is_kvm; > >> + } > >> + > >> + stsi_322 = alloc_page(); > >> + assert(stsi_322); > >> + > >> + if (stsi(stsi_322, 3, 2, 2)) > >> + goto out; > >> + > >> + /* > >> + * If the manufacturer string is "KVM/" in EBCDIC, then we > >> + * are on KVM. > >> + */ > >> + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); > >> + initialized = true; > >> +out: > >> + free_page(stsi_322); > >> + return is_kvm; > >> +} > >> + > >> +bool vm_is_lpar(void) > >> +{ > >> + return stsi_get_fc() == 2; > >> +} > >> + > >> +bool vm_is_vm(void) > >> +{ > >> + return stsi_get_fc() == 3; > >> +} > >> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h > >> index 7abba0cc..3aaf76af 100644 > >> --- a/lib/s390x/vm.h > >> +++ b/lib/s390x/vm.h > >> @@ -9,5 +9,8 @@ > >> #define _S390X_VM_H_ > >> > >> bool vm_is_tcg(void); > >> +bool vm_is_kvm(void); > >> +bool vm_is_vm(void); > >> +bool vm_is_lpar(void); > >> > >> #endif /* _S390X_VM_H_ */ > >> diff --git a/s390x/stsi.c b/s390x/stsi.c > >> index 391f8849..dccc53e7 100644 > >> --- a/s390x/stsi.c > >> +++ b/s390x/stsi.c > >> @@ -13,27 +13,8 @@ > >> #include <asm/asm-offsets.h> > >> #include <asm/interrupt.h> > >> #include <smp.h> > >> +#include <stsi.h> > >> > >> -struct stsi_322 { > >> - uint8_t reserved[31]; > >> - uint8_t count; > >> - struct { > >> - uint8_t reserved2[4]; > >> - uint16_t total_cpus; > >> - uint16_t conf_cpus; > >> - uint16_t standby_cpus; > >> - uint16_t reserved_cpus; > >> - uint8_t name[8]; > >> - uint32_t caf; > >> - uint8_t cpi[16]; > >> - uint8_t reserved5[3]; > >> - uint8_t ext_name_encoding; > >> - uint32_t reserved3; > >> - uint8_t uuid[16]; > >> - } vm[8]; > >> - uint8_t reserved4[1504]; > >> - uint8_t ext_names[8][256]; > >> -}; > >> static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); > >> > >> static void test_specs(void) > >> @@ -91,7 +72,7 @@ static void test_3_2_2(void) > >> /* EBCDIC for "KVM/" */ > >> const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > >> const char vm_name_ext[] = "kvm-unit-test"; > >> - struct stsi_322 *data = (void *)pagebuf; > >> + struct sysinfo_3_2_2 *data = (void *)pagebuf; > >> > >> report_prefix_push("3.2.2"); > >> > > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 15:21 ` Claudio Imbrenda @ 2022-02-15 17:30 ` Pierre Morel 2022-02-16 8:13 ` Janosch Frank 0 siblings, 1 reply; 9+ messages in thread From: Pierre Morel @ 2022-02-15 17:30 UTC (permalink / raw) To: Claudio Imbrenda, Janosch Frank Cc: linux-s390, thuth, kvm, cohuck, david, nrb On 2/15/22 16:21, Claudio Imbrenda wrote: > On Tue, 15 Feb 2022 16:08:16 +0100 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> On 2/15/22 13:06, Claudio Imbrenda wrote: >>> On Tue, 15 Feb 2022 11:46:32 +0100 >>> Pierre Morel <pmorel@linux.ibm.com> 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. Or do you want to implement a VSIE specificity ? >> >>> >>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>> --- >>>> lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ >>>> lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- >>>> lib/s390x/vm.h | 3 +++ >>>> s390x/stsi.c | 23 ++------------------ >>>> 4 files changed, 90 insertions(+), 23 deletions(-) >>>> create mode 100644 lib/s390x/stsi.h >>>> >>>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h >>>> new file mode 100644 >>>> index 00000000..bebc492d >>>> --- /dev/null >>>> +++ b/lib/s390x/stsi.h >>>> @@ -0,0 +1,32 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>>> +/* >>>> + * Structures used to Store System Information >>>> + * >>>> + * Copyright IBM Corp. 2022 >>>> + */ >>>> + >>>> +#ifndef _S390X_STSI_H_ >>>> +#define _S390X_STSI_H_ >>>> + >>>> +struct sysinfo_3_2_2 { >>>> + uint8_t reserved[31]; >>>> + uint8_t count; >>>> + struct { >>>> + uint8_t reserved2[4]; >>>> + uint16_t total_cpus; >>>> + uint16_t conf_cpus; >>>> + uint16_t standby_cpus; >>>> + uint16_t reserved_cpus; >>>> + uint8_t name[8]; >>>> + uint32_t caf; >>>> + uint8_t cpi[16]; >>>> + uint8_t reserved5[3]; >>>> + uint8_t ext_name_encoding; >>>> + uint32_t reserved3; >>>> + uint8_t uuid[16]; >>>> + } vm[8]; >>>> + uint8_t reserved4[1504]; >>>> + uint8_t ext_names[8][256]; >>>> +}; >>>> + >>>> +#endif /* _S390X_STSI_H_ */ >>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >>>> index a5b92863..91acd05b 100644 >>>> --- a/lib/s390x/vm.c >>>> +++ b/lib/s390x/vm.c >>>> @@ -12,6 +12,7 @@ >>>> #include <alloc_page.h> >>>> #include <asm/arch_def.h> >>>> #include "vm.h" >>>> +#include "stsi.h" >>>> >>>> /** >>>> * Detect whether we are running with TCG (instead of KVM) >>>> @@ -26,9 +27,13 @@ bool vm_is_tcg(void) >>>> if (initialized) >>>> return is_tcg; >>>> >>>> + if (!vm_is_vm()) { >>>> + initialized = true; >>>> + return is_tcg; >>>> + } >>>> + >>>> buf = alloc_page(); >>>> - if (!buf) >>>> - return false; >>>> + assert(buf); >>>> >>>> if (stsi(buf, 1, 1, 1)) >>>> goto out; >>>> @@ -43,3 +48,49 @@ out: >>>> free_page(buf); >>>> return is_tcg; >>>> } >>>> + >>>> +/** >>>> + * Detect whether we are running with KVM >>>> + */ >>>> +bool vm_is_kvm(void) >>>> +{ >>>> + /* EBCDIC for "KVM/" */ >>>> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>> + static bool initialized; >>>> + static bool is_kvm; >>>> + struct sysinfo_3_2_2 *stsi_322; >>>> + >>>> + if (initialized) >>>> + return is_kvm; >>>> + >>>> + if (!vm_is_vm() || vm_is_tcg()) { >>>> + initialized = true; >>>> + return is_kvm; >>>> + } >>>> + >>>> + stsi_322 = alloc_page(); >>>> + assert(stsi_322); >>>> + >>>> + if (stsi(stsi_322, 3, 2, 2)) >>>> + goto out; >>>> + >>>> + /* >>>> + * If the manufacturer string is "KVM/" in EBCDIC, then we >>>> + * are on KVM. >>>> + */ >>>> + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); >>>> + initialized = true; >>>> +out: >>>> + free_page(stsi_322); >>>> + return is_kvm; >>>> +} >>>> + >>>> +bool vm_is_lpar(void) >>>> +{ >>>> + return stsi_get_fc() == 2; >>>> +} >>>> + >>>> +bool vm_is_vm(void) >>>> +{ >>>> + return stsi_get_fc() == 3; >>>> +} >>>> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h >>>> index 7abba0cc..3aaf76af 100644 >>>> --- a/lib/s390x/vm.h >>>> +++ b/lib/s390x/vm.h >>>> @@ -9,5 +9,8 @@ >>>> #define _S390X_VM_H_ >>>> >>>> bool vm_is_tcg(void); >>>> +bool vm_is_kvm(void); >>>> +bool vm_is_vm(void); >>>> +bool vm_is_lpar(void); >>>> >>>> #endif /* _S390X_VM_H_ */ >>>> diff --git a/s390x/stsi.c b/s390x/stsi.c >>>> index 391f8849..dccc53e7 100644 >>>> --- a/s390x/stsi.c >>>> +++ b/s390x/stsi.c >>>> @@ -13,27 +13,8 @@ >>>> #include <asm/asm-offsets.h> >>>> #include <asm/interrupt.h> >>>> #include <smp.h> >>>> +#include <stsi.h> >>>> >>>> -struct stsi_322 { >>>> - uint8_t reserved[31]; >>>> - uint8_t count; >>>> - struct { >>>> - uint8_t reserved2[4]; >>>> - uint16_t total_cpus; >>>> - uint16_t conf_cpus; >>>> - uint16_t standby_cpus; >>>> - uint16_t reserved_cpus; >>>> - uint8_t name[8]; >>>> - uint32_t caf; >>>> - uint8_t cpi[16]; >>>> - uint8_t reserved5[3]; >>>> - uint8_t ext_name_encoding; >>>> - uint32_t reserved3; >>>> - uint8_t uuid[16]; >>>> - } vm[8]; >>>> - uint8_t reserved4[1504]; >>>> - uint8_t ext_names[8][256]; >>>> -}; >>>> static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); >>>> >>>> static void test_specs(void) >>>> @@ -91,7 +72,7 @@ static void test_3_2_2(void) >>>> /* EBCDIC for "KVM/" */ >>>> const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>> const char vm_name_ext[] = "kvm-unit-test"; >>>> - struct stsi_322 *data = (void *)pagebuf; >>>> + struct sysinfo_3_2_2 *data = (void *)pagebuf; >>>> >>>> report_prefix_push("3.2.2"); >>>> >>> >> > -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-15 17:30 ` Pierre Morel @ 2022-02-16 8:13 ` Janosch Frank 2022-02-16 12:26 ` Pierre Morel 0 siblings, 1 reply; 9+ messages in thread From: Janosch Frank @ 2022-02-16 8:13 UTC (permalink / raw) To: Pierre Morel, Claudio Imbrenda; +Cc: linux-s390, thuth, kvm, cohuck, david, nrb 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@linux.ibm.com> wrote: >> >>> On 2/15/22 13:06, Claudio Imbrenda wrote: >>>> On Tue, 15 Feb 2022 11:46:32 +0100 >>>> Pierre Morel <pmorel@linux.ibm.com> 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. 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. We're currently arguing about a function that's only used in this patch, no? > > Or do you want to implement a VSIE specificity ? > > >>> >>>> >>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>>> --- >>>>> lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ >>>>> lib/s390x/vm.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++-- >>>>> lib/s390x/vm.h | 3 +++ >>>>> s390x/stsi.c | 23 ++------------------ >>>>> 4 files changed, 90 insertions(+), 23 deletions(-) >>>>> create mode 100644 lib/s390x/stsi.h >>>>> >>>>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h >>>>> new file mode 100644 >>>>> index 00000000..bebc492d >>>>> --- /dev/null >>>>> +++ b/lib/s390x/stsi.h >>>>> @@ -0,0 +1,32 @@ >>>>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>>>> +/* >>>>> + * Structures used to Store System Information >>>>> + * >>>>> + * Copyright IBM Corp. 2022 >>>>> + */ >>>>> + >>>>> +#ifndef _S390X_STSI_H_ >>>>> +#define _S390X_STSI_H_ >>>>> + >>>>> +struct sysinfo_3_2_2 { >>>>> + uint8_t reserved[31]; >>>>> + uint8_t count; >>>>> + struct { >>>>> + uint8_t reserved2[4]; >>>>> + uint16_t total_cpus; >>>>> + uint16_t conf_cpus; >>>>> + uint16_t standby_cpus; >>>>> + uint16_t reserved_cpus; >>>>> + uint8_t name[8]; >>>>> + uint32_t caf; >>>>> + uint8_t cpi[16]; >>>>> + uint8_t reserved5[3]; >>>>> + uint8_t ext_name_encoding; >>>>> + uint32_t reserved3; >>>>> + uint8_t uuid[16]; >>>>> + } vm[8]; >>>>> + uint8_t reserved4[1504]; >>>>> + uint8_t ext_names[8][256]; >>>>> +}; >>>>> + >>>>> +#endif /* _S390X_STSI_H_ */ >>>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >>>>> index a5b92863..91acd05b 100644 >>>>> --- a/lib/s390x/vm.c >>>>> +++ b/lib/s390x/vm.c >>>>> @@ -12,6 +12,7 @@ >>>>> #include <alloc_page.h> >>>>> #include <asm/arch_def.h> >>>>> #include "vm.h" >>>>> +#include "stsi.h" >>>>> >>>>> /** >>>>> * Detect whether we are running with TCG (instead of KVM) >>>>> @@ -26,9 +27,13 @@ bool vm_is_tcg(void) >>>>> if (initialized) >>>>> return is_tcg; >>>>> >>>>> + if (!vm_is_vm()) { >>>>> + initialized = true; >>>>> + return is_tcg; >>>>> + } >>>>> + >>>>> buf = alloc_page(); >>>>> - if (!buf) >>>>> - return false; >>>>> + assert(buf); >>>>> >>>>> if (stsi(buf, 1, 1, 1)) >>>>> goto out; >>>>> @@ -43,3 +48,49 @@ out: >>>>> free_page(buf); >>>>> return is_tcg; >>>>> } >>>>> + >>>>> +/** >>>>> + * Detect whether we are running with KVM >>>>> + */ >>>>> +bool vm_is_kvm(void) >>>>> +{ >>>>> + /* EBCDIC for "KVM/" */ >>>>> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>>> + static bool initialized; >>>>> + static bool is_kvm; >>>>> + struct sysinfo_3_2_2 *stsi_322; >>>>> + >>>>> + if (initialized) >>>>> + return is_kvm; >>>>> + >>>>> + if (!vm_is_vm() || vm_is_tcg()) { >>>>> + initialized = true; >>>>> + return is_kvm; >>>>> + } >>>>> + >>>>> + stsi_322 = alloc_page(); >>>>> + assert(stsi_322); >>>>> + >>>>> + if (stsi(stsi_322, 3, 2, 2)) >>>>> + goto out; >>>>> + >>>>> + /* >>>>> + * If the manufacturer string is "KVM/" in EBCDIC, then we >>>>> + * are on KVM. >>>>> + */ >>>>> + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, sizeof(kvm_ebcdic)); >>>>> + initialized = true; >>>>> +out: >>>>> + free_page(stsi_322); >>>>> + return is_kvm; >>>>> +} >>>>> + >>>>> +bool vm_is_lpar(void) >>>>> +{ >>>>> + return stsi_get_fc() == 2; >>>>> +} >>>>> + >>>>> +bool vm_is_vm(void) >>>>> +{ >>>>> + return stsi_get_fc() == 3; >>>>> +} >>>>> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h >>>>> index 7abba0cc..3aaf76af 100644 >>>>> --- a/lib/s390x/vm.h >>>>> +++ b/lib/s390x/vm.h >>>>> @@ -9,5 +9,8 @@ >>>>> #define _S390X_VM_H_ >>>>> >>>>> bool vm_is_tcg(void); >>>>> +bool vm_is_kvm(void); >>>>> +bool vm_is_vm(void); >>>>> +bool vm_is_lpar(void); >>>>> >>>>> #endif /* _S390X_VM_H_ */ >>>>> diff --git a/s390x/stsi.c b/s390x/stsi.c >>>>> index 391f8849..dccc53e7 100644 >>>>> --- a/s390x/stsi.c >>>>> +++ b/s390x/stsi.c >>>>> @@ -13,27 +13,8 @@ >>>>> #include <asm/asm-offsets.h> >>>>> #include <asm/interrupt.h> >>>>> #include <smp.h> >>>>> +#include <stsi.h> >>>>> >>>>> -struct stsi_322 { >>>>> - uint8_t reserved[31]; >>>>> - uint8_t count; >>>>> - struct { >>>>> - uint8_t reserved2[4]; >>>>> - uint16_t total_cpus; >>>>> - uint16_t conf_cpus; >>>>> - uint16_t standby_cpus; >>>>> - uint16_t reserved_cpus; >>>>> - uint8_t name[8]; >>>>> - uint32_t caf; >>>>> - uint8_t cpi[16]; >>>>> - uint8_t reserved5[3]; >>>>> - uint8_t ext_name_encoding; >>>>> - uint32_t reserved3; >>>>> - uint8_t uuid[16]; >>>>> - } vm[8]; >>>>> - uint8_t reserved4[1504]; >>>>> - uint8_t ext_names[8][256]; >>>>> -}; >>>>> static uint8_t pagebuf[PAGE_SIZE * 2] __attribute__((aligned(PAGE_SIZE * 2))); >>>>> >>>>> static void test_specs(void) >>>>> @@ -91,7 +72,7 @@ static void test_3_2_2(void) >>>>> /* EBCDIC for "KVM/" */ >>>>> const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>>> const char vm_name_ext[] = "kvm-unit-test"; >>>>> - struct stsi_322 *data = (void *)pagebuf; >>>>> + struct sysinfo_3_2_2 *data = (void *)pagebuf; >>>>> >>>>> report_prefix_push("3.2.2"); >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-16 8:13 ` Janosch Frank @ 2022-02-16 12:26 ` Pierre Morel 2022-02-16 12:55 ` Janosch Frank 0 siblings, 1 reply; 9+ messages in thread From: Pierre Morel @ 2022-02-16 12:26 UTC (permalink / raw) To: Janosch Frank, Claudio Imbrenda Cc: linux-s390, thuth, kvm, cohuck, david, nrb 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@linux.ibm.com> wrote: >>> >>>> On 2/15/22 13:06, Claudio Imbrenda wrote: >>>>> On Tue, 15 Feb 2022 11:46:32 +0100 >>>>> Pierre Morel <pmorel@linux.ibm.com> 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 ? > > 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. > >> >> Or do you want to implement a VSIE specificity ? >> >> >>>> >>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> >>>>>> --- >>>>>> lib/s390x/stsi.h | 32 ++++++++++++++++++++++++++++ >>>>>> lib/s390x/vm.c | 55 >>>>>> ++++++++++++++++++++++++++++++++++++++++++++++-- >>>>>> lib/s390x/vm.h | 3 +++ >>>>>> s390x/stsi.c | 23 ++------------------ >>>>>> 4 files changed, 90 insertions(+), 23 deletions(-) >>>>>> create mode 100644 lib/s390x/stsi.h >>>>>> >>>>>> diff --git a/lib/s390x/stsi.h b/lib/s390x/stsi.h >>>>>> new file mode 100644 >>>>>> index 00000000..bebc492d >>>>>> --- /dev/null >>>>>> +++ b/lib/s390x/stsi.h >>>>>> @@ -0,0 +1,32 @@ >>>>>> +/* SPDX-License-Identifier: GPL-2.0-only */ >>>>>> +/* >>>>>> + * Structures used to Store System Information >>>>>> + * >>>>>> + * Copyright IBM Corp. 2022 >>>>>> + */ >>>>>> + >>>>>> +#ifndef _S390X_STSI_H_ >>>>>> +#define _S390X_STSI_H_ >>>>>> + >>>>>> +struct sysinfo_3_2_2 { >>>>>> + uint8_t reserved[31]; >>>>>> + uint8_t count; >>>>>> + struct { >>>>>> + uint8_t reserved2[4]; >>>>>> + uint16_t total_cpus; >>>>>> + uint16_t conf_cpus; >>>>>> + uint16_t standby_cpus; >>>>>> + uint16_t reserved_cpus; >>>>>> + uint8_t name[8]; >>>>>> + uint32_t caf; >>>>>> + uint8_t cpi[16]; >>>>>> + uint8_t reserved5[3]; >>>>>> + uint8_t ext_name_encoding; >>>>>> + uint32_t reserved3; >>>>>> + uint8_t uuid[16]; >>>>>> + } vm[8]; >>>>>> + uint8_t reserved4[1504]; >>>>>> + uint8_t ext_names[8][256]; >>>>>> +}; >>>>>> + >>>>>> +#endif /* _S390X_STSI_H_ */ >>>>>> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >>>>>> index a5b92863..91acd05b 100644 >>>>>> --- a/lib/s390x/vm.c >>>>>> +++ b/lib/s390x/vm.c >>>>>> @@ -12,6 +12,7 @@ >>>>>> #include <alloc_page.h> >>>>>> #include <asm/arch_def.h> >>>>>> #include "vm.h" >>>>>> +#include "stsi.h" >>>>>> /** >>>>>> * Detect whether we are running with TCG (instead of KVM) >>>>>> @@ -26,9 +27,13 @@ bool vm_is_tcg(void) >>>>>> if (initialized) >>>>>> return is_tcg; >>>>>> + if (!vm_is_vm()) { >>>>>> + initialized = true; >>>>>> + return is_tcg; >>>>>> + } >>>>>> + >>>>>> buf = alloc_page(); >>>>>> - if (!buf) >>>>>> - return false; >>>>>> + assert(buf); >>>>>> if (stsi(buf, 1, 1, 1)) >>>>>> goto out; >>>>>> @@ -43,3 +48,49 @@ out: >>>>>> free_page(buf); >>>>>> return is_tcg; >>>>>> } >>>>>> + >>>>>> +/** >>>>>> + * Detect whether we are running with KVM >>>>>> + */ >>>>>> +bool vm_is_kvm(void) >>>>>> +{ >>>>>> + /* EBCDIC for "KVM/" */ >>>>>> + const uint8_t kvm_ebcdic[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>>>> + static bool initialized; >>>>>> + static bool is_kvm; >>>>>> + struct sysinfo_3_2_2 *stsi_322; >>>>>> + >>>>>> + if (initialized) >>>>>> + return is_kvm; >>>>>> + >>>>>> + if (!vm_is_vm() || vm_is_tcg()) { >>>>>> + initialized = true; >>>>>> + return is_kvm; >>>>>> + } >>>>>> + >>>>>> + stsi_322 = alloc_page(); >>>>>> + assert(stsi_322); >>>>>> + >>>>>> + if (stsi(stsi_322, 3, 2, 2)) >>>>>> + goto out; >>>>>> + >>>>>> + /* >>>>>> + * If the manufacturer string is "KVM/" in EBCDIC, then we >>>>>> + * are on KVM. >>>>>> + */ >>>>>> + is_kvm = !memcmp(&stsi_322->vm[0].cpi, kvm_ebcdic, >>>>>> sizeof(kvm_ebcdic)); >>>>>> + initialized = true; >>>>>> +out: >>>>>> + free_page(stsi_322); >>>>>> + return is_kvm; >>>>>> +} >>>>>> + >>>>>> +bool vm_is_lpar(void) >>>>>> +{ >>>>>> + return stsi_get_fc() == 2; >>>>>> +} >>>>>> + >>>>>> +bool vm_is_vm(void) >>>>>> +{ >>>>>> + return stsi_get_fc() == 3; >>>>>> +} >>>>>> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h >>>>>> index 7abba0cc..3aaf76af 100644 >>>>>> --- a/lib/s390x/vm.h >>>>>> +++ b/lib/s390x/vm.h >>>>>> @@ -9,5 +9,8 @@ >>>>>> #define _S390X_VM_H_ >>>>>> bool vm_is_tcg(void); >>>>>> +bool vm_is_kvm(void); >>>>>> +bool vm_is_vm(void); >>>>>> +bool vm_is_lpar(void); >>>>>> #endif /* _S390X_VM_H_ */ >>>>>> diff --git a/s390x/stsi.c b/s390x/stsi.c >>>>>> index 391f8849..dccc53e7 100644 >>>>>> --- a/s390x/stsi.c >>>>>> +++ b/s390x/stsi.c >>>>>> @@ -13,27 +13,8 @@ >>>>>> #include <asm/asm-offsets.h> >>>>>> #include <asm/interrupt.h> >>>>>> #include <smp.h> >>>>>> +#include <stsi.h> >>>>>> -struct stsi_322 { >>>>>> - uint8_t reserved[31]; >>>>>> - uint8_t count; >>>>>> - struct { >>>>>> - uint8_t reserved2[4]; >>>>>> - uint16_t total_cpus; >>>>>> - uint16_t conf_cpus; >>>>>> - uint16_t standby_cpus; >>>>>> - uint16_t reserved_cpus; >>>>>> - uint8_t name[8]; >>>>>> - uint32_t caf; >>>>>> - uint8_t cpi[16]; >>>>>> - uint8_t reserved5[3]; >>>>>> - uint8_t ext_name_encoding; >>>>>> - uint32_t reserved3; >>>>>> - uint8_t uuid[16]; >>>>>> - } vm[8]; >>>>>> - uint8_t reserved4[1504]; >>>>>> - uint8_t ext_names[8][256]; >>>>>> -}; >>>>>> static uint8_t pagebuf[PAGE_SIZE * 2] >>>>>> __attribute__((aligned(PAGE_SIZE * 2))); >>>>>> static void test_specs(void) >>>>>> @@ -91,7 +72,7 @@ static void test_3_2_2(void) >>>>>> /* EBCDIC for "KVM/" */ >>>>>> const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>>>>> const char vm_name_ext[] = "kvm-unit-test"; >>>>>> - struct stsi_322 *data = (void *)pagebuf; >>>>>> + struct sysinfo_3_2_2 *data = (void *)pagebuf; >>>>>> report_prefix_push("3.2.2"); >>>> >>> >> > -- Pierre Morel IBM Lab Boeblingen ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: stsi: Define vm_is_kvm to be used in different tests 2022-02-16 12:26 ` Pierre Morel @ 2022-02-16 12:55 ` Janosch Frank 0 siblings, 0 replies; 9+ messages in thread From: Janosch Frank @ 2022-02-16 12:55 UTC (permalink / raw) To: Pierre Morel, Claudio Imbrenda; +Cc: linux-s390, thuth, kvm, cohuck, david, nrb 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@linux.ibm.com> wrote: >>>> >>>>> On 2/15/22 13:06, Claudio Imbrenda wrote: >>>>>> On Tue, 15 Feb 2022 11:46:32 +0100 >>>>>> Pierre Morel <pmorel@linux.ibm.com> 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2022-02-16 12:55 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-02-15 10:46 [kvm-unit-tests PATCH 0/1] s390x: stsi: Define vm_is_kvm to be used in different tests Pierre Morel 2022-02-15 10:46 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel 2022-02-15 12:06 ` Claudio Imbrenda 2022-02-15 15:08 ` Janosch Frank 2022-02-15 15:21 ` Claudio Imbrenda 2022-02-15 17:30 ` Pierre Morel 2022-02-16 8:13 ` Janosch Frank 2022-02-16 12:26 ` Pierre Morel 2022-02-16 12:55 ` Janosch Frank
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox