| Message ID | 20200330122035.19607-1-frankja@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] s390x: Add stsi 3.2.2 tests | expand |
On 30.03.20 14:20, Janosch Frank wrote: > Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested > a bit more thorough. > > In this test we set a custom name and uuid through the QEMU command > line. Both parameters will be passed to the guest on a stsi subcode > 3.2.2 call and will then be checked. > > We also compare the total and configured cpu numbers against the smp > reported numbers. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 1 + > 2 files changed, 63 insertions(+) > > diff --git a/s390x/stsi.c b/s390x/stsi.c > index e9206bca137d2edb..10e588a78cc05186 100644 > --- a/s390x/stsi.c > +++ b/s390x/stsi.c > @@ -14,7 +14,28 @@ > #include <asm/page.h> > #include <asm/asm-offsets.h> > #include <asm/interrupt.h> > +#include <smp.h> > > +struct stsi_322 { > + uint8_t reserved[31]; > + uint8_t count; > + struct { > + uint8_t reserved2[4]; I dislike aligning the members using double-spaces ... > + 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]; ... e.g., here it's not aligned anymore. Just use single spaces. > + 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) > @@ -76,11 +97,52 @@ static void test_fc(void) > report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); > } > > +static void test_3_2_2(void) > +{ > + int rc; > + /* EBCDIC for "kvm-unit" */ > + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; > + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, > + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, > + 0x00, 0x03 }; > + /* EBCDIC for "KVM/" */ > + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; All of these can be const. > + const char *vm_name_ext = "kvm-unit-test"; > + struct stsi_322 *data = (void *)pagebuf; > + > + /* Is the function code available at all? */ > + if (stsi_get_fc(pagebuf) < 3) Maybe report_skip() ? > + return; > + > + report_prefix_push("3.2.2"); > + rc = stsi(pagebuf, 3, 2, 2); > + report(!rc, "call"); > + > + /* For now we concentrate on KVM/QEMU */ > + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) Maybe report_skip() ? > + goto out; > + > + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); > + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); > + report(data->vm[0].standby_cpus == 0, "cpu # standby"); > + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved CPUs. Also passes under TCG, nice :)
On 30.03.20 14:20, Janosch Frank wrote: > Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested > a bit more thorough. > > In this test we set a custom name and uuid through the QEMU command > line. Both parameters will be passed to the guest on a stsi subcode > 3.2.2 call and will then be checked. > > We also compare the total and configured cpu numbers against the smp > reported numbers. > > Signed-off-by: Janosch Frank <frankja@linux.ibm.com> > --- > s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ > s390x/unittests.cfg | 1 + > 2 files changed, 63 insertions(+) > > diff --git a/s390x/stsi.c b/s390x/stsi.c > index e9206bca137d2edb..10e588a78cc05186 100644 > --- a/s390x/stsi.c > +++ b/s390x/stsi.c > @@ -14,7 +14,28 @@ > #include <asm/page.h> > #include <asm/asm-offsets.h> > #include <asm/interrupt.h> > +#include <smp.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) > @@ -76,11 +97,52 @@ static void test_fc(void) > report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); > } > > +static void test_3_2_2(void) > +{ > + int rc; > + /* EBCDIC for "kvm-unit" */ > + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; > + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, > + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, > + 0x00, 0x03 }; > + /* EBCDIC for "KVM/" */ > + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > + const char *vm_name_ext = "kvm-unit-test"; > + struct stsi_322 *data = (void *)pagebuf; > + > + /* Is the function code available at all? */ > + if (stsi_get_fc(pagebuf) < 3) > + return; > + > + report_prefix_push("3.2.2"); > + rc = stsi(pagebuf, 3, 2, 2); > + report(!rc, "call"); > + > + /* For now we concentrate on KVM/QEMU */ > + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) > + goto out; > + > + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); > + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); > + report(data->vm[0].standby_cpus == 0, "cpu # standby"); > + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); > + report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)), > + "VM name == kvm-unit-test"); > + report(data->vm[0].ext_name_encoding == 2, "ext name encoding UTF-8"); should you rather do if (data->vm[0].ext_name_encoding == 2) { ... } else { report_skip(...); } to make this future-proof?
On 3/30/20 2:51 PM, David Hildenbrand wrote: > On 30.03.20 14:20, Janosch Frank wrote: >> Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested >> a bit more thorough. >> >> In this test we set a custom name and uuid through the QEMU command >> line. Both parameters will be passed to the guest on a stsi subcode >> 3.2.2 call and will then be checked. >> >> We also compare the total and configured cpu numbers against the smp >> reported numbers. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 1 + >> 2 files changed, 63 insertions(+) >> >> diff --git a/s390x/stsi.c b/s390x/stsi.c >> index e9206bca137d2edb..10e588a78cc05186 100644 >> --- a/s390x/stsi.c >> +++ b/s390x/stsi.c >> @@ -14,7 +14,28 @@ >> #include <asm/page.h> >> #include <asm/asm-offsets.h> >> #include <asm/interrupt.h> >> +#include <smp.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) >> @@ -76,11 +97,52 @@ static void test_fc(void) >> report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); >> } >> >> +static void test_3_2_2(void) >> +{ >> + int rc; >> + /* EBCDIC for "kvm-unit" */ >> + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; >> + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, >> + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, >> + 0x00, 0x03 }; >> + /* EBCDIC for "KVM/" */ >> + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >> + const char *vm_name_ext = "kvm-unit-test"; >> + struct stsi_322 *data = (void *)pagebuf; >> + >> + /* Is the function code available at all? */ >> + if (stsi_get_fc(pagebuf) < 3) >> + return; >> + >> + report_prefix_push("3.2.2"); >> + rc = stsi(pagebuf, 3, 2, 2); >> + report(!rc, "call"); >> + >> + /* For now we concentrate on KVM/QEMU */ >> + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) >> + goto out; >> + >> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >> + report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)), >> + "VM name == kvm-unit-test"); >> + report(data->vm[0].ext_name_encoding == 2, "ext name encoding UTF-8"); > > should you rather do > > if (data->vm[0].ext_name_encoding == 2) { > ... > } else { > report_skip(...); > } > > to make this future-proof? > Do you expect UTF-16 or EBCDIC in the future? :)
On 3/30/20 2:50 PM, David Hildenbrand wrote: > On 30.03.20 14:20, Janosch Frank wrote: >> Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested >> a bit more thorough. >> >> In this test we set a custom name and uuid through the QEMU command >> line. Both parameters will be passed to the guest on a stsi subcode >> 3.2.2 call and will then be checked. >> >> We also compare the total and configured cpu numbers against the smp >> reported numbers. >> >> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >> --- >> s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ >> s390x/unittests.cfg | 1 + >> 2 files changed, 63 insertions(+) >> >> diff --git a/s390x/stsi.c b/s390x/stsi.c >> index e9206bca137d2edb..10e588a78cc05186 100644 >> --- a/s390x/stsi.c >> +++ b/s390x/stsi.c >> @@ -14,7 +14,28 @@ >> #include <asm/page.h> >> #include <asm/asm-offsets.h> >> #include <asm/interrupt.h> >> +#include <smp.h> >> >> +struct stsi_322 { >> + uint8_t reserved[31]; >> + uint8_t count; >> + struct { >> + uint8_t reserved2[4]; > > I dislike aligning the members using double-spaces ... Time to fix target/s390x/cpu.h then :) > >> + 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]; > > ... e.g., here it's not aligned anymore. Just use single spaces. > >> + 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) >> @@ -76,11 +97,52 @@ static void test_fc(void) >> report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); >> } >> >> +static void test_3_2_2(void) >> +{ >> + int rc; >> + /* EBCDIC for "kvm-unit" */ >> + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; >> + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, >> + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, >> + 0x00, 0x03 }; >> + /* EBCDIC for "KVM/" */ >> + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; > > All of these can be const. > >> + const char *vm_name_ext = "kvm-unit-test"; >> + struct stsi_322 *data = (void *)pagebuf; >> + >> + /* Is the function code available at all? */ >> + if (stsi_get_fc(pagebuf) < 3) > > Maybe report_skip() ? Ack > >> + return; >> + >> + report_prefix_push("3.2.2"); >> + rc = stsi(pagebuf, 3, 2, 2); >> + report(!rc, "call"); >> + >> + /* For now we concentrate on KVM/QEMU */ >> + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) > > Maybe report_skip() ? Ack > >> + goto out; >> + >> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); > > IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved > CPUs. Will try that > > > Also passes under TCG, nice :) >
On 3/30/20 3:03 PM, Janosch Frank wrote: > On 3/30/20 2:50 PM, David Hildenbrand wrote: >> On 30.03.20 14:20, Janosch Frank wrote: >>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >> >> IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved >> CPUs. > > Will try that Just like I thought, QEMU does not manipulate cpu counts and KVM pre-sets standby and reserved to 0. So we have absolutely no change when adding the smp parameter. > >> >> >> Also passes under TCG, nice :) >> > >
On 30.03.20 15:00, Janosch Frank wrote: > On 3/30/20 2:51 PM, David Hildenbrand wrote: >> On 30.03.20 14:20, Janosch Frank wrote: >>> Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested >>> a bit more thorough. >>> >>> In this test we set a custom name and uuid through the QEMU command >>> line. Both parameters will be passed to the guest on a stsi subcode >>> 3.2.2 call and will then be checked. >>> >>> We also compare the total and configured cpu numbers against the smp >>> reported numbers. >>> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> >>> --- >>> s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ >>> s390x/unittests.cfg | 1 + >>> 2 files changed, 63 insertions(+) >>> >>> diff --git a/s390x/stsi.c b/s390x/stsi.c >>> index e9206bca137d2edb..10e588a78cc05186 100644 >>> --- a/s390x/stsi.c >>> +++ b/s390x/stsi.c >>> @@ -14,7 +14,28 @@ >>> #include <asm/page.h> >>> #include <asm/asm-offsets.h> >>> #include <asm/interrupt.h> >>> +#include <smp.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) >>> @@ -76,11 +97,52 @@ static void test_fc(void) >>> report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); >>> } >>> >>> +static void test_3_2_2(void) >>> +{ >>> + int rc; >>> + /* EBCDIC for "kvm-unit" */ >>> + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; >>> + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, >>> + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, >>> + 0x00, 0x03 }; >>> + /* EBCDIC for "KVM/" */ >>> + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; >>> + const char *vm_name_ext = "kvm-unit-test"; >>> + struct stsi_322 *data = (void *)pagebuf; >>> + >>> + /* Is the function code available at all? */ >>> + if (stsi_get_fc(pagebuf) < 3) >>> + return; >>> + >>> + report_prefix_push("3.2.2"); >>> + rc = stsi(pagebuf, 3, 2, 2); >>> + report(!rc, "call"); >>> + >>> + /* For now we concentrate on KVM/QEMU */ >>> + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) >>> + goto out; >>> + >>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >>> + report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)), >>> + "VM name == kvm-unit-test"); >>> + report(data->vm[0].ext_name_encoding == 2, "ext name encoding UTF-8"); >> >> should you rather do >> >> if (data->vm[0].ext_name_encoding == 2) { >> ... >> } else { >> report_skip(...); >> } >> >> to make this future-proof? >> > Do you expect UTF-16 or EBCDIC in the future? :) Well, you never know :) If there is an option for other encodings ...
On 30.03.20 15:09, Janosch Frank wrote: > On 3/30/20 3:03 PM, Janosch Frank wrote: >> On 3/30/20 2:50 PM, David Hildenbrand wrote: >>> On 30.03.20 14:20, Janosch Frank wrote: >>>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >>> >>> IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved >>> CPUs. >> >> Will try that > > Just like I thought, QEMU does not manipulate cpu counts and KVM > pre-sets standby and reserved to 0. So we have absolutely no change when > adding the smp parameter. Well, for TCG it is properly implemented. Is this a BUG in KVM's STSI code?
On 3/30/20 3:15 PM, David Hildenbrand wrote: > On 30.03.20 15:09, Janosch Frank wrote: >> On 3/30/20 3:03 PM, Janosch Frank wrote: >>> On 3/30/20 2:50 PM, David Hildenbrand wrote: >>>> On 30.03.20 14:20, Janosch Frank wrote: >>>>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>>>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>>>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>>>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >>>> >>>> IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved >>>> CPUs. >>> >>> Will try that >> >> Just like I thought, QEMU does not manipulate cpu counts and KVM >> pre-sets standby and reserved to 0. So we have absolutely no change when >> adding the smp parameter. > > Well, for TCG it is properly implemented. Is this a BUG in KVM's STSI code? > KVM tracks online cpus and created cpus, but only reports the online ones in stsi. Will QEMU register/create a reserved CPU with KVM? To fix this we could also fix-up the cpu reporting in QEMU after KVM wrote its results. @Christian: Guidance?
On 30.03.20 15:30, Janosch Frank wrote: > On 3/30/20 3:15 PM, David Hildenbrand wrote: >> On 30.03.20 15:09, Janosch Frank wrote: >>> On 3/30/20 3:03 PM, Janosch Frank wrote: >>>> On 3/30/20 2:50 PM, David Hildenbrand wrote: >>>>> On 30.03.20 14:20, Janosch Frank wrote: >>>>>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>>>>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>>>>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>>>>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >>>>> >>>>> IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved >>>>> CPUs. >>>> >>>> Will try that >>> >>> Just like I thought, QEMU does not manipulate cpu counts and KVM >>> pre-sets standby and reserved to 0. So we have absolutely no change when >>> adding the smp parameter. >> >> Well, for TCG it is properly implemented. Is this a BUG in KVM's STSI code? >> > > KVM tracks online cpus and created cpus, but only reports the online > ones in stsi. > Will QEMU register/create a reserved CPU with KVM? > > To fix this we could also fix-up the cpu reporting in QEMU after KVM > wrote its results. I think that would be preferred, and handling it similar to the TCG implementation > > @Christian: Guidance? >
On 30.03.20 15:30, Janosch Frank wrote: > On 3/30/20 3:15 PM, David Hildenbrand wrote: >> On 30.03.20 15:09, Janosch Frank wrote: >>> On 3/30/20 3:03 PM, Janosch Frank wrote: >>>> On 3/30/20 2:50 PM, David Hildenbrand wrote: >>>>> On 30.03.20 14:20, Janosch Frank wrote: >>>>>> + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); >>>>>> + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); >>>>>> + report(data->vm[0].standby_cpus == 0, "cpu # standby"); >>>>>> + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); >>>>> >>>>> IIRC, using -smp 1,maxcpus=X, you could also test the reported reserved >>>>> CPUs. >>>> >>>> Will try that >>> >>> Just like I thought, QEMU does not manipulate cpu counts and KVM >>> pre-sets standby and reserved to 0. So we have absolutely no change when >>> adding the smp parameter. >> >> Well, for TCG it is properly implemented. Is this a BUG in KVM's STSI code? >> > > KVM tracks online cpus and created cpus, but only reports the online > ones in stsi. > Will QEMU register/create a reserved CPU with KVM? > > To fix this we could also fix-up the cpu reporting in QEMU after KVM > wrote its results. > > @Christian: Guidance? The standby only make sense if we implement the sclp cpu configure things, which we do not. So this can be considered reserved, while standby must be 0. I think we could implement this in QEMUs insert_stsi_3_2_2 function in kvm.c
diff --git a/s390x/stsi.c b/s390x/stsi.c index e9206bca137d2edb..10e588a78cc05186 100644 --- a/s390x/stsi.c +++ b/s390x/stsi.c @@ -14,7 +14,28 @@ #include <asm/page.h> #include <asm/asm-offsets.h> #include <asm/interrupt.h> +#include <smp.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) @@ -76,11 +97,52 @@ static void test_fc(void) report(stsi_get_fc(pagebuf) >= 2, "query fc >= 2"); } +static void test_3_2_2(void) +{ + int rc; + /* EBCDIC for "kvm-unit" */ + uint8_t vm_name[] = { 0x92, 0xa5, 0x94, 0x60, 0xa4, 0x95, 0x89, 0xa3 }; + uint8_t uuid[] = { 0x0f, 0xb8, 0x4a, 0x86, 0x72, 0x7c, + 0x11, 0xea, 0xbc, 0x55, 0x02, 0x42, 0xac, 0x13, + 0x00, 0x03 }; + /* EBCDIC for "KVM/" */ + uint8_t cpi_kvm[] = { 0xd2, 0xe5, 0xd4, 0x61 }; + const char *vm_name_ext = "kvm-unit-test"; + struct stsi_322 *data = (void *)pagebuf; + + /* Is the function code available at all? */ + if (stsi_get_fc(pagebuf) < 3) + return; + + report_prefix_push("3.2.2"); + rc = stsi(pagebuf, 3, 2, 2); + report(!rc, "call"); + + /* For now we concentrate on KVM/QEMU */ + if (memcmp(&data->vm[0].cpi, cpi_kvm, sizeof(cpi_kvm))) + goto out; + + report(data->vm[0].total_cpus == smp_query_num_cpus(), "cpu # total"); + report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured"); + report(data->vm[0].standby_cpus == 0, "cpu # standby"); + report(data->vm[0].reserved_cpus == 0, "cpu # reserved"); + report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)), + "VM name == kvm-unit-test"); + report(data->vm[0].ext_name_encoding == 2, "ext name encoding UTF-8"); + report(!memcmp(data->ext_names[0], vm_name_ext, sizeof(vm_name_ext)), + "ext VM name == kvm-unit-test"); + report(!memcmp(data->vm[0].uuid, uuid, sizeof(uuid)), "uuid"); + +out: + report_prefix_pop(); +} + int main(void) { report_prefix_push("stsi"); test_priv(); test_specs(); test_fc(); + test_3_2_2(); return report_summary(); } diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index 12d46c5b402328bb..16e876344e1957ec 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -71,6 +71,7 @@ extra_params=-device diag288,id=watchdog0 --watchdog-action inject-nmi [stsi] file = stsi.elf +extra_params=-name kvm-unit-test --uuid 0fb84a86-727c-11ea-bc55-0242ac130003 [smp] file = smp.elf
Subcode 3.2.2 is handled by KVM/QEMU and should therefore be tested a bit more thorough. In this test we set a custom name and uuid through the QEMU command line. Both parameters will be passed to the guest on a stsi subcode 3.2.2 call and will then be checked. We also compare the total and configured cpu numbers against the smp reported numbers. Signed-off-by: Janosch Frank <frankja@linux.ibm.com> --- s390x/stsi.c | 62 +++++++++++++++++++++++++++++++++++++++++++++ s390x/unittests.cfg | 1 + 2 files changed, 63 insertions(+)