* [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
@ 2022-01-14 10:02 ` Janosch Frank
2022-01-14 11:18 ` Claudio Imbrenda
2022-01-14 13:27 ` Nico Boehr
2022-01-14 10:02 ` [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu Janosch Frank
` (4 subsequent siblings)
5 siblings, 2 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 10:02 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck, nrb
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;
+ }
+
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)
--
2.32.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 10:02 ` [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries Janosch Frank
@ 2022-01-14 11:18 ` Claudio Imbrenda
2022-01-14 12:28 ` Janosch Frank
2022-01-14 13:27 ` Nico Boehr
1 sibling, 1 reply; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 11:18 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
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)
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 11:18 ` Claudio Imbrenda
@ 2022-01-14 12:28 ` Janosch Frank
2022-01-14 12:55 ` Claudio Imbrenda
0 siblings, 1 reply; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 12:28 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
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)
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 12:28 ` Janosch Frank
@ 2022-01-14 12:55 ` Claudio Imbrenda
0 siblings, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 12:55 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 10:02 ` [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries Janosch Frank
2022-01-14 11:18 ` Claudio Imbrenda
@ 2022-01-14 13:27 ` Nico Boehr
2022-01-14 13:35 ` Janosch Frank
1 sibling, 1 reply; 29+ messages in thread
From: Nico Boehr @ 2022-01-14 13:27 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
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;
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 13:27 ` Nico Boehr
@ 2022-01-14 13:35 ` Janosch Frank
2022-01-14 13:43 ` Nico Boehr
0 siblings, 1 reply; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 13:35 UTC (permalink / raw)
To: Nico Boehr, kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries
2022-01-14 13:35 ` Janosch Frank
@ 2022-01-14 13:43 ` Nico Boehr
0 siblings, 0 replies; 29+ messages in thread
From: Nico Boehr @ 2022-01-14 13:43 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
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.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
2022-01-14 10:02 ` [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries Janosch Frank
@ 2022-01-14 10:02 ` Janosch Frank
2022-01-14 10:41 ` Claudio Imbrenda
2022-01-17 7:01 ` Thomas Huth
2022-01-14 10:02 ` [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU Janosch Frank
` (3 subsequent siblings)
5 siblings, 2 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 10:02 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck, nrb
There's no guarantee that we even find a device at the address we're
testing for if we're not running under QEMU.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/css.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/s390x/css.c b/s390x/css.c
index 881206ba..c24119b4 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -15,6 +15,7 @@
#include <interrupt.h>
#include <asm/arch_def.h>
#include <alloc_page.h>
+#include <vm.h>
#include <malloc_io.h>
#include <css.h>
@@ -350,6 +351,12 @@ int main(int argc, char *argv[])
{
int i;
+ /* There's no guarantee where our devices are without qemu */
+ if (!vm_is_kvm() && !vm_is_tcg()) {
+ report_skip("Not running under QEMU");
+ goto done;
+ }
+
report_prefix_push("Channel Subsystem");
enable_io_isc(0x80 >> IO_SCH_ISC);
for (i = 0; tests[i].name; i++) {
@@ -357,7 +364,8 @@ int main(int argc, char *argv[])
tests[i].func();
report_prefix_pop();
}
- report_prefix_pop();
+done:
+ report_prefix_pop();
return report_summary();
}
--
2.32.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu
2022-01-14 10:02 ` [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu Janosch Frank
@ 2022-01-14 10:41 ` Claudio Imbrenda
2022-01-17 7:01 ` Thomas Huth
1 sibling, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 10:41 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On Fri, 14 Jan 2022 10:02:42 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> There's no guarantee that we even find a device at the address we're
> testing for if we're not running under QEMU.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/css.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/css.c b/s390x/css.c
> index 881206ba..c24119b4 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -15,6 +15,7 @@
> #include <interrupt.h>
> #include <asm/arch_def.h>
> #include <alloc_page.h>
> +#include <vm.h>
>
> #include <malloc_io.h>
> #include <css.h>
> @@ -350,6 +351,12 @@ int main(int argc, char *argv[])
> {
> int i;
>
> + /* There's no guarantee where our devices are without qemu */
> + if (!vm_is_kvm() && !vm_is_tcg()) {
> + report_skip("Not running under QEMU");
> + goto done;
> + }
> +
> report_prefix_push("Channel Subsystem");
> enable_io_isc(0x80 >> IO_SCH_ISC);
> for (i = 0; tests[i].name; i++) {
> @@ -357,7 +364,8 @@ int main(int argc, char *argv[])
> tests[i].func();
> report_prefix_pop();
> }
> - report_prefix_pop();
>
> +done:
> + report_prefix_pop();
> return report_summary();
> }
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu
2022-01-14 10:02 ` [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu Janosch Frank
2022-01-14 10:41 ` Claudio Imbrenda
@ 2022-01-17 7:01 ` Thomas Huth
1 sibling, 0 replies; 29+ messages in thread
From: Thomas Huth @ 2022-01-17 7:01 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck, nrb
On 14/01/2022 11.02, Janosch Frank wrote:
> There's no guarantee that we even find a device at the address we're
> testing for if we're not running under QEMU.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/css.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/css.c b/s390x/css.c
> index 881206ba..c24119b4 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -15,6 +15,7 @@
> #include <interrupt.h>
> #include <asm/arch_def.h>
> #include <alloc_page.h>
> +#include <vm.h>
>
> #include <malloc_io.h>
> #include <css.h>
> @@ -350,6 +351,12 @@ int main(int argc, char *argv[])
> {
> int i;
>
> + /* There's no guarantee where our devices are without qemu */
> + if (!vm_is_kvm() && !vm_is_tcg()) {
> + report_skip("Not running under QEMU");
> + goto done;
> + }
You've added the check before the report_prefix_push() ...
> report_prefix_push("Channel Subsystem");
> enable_io_isc(0x80 >> IO_SCH_ISC);
> for (i = 0; tests[i].name; i++) {
> @@ -357,7 +364,8 @@ int main(int argc, char *argv[])
> tests[i].func();
> report_prefix_pop();
> }
> - report_prefix_pop();
>
> +done:
> + report_prefix_pop();
... but in case of the goto you now do the pop without the push. I think you
have to drop the second hunk.
Thomas
> return report_summary();
> }
^ permalink raw reply [flat|nested] 29+ messages in thread
* [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
2022-01-14 10:02 ` [kvm-unit-tests PATCH 1/5] lib: s390x: vm: Add kvm and lpar vm queries Janosch Frank
2022-01-14 10:02 ` [kvm-unit-tests PATCH 2/5] s390x: css: Skip if we're not run by qemu Janosch Frank
@ 2022-01-14 10:02 ` Janosch Frank
2022-01-14 10:39 ` Claudio Imbrenda
2022-01-17 7:04 ` Thomas Huth
2022-01-14 10:02 ` [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space Janosch Frank
` (2 subsequent siblings)
5 siblings, 2 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 10:02 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck, nrb
Other hypervisors might implement it and therefore not send a
specification exception.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/diag308.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/s390x/diag308.c b/s390x/diag308.c
index c9d6c499..414dbdf4 100644
--- a/s390x/diag308.c
+++ b/s390x/diag308.c
@@ -8,6 +8,7 @@
#include <libcflat.h>
#include <asm/asm-offsets.h>
#include <asm/interrupt.h>
+#include <vm.h>
/* The diagnose calls should be blocked in problem state */
static void test_priv(void)
@@ -75,7 +76,7 @@ static void test_subcode6(void)
/* Unsupported subcodes should generate a specification exception */
static void test_unsupported_subcode(void)
{
- int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
+ int subcodes[] = { 0x101, 0xffff, 0x10001, -1 };
int idx;
for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
@@ -85,6 +86,18 @@ static void test_unsupported_subcode(void)
check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
report_prefix_pop();
}
+
+ /*
+ * Subcode 2 is not available under QEMU but might be on other
+ * hypervisors.
+ */
+ if (vm_is_tcg() || vm_is_kvm()) {
+ report_prefix_pushf("0x%04x", 2);
+ expect_pgm_int();
+ asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
+ check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
+ report_prefix_pop();
+ }
}
static struct {
--
2.32.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU
2022-01-14 10:02 ` [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU Janosch Frank
@ 2022-01-14 10:39 ` Claudio Imbrenda
2022-01-17 7:04 ` Thomas Huth
1 sibling, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 10:39 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On Fri, 14 Jan 2022 10:02:43 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> Other hypervisors might implement it and therefore not send a
> specification exception.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/diag308.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/diag308.c b/s390x/diag308.c
> index c9d6c499..414dbdf4 100644
> --- a/s390x/diag308.c
> +++ b/s390x/diag308.c
> @@ -8,6 +8,7 @@
> #include <libcflat.h>
> #include <asm/asm-offsets.h>
> #include <asm/interrupt.h>
> +#include <vm.h>
>
> /* The diagnose calls should be blocked in problem state */
> static void test_priv(void)
> @@ -75,7 +76,7 @@ static void test_subcode6(void)
> /* Unsupported subcodes should generate a specification exception */
> static void test_unsupported_subcode(void)
> {
> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 };
> int idx;
>
> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void)
> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> report_prefix_pop();
> }
> +
> + /*
> + * Subcode 2 is not available under QEMU but might be on other
> + * hypervisors.
> + */
> + if (vm_is_tcg() || vm_is_kvm()) {
> + report_prefix_pushf("0x%04x", 2);
> + expect_pgm_int();
> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> + report_prefix_pop();
> + }
maybe add
else { report_skip("0x0002 Only tested in Qemu"); }
> }
>
> static struct {
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU
2022-01-14 10:02 ` [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU Janosch Frank
2022-01-14 10:39 ` Claudio Imbrenda
@ 2022-01-17 7:04 ` Thomas Huth
2022-01-17 9:39 ` Janosch Frank
1 sibling, 1 reply; 29+ messages in thread
From: Thomas Huth @ 2022-01-17 7:04 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, cohuck, nrb
On 14/01/2022 11.02, Janosch Frank wrote:
> Other hypervisors might implement it and therefore not send a
> specification exception.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/diag308.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/s390x/diag308.c b/s390x/diag308.c
> index c9d6c499..414dbdf4 100644
> --- a/s390x/diag308.c
> +++ b/s390x/diag308.c
> @@ -8,6 +8,7 @@
> #include <libcflat.h>
> #include <asm/asm-offsets.h>
> #include <asm/interrupt.h>
> +#include <vm.h>
>
> /* The diagnose calls should be blocked in problem state */
> static void test_priv(void)
> @@ -75,7 +76,7 @@ static void test_subcode6(void)
> /* Unsupported subcodes should generate a specification exception */
> static void test_unsupported_subcode(void)
> {
> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 };
> int idx;
>
> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void)
> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> report_prefix_pop();
> }
> +
> + /*
> + * Subcode 2 is not available under QEMU but might be on other
> + * hypervisors.
> + */
> + if (vm_is_tcg() || vm_is_kvm()) {
> + report_prefix_pushf("0x%04x", 2);
Maybe replace the format string with "0002" and drop the "2" parameter?
> + expect_pgm_int();
> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
> + report_prefix_pop();
> + }
> }
>
> static struct {
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU
2022-01-17 7:04 ` Thomas Huth
@ 2022-01-17 9:39 ` Janosch Frank
0 siblings, 0 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-17 9:39 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: linux-s390, imbrenda, david, cohuck, nrb
On 1/17/22 08:04, Thomas Huth wrote:
> On 14/01/2022 11.02, Janosch Frank wrote:
>> Other hypervisors might implement it and therefore not send a
>> specification exception.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>> s390x/diag308.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/s390x/diag308.c b/s390x/diag308.c
>> index c9d6c499..414dbdf4 100644
>> --- a/s390x/diag308.c
>> +++ b/s390x/diag308.c
>> @@ -8,6 +8,7 @@
>> #include <libcflat.h>
>> #include <asm/asm-offsets.h>
>> #include <asm/interrupt.h>
>> +#include <vm.h>
>>
>> /* The diagnose calls should be blocked in problem state */
>> static void test_priv(void)
>> @@ -75,7 +76,7 @@ static void test_subcode6(void)
>> /* Unsupported subcodes should generate a specification exception */
>> static void test_unsupported_subcode(void)
>> {
>> - int subcodes[] = { 2, 0x101, 0xffff, 0x10001, -1 };
>> + int subcodes[] = { 0x101, 0xffff, 0x10001, -1 };
>> int idx;
>>
>> for (idx = 0; idx < ARRAY_SIZE(subcodes); idx++) {
>> @@ -85,6 +86,18 @@ static void test_unsupported_subcode(void)
>> check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>> report_prefix_pop();
>> }
>> +
>> + /*
>> + * Subcode 2 is not available under QEMU but might be on other
>> + * hypervisors.
>> + */
>> + if (vm_is_tcg() || vm_is_kvm()) {
>> + report_prefix_pushf("0x%04x", 2);
>
> Maybe replace the format string with "0002" and drop the "2" parameter?
Sure
>
>> + expect_pgm_int();
>> + asm volatile ("diag %0,%1,0x308" :: "d"(0), "d"(2));
>> + check_pgm_int_code(PGM_INT_CODE_SPECIFICATION);
>> + report_prefix_pop();
>> + }
>> }
>>
>> static struct {
>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
Thanks
^ permalink raw reply [flat|nested] 29+ messages in thread
* [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
` (2 preceding siblings ...)
2022-01-14 10:02 ` [kvm-unit-tests PATCH 3/5] s390x: diag308: Only test subcode 2 under QEMU Janosch Frank
@ 2022-01-14 10:02 ` Janosch Frank
2022-01-14 11:18 ` Claudio Imbrenda
2022-01-14 12:50 ` Nico Boehr
2022-01-14 10:02 ` [kvm-unit-tests PATCH 5/5] s390x: firq: Fix sclp buffer allocation Janosch Frank
2022-01-14 11:19 ` [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Claudio Imbrenda
5 siblings, 2 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 10:02 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck, nrb
The store status at address order works with 31 bit addresses so let's
use them.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/smp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/s390x/smp.c b/s390x/smp.c
index 32f128b3..c91f170b 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -124,7 +124,7 @@ static void test_stop_store_status(void)
static void test_store_status(void)
{
- struct cpu_status *status = alloc_pages(1);
+ struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
uint32_t r;
report_prefix_push("store status at address");
@@ -244,7 +244,7 @@ static void test_func_initial(void)
static void test_reset_initial(void)
{
- struct cpu_status *status = alloc_pages(0);
+ struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
struct psw psw;
int i;
--
2.32.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 10:02 ` [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space Janosch Frank
@ 2022-01-14 11:18 ` Claudio Imbrenda
2022-01-14 12:50 ` Nico Boehr
1 sibling, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 11:18 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On Fri, 14 Jan 2022 10:02:44 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> The store status at address order works with 31 bit addresses so let's
> use them.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/smp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 32f128b3..c91f170b 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -124,7 +124,7 @@ static void test_stop_store_status(void)
>
> static void test_store_status(void)
> {
> - struct cpu_status *status = alloc_pages(1);
> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> uint32_t r;
>
> report_prefix_push("store status at address");
> @@ -244,7 +244,7 @@ static void test_func_initial(void)
>
> static void test_reset_initial(void)
> {
> - struct cpu_status *status = alloc_pages(0);
> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> struct psw psw;
> int i;
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 10:02 ` [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space Janosch Frank
2022-01-14 11:18 ` Claudio Imbrenda
@ 2022-01-14 12:50 ` Nico Boehr
2022-01-14 12:57 ` Claudio Imbrenda
2022-01-14 13:01 ` Claudio Imbrenda
1 sibling, 2 replies; 29+ messages in thread
From: Nico Boehr @ 2022-01-14 12:50 UTC (permalink / raw)
To: Janosch Frank, kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck
On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
> The store status at address order works with 31 bit addresses so
> let's
> use them.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
> s390x/smp.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 32f128b3..c91f170b 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
[...]
> @@ -244,7 +244,7 @@ static void test_func_initial(void)
>
> static void test_reset_initial(void)
> {
> - struct cpu_status *status = alloc_pages(0);
> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
Why do we need two pages now?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 12:50 ` Nico Boehr
@ 2022-01-14 12:57 ` Claudio Imbrenda
2022-01-14 13:07 ` Janosch Frank
2022-01-14 13:01 ` Claudio Imbrenda
1 sibling, 1 reply; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 12:57 UTC (permalink / raw)
To: Nico Boehr; +Cc: Janosch Frank, kvm, linux-s390, david, thuth, cohuck
On Fri, 14 Jan 2022 13:50:52 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:
> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
> > The store status at address order works with 31 bit addresses so
> > let's
> > use them.
> >
> > Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> > ---
> > s390x/smp.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/s390x/smp.c b/s390x/smp.c
> > index 32f128b3..c91f170b 100644
> > --- a/s390x/smp.c
> > +++ b/s390x/smp.c
>
> [...]
>
> > @@ -244,7 +244,7 @@ static void test_func_initial(void)
> >
> > static void test_reset_initial(void)
> > {
> > - struct cpu_status *status = alloc_pages(0);
> > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>
> Why do we need two pages now?
oh, good catch
the next patch has the same issue
I can fix them up when I queue them
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 12:57 ` Claudio Imbrenda
@ 2022-01-14 13:07 ` Janosch Frank
0 siblings, 0 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 13:07 UTC (permalink / raw)
To: Claudio Imbrenda, Nico Boehr; +Cc: kvm, linux-s390, david, thuth, cohuck
On 1/14/22 13:57, Claudio Imbrenda wrote:
> On Fri, 14 Jan 2022 13:50:52 +0100
> Nico Boehr <nrb@linux.ibm.com> wrote:
>
>> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
>>> The store status at address order works with 31 bit addresses so
>>> let's
>>> use them.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>> s390x/smp.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/s390x/smp.c b/s390x/smp.c
>>> index 32f128b3..c91f170b 100644
>>> --- a/s390x/smp.c
>>> +++ b/s390x/smp.c
>>
>> [...]
>>
>>> @@ -244,7 +244,7 @@ static void test_func_initial(void)
>>>
>>> static void test_reset_initial(void)
>>> {
>>> - struct cpu_status *status = alloc_pages(0);
>>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>>
>> Why do we need two pages now?
>
> oh, good catch
Indeed
>
> the next patch has the same issue
>
> I can fix them up when I queue them
>
Probably because I copied that line from somewhere :-)
Yeah, you can fix that up if you want.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 12:50 ` Nico Boehr
2022-01-14 12:57 ` Claudio Imbrenda
@ 2022-01-14 13:01 ` Claudio Imbrenda
2022-01-14 13:13 ` Janosch Frank
1 sibling, 1 reply; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 13:01 UTC (permalink / raw)
To: Nico Boehr; +Cc: Janosch Frank, kvm, linux-s390, david, thuth, cohuck
On Fri, 14 Jan 2022 13:50:52 +0100
Nico Boehr <nrb@linux.ibm.com> wrote:
> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
> > The store status at address order works with 31 bit addresses so
> > let's
> > use them.
> >
> > Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> > ---
> > s390x/smp.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/s390x/smp.c b/s390x/smp.c
> > index 32f128b3..c91f170b 100644
> > --- a/s390x/smp.c
> > +++ b/s390x/smp.c
>
> [...]
>
> > @@ -244,7 +244,7 @@ static void test_func_initial(void)
> >
> > static void test_reset_initial(void)
> > {
> > - struct cpu_status *status = alloc_pages(0);
> > + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>
> Why do we need two pages now?
actually, wait.....
struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
uint32_t r;
report_prefix_push("store status at address");
memset(status, 0, PAGE_SIZE * 2);
we were allocating one page, and using 2!
@Janosch do we need 1 or 2 pages?
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 13:01 ` Claudio Imbrenda
@ 2022-01-14 13:13 ` Janosch Frank
2022-01-14 13:16 ` Claudio Imbrenda
2022-01-14 13:19 ` Claudio Imbrenda
0 siblings, 2 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 13:13 UTC (permalink / raw)
To: Claudio Imbrenda, Nico Boehr; +Cc: kvm, linux-s390, david, thuth, cohuck
On 1/14/22 14:01, Claudio Imbrenda wrote:
> On Fri, 14 Jan 2022 13:50:52 +0100
> Nico Boehr <nrb@linux.ibm.com> wrote:
>
>> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
>>> The store status at address order works with 31 bit addresses so
>>> let's
>>> use them.
>>>
>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>> ---
>>> s390x/smp.c | 4 ++--
>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/s390x/smp.c b/s390x/smp.c
>>> index 32f128b3..c91f170b 100644
>>> --- a/s390x/smp.c
>>> +++ b/s390x/smp.c
>>
>> [...]
>>
>>> @@ -244,7 +244,7 @@ static void test_func_initial(void)
>>>
>>> static void test_reset_initial(void)
>>> {
>>> - struct cpu_status *status = alloc_pages(0);
>>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>>
>> Why do we need two pages now?
>
> actually, wait.....
>
> struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> uint32_t r;
>
> report_prefix_push("store status at address");
> memset(status, 0, PAGE_SIZE * 2);
>
> we were allocating one page, and using 2!
>
> @Janosch do we need 1 or 2 pages?
>
Have a look at the memcmp() below those lines.
I test if the status page has changed by doing a memcmp against the
second page.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 13:13 ` Janosch Frank
@ 2022-01-14 13:16 ` Claudio Imbrenda
2022-01-14 13:18 ` Janosch Frank
2022-01-14 13:19 ` Claudio Imbrenda
1 sibling, 1 reply; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 13:16 UTC (permalink / raw)
To: Janosch Frank; +Cc: Nico Boehr, kvm, linux-s390, david, thuth, cohuck
On Fri, 14 Jan 2022 14:13:01 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:
> On 1/14/22 14:01, Claudio Imbrenda wrote:
> > On Fri, 14 Jan 2022 13:50:52 +0100
> > Nico Boehr <nrb@linux.ibm.com> wrote:
> >
> >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
> >>> The store status at address order works with 31 bit addresses so
> >>> let's
> >>> use them.
> >>>
> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>> ---
> >>> s390x/smp.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/s390x/smp.c b/s390x/smp.c
> >>> index 32f128b3..c91f170b 100644
> >>> --- a/s390x/smp.c
> >>> +++ b/s390x/smp.c
> >>
> >> [...]
> >>
> >>> @@ -244,7 +244,7 @@ static void test_func_initial(void)
> >>>
> >>> static void test_reset_initial(void)
> >>> {
> >>> - struct cpu_status *status = alloc_pages(0);
> >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> >>
> >> Why do we need two pages now?
> >
> > actually, wait.....
> >
> > struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> > uint32_t r;
> >
> > report_prefix_push("store status at address");
> > memset(status, 0, PAGE_SIZE * 2);
> >
> > we were allocating one page, and using 2!
> >
> > @Janosch do we need 1 or 2 pages?
> >
>
> Have a look at the memcmp() below those lines.
>
> I test if the status page has changed by doing a memcmp against the
> second page.
so we do need 2 pages, and using 1 was a bug
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 13:16 ` Claudio Imbrenda
@ 2022-01-14 13:18 ` Janosch Frank
0 siblings, 0 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 13:18 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: Nico Boehr, kvm, linux-s390, david, thuth, cohuck
On 1/14/22 14:16, Claudio Imbrenda wrote:
> On Fri, 14 Jan 2022 14:13:01 +0100
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> On 1/14/22 14:01, Claudio Imbrenda wrote:
>>> On Fri, 14 Jan 2022 13:50:52 +0100
>>> Nico Boehr <nrb@linux.ibm.com> wrote:
>>>
>>>> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
>>>>> The store status at address order works with 31 bit addresses so
>>>>> let's
>>>>> use them.
>>>>>
>>>>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>>>>> ---
>>>>> s390x/smp.c | 4 ++--
>>>>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/s390x/smp.c b/s390x/smp.c
>>>>> index 32f128b3..c91f170b 100644
>>>>> --- a/s390x/smp.c
>>>>> +++ b/s390x/smp.c
>>>>
>>>> [...]
>>>>
>>>>> @@ -244,7 +244,7 @@ static void test_func_initial(void)
>>>>>
>>>>> static void test_reset_initial(void)
>>>>> {
>>>>> - struct cpu_status *status = alloc_pages(0);
>>>>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>>>>
>>>> Why do we need two pages now?
>>>
>>> actually, wait.....
>>>
>>> struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
>>> uint32_t r;
>>>
>>> report_prefix_push("store status at address");
>>> memset(status, 0, PAGE_SIZE * 2);
>>>
>>> we were allocating one page, and using 2!
>>>
>>> @Janosch do we need 1 or 2 pages?
>>>
>>
>> Have a look at the memcmp() below those lines.
>>
>> I test if the status page has changed by doing a memcmp against the
>> second page.
>
> so we do need 2 pages, and using 1 was a bug
>
We need two for the store status at address tests but only one for the
initial reset test Nico is pointing out here.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space
2022-01-14 13:13 ` Janosch Frank
2022-01-14 13:16 ` Claudio Imbrenda
@ 2022-01-14 13:19 ` Claudio Imbrenda
1 sibling, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 13:19 UTC (permalink / raw)
To: Janosch Frank; +Cc: Nico Boehr, kvm, linux-s390, david, thuth, cohuck
On Fri, 14 Jan 2022 14:13:01 +0100
Janosch Frank <frankja@linux.ibm.com> wrote:
> On 1/14/22 14:01, Claudio Imbrenda wrote:
> > On Fri, 14 Jan 2022 13:50:52 +0100
> > Nico Boehr <nrb@linux.ibm.com> wrote:
> >
> >> On Fri, 2022-01-14 at 10:02 +0000, Janosch Frank wrote:
> >>> The store status at address order works with 31 bit addresses so
> >>> let's
> >>> use them.
> >>>
> >>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> >>> ---
> >>> s390x/smp.c | 4 ++--
> >>> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/s390x/smp.c b/s390x/smp.c
> >>> index 32f128b3..c91f170b 100644
> >>> --- a/s390x/smp.c
> >>> +++ b/s390x/smp.c
> >>
> >> [...]
> >>
> >>> @@ -244,7 +244,7 @@ static void test_func_initial(void)
> >>>
> >>> static void test_reset_initial(void)
> >>> {
> >>> - struct cpu_status *status = alloc_pages(0);
> >>> + struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> >>
> >> Why do we need two pages now?
> >
> > actually, wait.....
> >
> > struct cpu_status *status = alloc_pages_flags(1, AREA_DMA31);
> > uint32_t r;
> >
> > report_prefix_push("store status at address");
> > memset(status, 0, PAGE_SIZE * 2);
> >
> > we were allocating one page, and using 2!
> >
> > @Janosch do we need 1 or 2 pages?
> >
>
> Have a look at the memcmp() below those lines.
>
> I test if the status page has changed by doing a memcmp against the
> second page.
ENOCOFEE
sorry I mixed things up
we were using 2, it's the second part of the patch that only needs one
^ permalink raw reply [flat|nested] 29+ messages in thread
* [kvm-unit-tests PATCH 5/5] s390x: firq: Fix sclp buffer allocation
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
` (3 preceding siblings ...)
2022-01-14 10:02 ` [kvm-unit-tests PATCH 4/5] s390x: smp: Allocate memory in DMA31 space Janosch Frank
@ 2022-01-14 10:02 ` Janosch Frank
2022-01-14 11:19 ` Claudio Imbrenda
2022-01-14 11:19 ` [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Claudio Imbrenda
5 siblings, 1 reply; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 10:02 UTC (permalink / raw)
To: kvm; +Cc: linux-s390, imbrenda, david, thuth, cohuck, nrb
We need a 32 bit address for the sclp buffer so let's use a page from
the first 31 bits.
Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
s390x/firq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/s390x/firq.c b/s390x/firq.c
index 1f877183..a0ef1555 100644
--- a/s390x/firq.c
+++ b/s390x/firq.c
@@ -87,7 +87,7 @@ static void test_wait_state_delivery(void)
*/
while(smp_sense_running_status(1));
- h = alloc_page();
+ h = alloc_pages_flags(1, AREA_DMA31);
h->length = 4096;
ret = servc(SCLP_CMDW_READ_CPU_INFO, __pa(h));
if (ret) {
--
2.32.0
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 5/5] s390x: firq: Fix sclp buffer allocation
2022-01-14 10:02 ` [kvm-unit-tests PATCH 5/5] s390x: firq: Fix sclp buffer allocation Janosch Frank
@ 2022-01-14 11:19 ` Claudio Imbrenda
0 siblings, 0 replies; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 11:19 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On Fri, 14 Jan 2022 10:02:45 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> We need a 32 bit address for the sclp buffer so let's use a page from
> the first 31 bits.
>
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
> ---
> s390x/firq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/s390x/firq.c b/s390x/firq.c
> index 1f877183..a0ef1555 100644
> --- a/s390x/firq.c
> +++ b/s390x/firq.c
> @@ -87,7 +87,7 @@ static void test_wait_state_delivery(void)
> */
> while(smp_sense_running_status(1));
>
> - h = alloc_page();
> + h = alloc_pages_flags(1, AREA_DMA31);
> h->length = 4096;
> ret = servc(SCLP_CMDW_READ_CPU_INFO, __pa(h));
> if (ret) {
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes
2022-01-14 10:02 [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Janosch Frank
` (4 preceding siblings ...)
2022-01-14 10:02 ` [kvm-unit-tests PATCH 5/5] s390x: firq: Fix sclp buffer allocation Janosch Frank
@ 2022-01-14 11:19 ` Claudio Imbrenda
2022-01-14 12:23 ` Janosch Frank
5 siblings, 1 reply; 29+ messages in thread
From: Claudio Imbrenda @ 2022-01-14 11:19 UTC (permalink / raw)
To: Janosch Frank; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On Fri, 14 Jan 2022 10:02:40 +0000
Janosch Frank <frankja@linux.ibm.com> wrote:
> I took some time before Christmas to write a test runner for lpar
> which automatically runs all tests and sends me the logs. It's based
> on the zhmc library to control starting and stopping of the lpar and
> works by having a menu entry for each kvm unit test.
>
> This revealed a number of test fails when the tests are run under lpar
> as there are a few differences:
> * lpars most often have a very high memory amount (upwards of 8GB)
> compared to our qemu env (256MB)
> * lpar supports diag308 subcode 2
> * lpar does not provide virtio devices
>
> The higher memory amount leads to allocations crossing the 2GB or 4GB
> border which made sclp and sigp calls fail that expect 31/32 bit
> addresses.
>
the series looks good to me; if you send me a fixed patch 3, I'll queue
this together with the other ones
> Janosch Frank (5):
> lib: s390x: vm: Add kvm and lpar vm queries
> s390x: css: Skip if we're not run by qemu
> s390x: diag308: Only test subcode 2 under QEMU
> s390x: smp: Allocate memory in DMA31 space
> s390x: firq: Fix sclp buffer allocation
>
> lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++
> lib/s390x/vm.h | 23 +++++++++++++++++++++++
> s390x/css.c | 10 +++++++++-
> s390x/diag308.c | 15 ++++++++++++++-
> s390x/firq.c | 2 +-
> s390x/smp.c | 4 ++--
> s390x/stsi.c | 21 +--------------------
> 7 files changed, 89 insertions(+), 25 deletions(-)
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes
2022-01-14 11:19 ` [kvm-unit-tests PATCH 0/5] s390x: Allocation and hosting environment detection fixes Claudio Imbrenda
@ 2022-01-14 12:23 ` Janosch Frank
0 siblings, 0 replies; 29+ messages in thread
From: Janosch Frank @ 2022-01-14 12:23 UTC (permalink / raw)
To: Claudio Imbrenda; +Cc: kvm, linux-s390, david, thuth, cohuck, nrb
On 1/14/22 12:19, Claudio Imbrenda wrote:
> On Fri, 14 Jan 2022 10:02:40 +0000
> Janosch Frank <frankja@linux.ibm.com> wrote:
>
>> I took some time before Christmas to write a test runner for lpar
>> which automatically runs all tests and sends me the logs. It's based
>> on the zhmc library to control starting and stopping of the lpar and
>> works by having a menu entry for each kvm unit test.
>>
>> This revealed a number of test fails when the tests are run under lpar
>> as there are a few differences:
>> * lpars most often have a very high memory amount (upwards of 8GB)
>> compared to our qemu env (256MB)
>> * lpar supports diag308 subcode 2
>> * lpar does not provide virtio devices
>>
>> The higher memory amount leads to allocations crossing the 2GB or 4GB
>> border which made sclp and sigp calls fail that expect 31/32 bit
>> addresses.
>>
>
> the series looks good to me; if you send me a fixed patch 3, I'll queue
> this together with the other ones
Well, since Pierre originally came up with a large part of the code for
patch 1 I'll wait with a new version until we picked his fixed patch so
I can rebase on it.
But you can already pick the allocation patches if you want.
>
>> Janosch Frank (5):
>> lib: s390x: vm: Add kvm and lpar vm queries
>> s390x: css: Skip if we're not run by qemu
>> s390x: diag308: Only test subcode 2 under QEMU
>> s390x: smp: Allocate memory in DMA31 space
>> s390x: firq: Fix sclp buffer allocation
>>
>> lib/s390x/vm.c | 39 +++++++++++++++++++++++++++++++++++++++
>> lib/s390x/vm.h | 23 +++++++++++++++++++++++
>> s390x/css.c | 10 +++++++++-
>> s390x/diag308.c | 15 ++++++++++++++-
>> s390x/firq.c | 2 +-
>> s390x/smp.c | 4 ++--
>> s390x/stsi.c | 21 +--------------------
>> 7 files changed, 89 insertions(+), 25 deletions(-)
>>
>
^ permalink raw reply [flat|nested] 29+ messages in thread