* [kvm-unit-tests PATCH 0/1] s390x: css: check the CSS is working with any ISC
@ 2021-08-12 11:53 Pierre Morel
2021-08-12 11:53 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel
0 siblings, 1 reply; 6+ messages in thread
From: Pierre Morel @ 2021-08-12 11:53 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
Picking the ISC used by Linux for I/O to make the CSS checks may cause
a bias.
Let's check all possible ISC for I/O to verify that QEMU/KVM is really ISC
independent.
Pierre Morel (1):
s390x: css: check the CSS is working with any ISC
s390x/css.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH 1/1] s390x: css: check the CSS is working with any ISC
2021-08-12 11:53 [kvm-unit-tests PATCH 0/1] s390x: css: check the CSS is working with any ISC Pierre Morel
@ 2021-08-12 11:53 ` Pierre Morel
2021-08-12 12:31 ` Cornelia Huck
2021-08-18 7:40 ` Thomas Huth
0 siblings, 2 replies; 6+ messages in thread
From: Pierre Morel @ 2021-08-12 11:53 UTC (permalink / raw)
To: kvm; +Cc: frankja, david, thuth, cohuck, imbrenda
In the previous version we did only check that one ISC dedicated by
Linux for I/O is working fine.
However, there is no reason to prefer one ISC to another ISC, we are
free to take anyone.
Let's check all possible ISC to verify that QEMU/KVM is really ISC
independent.
Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
---
s390x/css.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/s390x/css.c b/s390x/css.c
index c340c539..aa005309 100644
--- a/s390x/css.c
+++ b/s390x/css.c
@@ -22,6 +22,7 @@
#define DEFAULT_CU_TYPE 0x3832 /* virtio-ccw */
static unsigned long cu_type = DEFAULT_CU_TYPE;
+static int io_isc;
static int test_device_sid;
static struct senseid *senseid;
@@ -46,7 +47,7 @@ static void test_enable(void)
return;
}
- cc = css_enable(test_device_sid, IO_SCH_ISC);
+ cc = css_enable(test_device_sid, io_isc);
report(cc == 0, "Enable subchannel %08x", test_device_sid);
}
@@ -67,7 +68,7 @@ static void test_sense(void)
return;
}
- ret = css_enable(test_device_sid, IO_SCH_ISC);
+ ret = css_enable(test_device_sid, io_isc);
if (ret) {
report(0, "Could not enable the subchannel: %08x",
test_device_sid);
@@ -142,7 +143,6 @@ static void sense_id(void)
static void css_init(void)
{
- assert(register_io_int_func(css_irq_io) == 0);
lowcore_ptr->io_int_param = 0;
report(get_chsc_scsc(), "Store Channel Characteristics");
@@ -351,11 +351,20 @@ int main(int argc, char *argv[])
int i;
report_prefix_push("Channel Subsystem");
- enable_io_isc(0x80 >> IO_SCH_ISC);
- for (i = 0; tests[i].name; i++) {
- report_prefix_push(tests[i].name);
- tests[i].func();
- report_prefix_pop();
+
+ for (io_isc = 0; io_isc < 8; io_isc++) {
+ report_info("ISC: %d\n", io_isc);
+
+ enable_io_isc(0x80 >> io_isc);
+ assert(register_io_int_func(css_irq_io) == 0);
+
+ for (i = 0; tests[i].name; i++) {
+ report_prefix_push(tests[i].name);
+ tests[i].func();
+ report_prefix_pop();
+ }
+
+ unregister_io_int_func(css_irq_io);
}
report_prefix_pop();
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: css: check the CSS is working with any ISC
2021-08-12 11:53 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel
@ 2021-08-12 12:31 ` Cornelia Huck
2021-08-12 14:59 ` Pierre Morel
2021-08-18 7:40 ` Thomas Huth
1 sibling, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2021-08-12 12:31 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: frankja, david, thuth, imbrenda
On Thu, Aug 12 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:
> In the previous version we did only check that one ISC dedicated by
> Linux for I/O is working fine.
>
> However, there is no reason to prefer one ISC to another ISC, we are
> free to take anyone.
>
> Let's check all possible ISC to verify that QEMU/KVM is really ISC
> independent.
It's probably a good idea to test for a non-standard isc. Not sure
whether we need all of them, but it doesn't hurt.
Do you also have plans for a test to verify the priority handling for
the different iscs?
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> s390x/css.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
(...)
> @@ -142,7 +143,6 @@ static void sense_id(void)
>
> static void css_init(void)
> {
> - assert(register_io_int_func(css_irq_io) == 0);
> lowcore_ptr->io_int_param = 0;
>
> report(get_chsc_scsc(), "Store Channel Characteristics");
> @@ -351,11 +351,20 @@ int main(int argc, char *argv[])
> int i;
>
> report_prefix_push("Channel Subsystem");
> - enable_io_isc(0x80 >> IO_SCH_ISC);
> - for (i = 0; tests[i].name; i++) {
> - report_prefix_push(tests[i].name);
> - tests[i].func();
> - report_prefix_pop();
> +
> + for (io_isc = 0; io_isc < 8; io_isc++) {
> + report_info("ISC: %d\n", io_isc);
> +
> + enable_io_isc(0x80 >> io_isc);
> + assert(register_io_int_func(css_irq_io) == 0);
Why are you registering/deregistering the irq handler multiple times? It
should be the same, regardless of the isc?
> +
> + for (i = 0; tests[i].name; i++) {
> + report_prefix_push(tests[i].name);
> + tests[i].func();
> + report_prefix_pop();
> + }
> +
> + unregister_io_int_func(css_irq_io);
> }
> report_prefix_pop();
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: css: check the CSS is working with any ISC
2021-08-12 12:31 ` Cornelia Huck
@ 2021-08-12 14:59 ` Pierre Morel
0 siblings, 0 replies; 6+ messages in thread
From: Pierre Morel @ 2021-08-12 14:59 UTC (permalink / raw)
To: Cornelia Huck, kvm; +Cc: frankja, david, thuth, imbrenda
On 8/12/21 2:31 PM, Cornelia Huck wrote:
> On Thu, Aug 12 2021, Pierre Morel <pmorel@linux.ibm.com> wrote:
>
>> In the previous version we did only check that one ISC dedicated by
>> Linux for I/O is working fine.
>>
>> However, there is no reason to prefer one ISC to another ISC, we are
>> free to take anyone.
>>
>> Let's check all possible ISC to verify that QEMU/KVM is really ISC
>> independent.
>
> It's probably a good idea to test for a non-standard isc. Not sure
> whether we need all of them, but it doesn't hurt.
>
> Do you also have plans for a test to verify the priority handling for
> the different iscs?
No, I did not think about this yet.
>
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> s390x/css.c | 25 +++++++++++++++++--------
>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>
>
> (...)
>
>> @@ -142,7 +143,6 @@ static void sense_id(void)
>>
>> static void css_init(void)
>> {
>> - assert(register_io_int_func(css_irq_io) == 0);
>> lowcore_ptr->io_int_param = 0;
>>
>> report(get_chsc_scsc(), "Store Channel Characteristics");
>> @@ -351,11 +351,20 @@ int main(int argc, char *argv[])
>> int i;
>>
>> report_prefix_push("Channel Subsystem");
>> - enable_io_isc(0x80 >> IO_SCH_ISC);
>> - for (i = 0; tests[i].name; i++) {
>> - report_prefix_push(tests[i].name);
>> - tests[i].func();
>> - report_prefix_pop();
>> +
>> + for (io_isc = 0; io_isc < 8; io_isc++) {
>> + report_info("ISC: %d\n", io_isc);
>> +
>> + enable_io_isc(0x80 >> io_isc);
>> + assert(register_io_int_func(css_irq_io) == 0);
>
> Why are you registering/deregistering the irq handler multiple times? It
> should be the same, regardless of the isc?
Yes, right, did not pay attention when pushing all in the loop,
I will get it out of the loop.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: css: check the CSS is working with any ISC
2021-08-12 11:53 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel
2021-08-12 12:31 ` Cornelia Huck
@ 2021-08-18 7:40 ` Thomas Huth
2021-08-23 9:16 ` Pierre Morel
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2021-08-18 7:40 UTC (permalink / raw)
To: Pierre Morel, kvm; +Cc: frankja, david, cohuck, imbrenda
On 12/08/2021 13.53, Pierre Morel wrote:
> In the previous version we did only check that one ISC dedicated by
> Linux for I/O is working fine.
>
> However, there is no reason to prefer one ISC to another ISC, we are
> free to take anyone.
>
> Let's check all possible ISC to verify that QEMU/KVM is really ISC
> independent.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
> s390x/css.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/s390x/css.c b/s390x/css.c
> index c340c539..aa005309 100644
> --- a/s390x/css.c
> +++ b/s390x/css.c
> @@ -22,6 +22,7 @@
>
> #define DEFAULT_CU_TYPE 0x3832 /* virtio-ccw */
> static unsigned long cu_type = DEFAULT_CU_TYPE;
> +static int io_isc;
>
> static int test_device_sid;
> static struct senseid *senseid;
> @@ -46,7 +47,7 @@ static void test_enable(void)
> return;
> }
>
> - cc = css_enable(test_device_sid, IO_SCH_ISC);
> + cc = css_enable(test_device_sid, io_isc);
>
> report(cc == 0, "Enable subchannel %08x", test_device_sid);
> }
> @@ -67,7 +68,7 @@ static void test_sense(void)
> return;
> }
>
> - ret = css_enable(test_device_sid, IO_SCH_ISC);
> + ret = css_enable(test_device_sid, io_isc);
> if (ret) {
> report(0, "Could not enable the subchannel: %08x",
> test_device_sid);
> @@ -142,7 +143,6 @@ static void sense_id(void)
>
> static void css_init(void)
> {
> - assert(register_io_int_func(css_irq_io) == 0);
> lowcore_ptr->io_int_param = 0;
>
> report(get_chsc_scsc(), "Store Channel Characteristics");
> @@ -351,11 +351,20 @@ int main(int argc, char *argv[])
> int i;
>
> report_prefix_push("Channel Subsystem");
> - enable_io_isc(0x80 >> IO_SCH_ISC);
> - for (i = 0; tests[i].name; i++) {
> - report_prefix_push(tests[i].name);
> - tests[i].func();
> - report_prefix_pop();
> +
> + for (io_isc = 0; io_isc < 8; io_isc++) {
> + report_info("ISC: %d\n", io_isc);
Would it make sense to add the "ISC" string as a prefix with
report_prefix_push() instead, so that the tests get individual test names?
Thomas
> + enable_io_isc(0x80 >> io_isc);
> + assert(register_io_int_func(css_irq_io) == 0);
> +
> + for (i = 0; tests[i].name; i++) {
> + report_prefix_push(tests[i].name);
> + tests[i].func();
> + report_prefix_pop();
> + }
> +
> + unregister_io_int_func(css_irq_io);
> }
> report_prefix_pop();
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 1/1] s390x: css: check the CSS is working with any ISC
2021-08-18 7:40 ` Thomas Huth
@ 2021-08-23 9:16 ` Pierre Morel
0 siblings, 0 replies; 6+ messages in thread
From: Pierre Morel @ 2021-08-23 9:16 UTC (permalink / raw)
To: Thomas Huth, kvm; +Cc: frankja, david, cohuck, imbrenda
On 8/18/21 9:40 AM, Thomas Huth wrote:
> On 12/08/2021 13.53, Pierre Morel wrote:
>> In the previous version we did only check that one ISC dedicated by
>> Linux for I/O is working fine.
>>
>> However, there is no reason to prefer one ISC to another ISC, we are
>> free to take anyone.
>>
>> Let's check all possible ISC to verify that QEMU/KVM is really ISC
>> independent.
>>
>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>> ---
>> s390x/css.c | 25 +++++++++++++++++--------
>> 1 file changed, 17 insertions(+), 8 deletions(-)
>>
>> diff --git a/s390x/css.c b/s390x/css.c
>> index c340c539..aa005309 100644
>> --- a/s390x/css.c
>> +++ b/s390x/css.c
>> @@ -22,6 +22,7 @@
>> #define DEFAULT_CU_TYPE 0x3832 /* virtio-ccw */
>> static unsigned long cu_type = DEFAULT_CU_TYPE;
>> +static int io_isc;
>> static int test_device_sid;
>> static struct senseid *senseid;
>> @@ -46,7 +47,7 @@ static void test_enable(void)
>> return;
>> }
>> - cc = css_enable(test_device_sid, IO_SCH_ISC);
>> + cc = css_enable(test_device_sid, io_isc);
>> report(cc == 0, "Enable subchannel %08x", test_device_sid);
>> }
>> @@ -67,7 +68,7 @@ static void test_sense(void)
>> return;
>> }
>> - ret = css_enable(test_device_sid, IO_SCH_ISC);
>> + ret = css_enable(test_device_sid, io_isc);
>> if (ret) {
>> report(0, "Could not enable the subchannel: %08x",
>> test_device_sid);
>> @@ -142,7 +143,6 @@ static void sense_id(void)
>> static void css_init(void)
>> {
>> - assert(register_io_int_func(css_irq_io) == 0);
>> lowcore_ptr->io_int_param = 0;
>> report(get_chsc_scsc(), "Store Channel Characteristics");
>> @@ -351,11 +351,20 @@ int main(int argc, char *argv[])
>> int i;
>> report_prefix_push("Channel Subsystem");
>> - enable_io_isc(0x80 >> IO_SCH_ISC);
>> - for (i = 0; tests[i].name; i++) {
>> - report_prefix_push(tests[i].name);
>> - tests[i].func();
>> - report_prefix_pop();
>> +
>> + for (io_isc = 0; io_isc < 8; io_isc++) {
>> + report_info("ISC: %d\n", io_isc);
>
> Would it make sense to add the "ISC" string as a prefix with
> report_prefix_push() instead, so that the tests get individual test names?
>
> Thomas
>
Yes, this would make a better description I think.
Thanks,
Pierre
--
Pierre Morel
IBM Lab Boeblingen
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-08-23 9:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-12 11:53 [kvm-unit-tests PATCH 0/1] s390x: css: check the CSS is working with any ISC Pierre Morel
2021-08-12 11:53 ` [kvm-unit-tests PATCH 1/1] " Pierre Morel
2021-08-12 12:31 ` Cornelia Huck
2021-08-12 14:59 ` Pierre Morel
2021-08-18 7:40 ` Thomas Huth
2021-08-23 9:16 ` Pierre Morel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).