| Message ID | 20220114100245.8643-2-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390x: Allocation and hosting environment detection fixes | expand |
On Fri, 14 Jan 2022 10:02:41 +0000 Janosch Frank <frankja@linux.ibm.com> wrote: > This patch will likely (in parts) be replaced by Pierre's patch from > his topology test series. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> although I think there is some room for improvement, but nothing too serious, I'll probably fix it myself later > --- > lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++ > lib/s390x/vm.h | 23 +++++++++++++++++++++++ > s390x/stsi.c | 21 +-------------------- > 3 files changed, 63 insertions(+), 20 deletions(-) > > diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c > index a5b92863..266a81c1 100644 > --- a/lib/s390x/vm.c > +++ b/lib/s390x/vm.c > @@ -26,6 +26,11 @@ bool vm_is_tcg(void) > if (initialized) > return is_tcg; > > + if (stsi_get_fc() < 3) { > + initialized = true; > + return false; > + } > + > buf = alloc_page(); > if (!buf) > return false; > @@ -43,3 +48,37 @@ out: > free_page(buf); > return is_tcg; > } > + > +bool vm_is_kvm(void) > +{ > + const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > + struct stsi_322 *buf; > + static bool initialized = false; > + static bool is_kvm = false; > + > + if (initialized) > + return is_kvm; > + > + if (stsi_get_fc() < 3) { > + initialized = true; > + return false; > + } > + > + buf = alloc_page(); > + if (!buf) > + return false; > + > + if (stsi(buf, 3, 2, 2)) > + goto out; > + > + is_kvm = !(memcmp(&buf->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) && !vm_is_tcg(); > + initialized = true; > +out: > + free_page(buf); > + return is_kvm; > +} > + > +bool vm_is_lpar(void) > +{ > + return stsi_get_fc() == 1; > +} > diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h > index 7abba0cc..d4a82fc0 100644 > --- a/lib/s390x/vm.h > +++ b/lib/s390x/vm.h > @@ -8,6 +8,29 @@ > #ifndef _S390X_VM_H_ > #define _S390X_VM_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]; > +}; > + > bool vm_is_tcg(void); > +bool vm_is_kvm(void); > +bool vm_is_lpar(void); > > #endif /* _S390X_VM_H_ */ > diff --git a/s390x/stsi.c b/s390x/stsi.c > index 391f8849..e66d07a1 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 <vm.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)
On 1/14/22 12:18, Claudio Imbrenda wrote: > On Fri, 14 Jan 2022 10:02:41 +0000 > Janosch Frank <frankja@linux.ibm.com> wrote: > >> This patch will likely (in parts) be replaced by Pierre's patch from >> his topology test series. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > although I think there is some room for improvement, but nothing too > serious, I'll probably fix it myself later I think I should add this to clean up the checks in the following patches: bool vm_is_qemu(void) { return vm_is_kvm() || vm_is_tcg(); } That of course also isn't a 100% correct, we could have KVM + non-QEMU but I'll cross that bridge when I get there. And as I already mentioned in Pierre's patch, the STSI values are a bit of a mystery to me since AFAIK we store KVM/Linux even if we run under TCG. > >> --- >> lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++ >> lib/s390x/vm.h | 23 +++++++++++++++++++++++ >> s390x/stsi.c | 21 +-------------------- >> 3 files changed, 63 insertions(+), 20 deletions(-) >> >> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >> index a5b92863..266a81c1 100644 >> --- a/lib/s390x/vm.c >> +++ b/lib/s390x/vm.c >> @@ -26,6 +26,11 @@ bool vm_is_tcg(void) >> if (initialized) >> return is_tcg; >> >> + if (stsi_get_fc() < 3) { >> + initialized = true; >> + return false; >> + } >> + >> buf = alloc_page(); >> if (!buf) >> return false; >> @@ -43,3 +48,37 @@ out: >> free_page(buf); >> return is_tcg; >> } >> + >> +bool vm_is_kvm(void) >> +{ >> + const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >> + struct stsi_322 *buf; >> + static bool initialized = false; >> + static bool is_kvm = false; >> + >> + if (initialized) >> + return is_kvm; >> + >> + if (stsi_get_fc() < 3) { >> + initialized = true; >> + return false; >> + } >> + >> + buf = alloc_page(); >> + if (!buf) >> + return false; >> + >> + if (stsi(buf, 3, 2, 2)) >> + goto out; >> + >> + is_kvm = !(memcmp(&buf->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) && !vm_is_tcg(); >> + initialized = true; >> +out: >> + free_page(buf); >> + return is_kvm; >> +} >> + >> +bool vm_is_lpar(void) >> +{ >> + return stsi_get_fc() == 1; >> +} >> diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h >> index 7abba0cc..d4a82fc0 100644 >> --- a/lib/s390x/vm.h >> +++ b/lib/s390x/vm.h >> @@ -8,6 +8,29 @@ >> #ifndef _S390X_VM_H_ >> #define _S390X_VM_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]; >> +}; >> + >> bool vm_is_tcg(void); >> +bool vm_is_kvm(void); >> +bool vm_is_lpar(void); >> >> #endif /* _S390X_VM_H_ */ >> diff --git a/s390x/stsi.c b/s390x/stsi.c >> index 391f8849..e66d07a1 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 <vm.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) >
On Fri, 14 Jan 2022 13:28:19 +0100 Janosch Frank <frankja@linux.ibm.com> wrote: > On 1/14/22 12:18, Claudio Imbrenda wrote: > > On Fri, 14 Jan 2022 10:02:41 +0000 > > Janosch Frank <frankja@linux.ibm.com> wrote: > > > >> This patch will likely (in parts) be replaced by Pierre's patch from > >> his topology test series. > >> > >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > > > > Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > > > > although I think there is some room for improvement, but nothing too > > serious, I'll probably fix it myself later > > I think I should add this to clean up the checks in the following patches: > > bool vm_is_qemu(void) > { > return vm_is_kvm() || vm_is_tcg(); > } > > That of course also isn't a 100% correct, we could have KVM + non-QEMU > but I'll cross that bridge when I get there. > > And as I already mentioned in Pierre's patch, the STSI values are a bit > of a mystery to me since AFAIK we store KVM/Linux even if we run under TCG. I was actually thinking of a generic detection function that will call STSI only once and set all the necessary flags. then the various vm_is_* functions can become something like this: bool vm_is_${TYPE}() { if (vm_type == VM_TYPE_NOT_INITIALIZED) do_vm_detection(); return vm_type == VM_${TYPE}; } and then have all the magic hidden in the one do_vm_detection, which will call STSI once and try to find out what we are running on.
On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: > This patch will likely (in parts) be replaced by Pierre's patch from > his topology test series. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++ > lib/s390x/vm.h | 23 +++++++++++++++++++++++ > s390x/stsi.c | 21 +-------------------- > 3 files changed, 63 insertions(+), 20 deletions(-) > > diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c > index a5b92863..266a81c1 100644 > --- a/lib/s390x/vm.c > +++ b/lib/s390x/vm.c > @@ -26,6 +26,11 @@ bool vm_is_tcg(void) > if (initialized) > return is_tcg; > > + if (stsi_get_fc() < 3) { > + initialized = true; > + return false; Minor nit: By setting initialized to true, you rely on the previous initialization of is_tcg to false for subsequent calls. You could make this more obvious by saying: return is_tcg;
On 1/14/22 14:27, Nico Boehr wrote: > On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote: >> This patch will likely (in parts) be replaced by Pierre's patch from >> his topology test series. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++ >> lib/s390x/vm.h | 23 +++++++++++++++++++++++ >> s390x/stsi.c | 21 +-------------------- >> 3 files changed, 63 insertions(+), 20 deletions(-) >> >> diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c >> index a5b92863..266a81c1 100644 >> --- a/lib/s390x/vm.c >> +++ b/lib/s390x/vm.c >> @@ -26,6 +26,11 @@ bool vm_is_tcg(void) >> if (initialized) >> return is_tcg; >> >> + if (stsi_get_fc() < 3) { >> + initialized = true; >> + return false; > > Minor nit: By setting initialized to true, you rely on the previous > initialization of is_tcg to false for subsequent calls. > > You could make this more obvious by saying: > > return is_tcg; > Have a look at Pierre's patch which I will be relying on when it's done. As I said in the commit message, this is only a placeholder for his patch.
On Fri, 2022-01-14 at 14:35 +0100, Janosch Frank wrote: > Have a look at Pierre's patch which I will be relying on when it's > done. > As I said in the commit message, this is only a placeholder for his > patch. There it is just as I suggested. Thanks.
diff --git a/lib/s390x/vm.c b/lib/s390x/vm.c index a5b92863..266a81c1 100644 --- a/lib/s390x/vm.c +++ b/lib/s390x/vm.c @@ -26,6 +26,11 @@ bool vm_is_tcg(void) if (initialized) return is_tcg; + if (stsi_get_fc() < 3) { + initialized = true; + return false; + } + buf = alloc_page(); if (!buf) return false; @@ -43,3 +48,37 @@ out: free_page(buf); return is_tcg; } + +bool vm_is_kvm(void) +{ + const uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; + struct stsi_322 *buf; + static bool initialized = false; + static bool is_kvm = false; + + if (initialized) + return is_kvm; + + if (stsi_get_fc() < 3) { + initialized = true; + return false; + } + + buf = alloc_page(); + if (!buf) + return false; + + if (stsi(buf, 3, 2, 2)) + goto out; + + is_kvm = !(memcmp(&buf->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) && !vm_is_tcg(); + initialized = true; +out: + free_page(buf); + return is_kvm; +} + +bool vm_is_lpar(void) +{ + return stsi_get_fc() == 1; +} diff --git a/lib/s390x/vm.h b/lib/s390x/vm.h index 7abba0cc..d4a82fc0 100644 --- a/lib/s390x/vm.h +++ b/lib/s390x/vm.h @@ -8,6 +8,29 @@ #ifndef _S390X_VM_H_ #define _S390X_VM_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]; +}; + bool vm_is_tcg(void); +bool vm_is_kvm(void); +bool vm_is_lpar(void); #endif /* _S390X_VM_H_ */ diff --git a/s390x/stsi.c b/s390x/stsi.c index 391f8849..e66d07a1 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 <vm.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)
This patch will likely (in parts) be replaced by Pierre's patch from his topology test series. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++ lib/s390x/vm.h | 23 +++++++++++++++++++++++ s390x/stsi.c | 21 +-------------------- 3 files changed, 63 insertions(+), 20 deletions(-)