[kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[Christian Borntraeger]

Two minimal tests:
- our own CPU should be running when we check ourselves
- a CPU should at least have some times with a not running
indication. To speed things up we stop CPU1

Also rename smp_cpu_running to smp_sense_running_status.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
 lib/s390x/smp.c |  2 +-
 lib/s390x/smp.h |  2 +-
 s390x/smp.c     | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
index 5ed8b7b..492cb05 100644
--- a/lib/s390x/smp.c
+++ b/lib/s390x/smp.c
@@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
 	return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
 }
 
-bool smp_cpu_running(uint16_t addr)
+bool smp_sense_running_status(uint16_t addr)
 {
 	if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
 		return true;
diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
index a8b98c0..639ec92 100644
--- a/lib/s390x/smp.h
+++ b/lib/s390x/smp.h
@@ -40,7 +40,7 @@ struct cpu_status {
 int smp_query_num_cpus(void);
 struct cpu *smp_cpu_from_addr(uint16_t addr);
 bool smp_cpu_stopped(uint16_t addr);
-bool smp_cpu_running(uint16_t addr);
+bool smp_sense_running_status(uint16_t addr);
 int smp_cpu_restart(uint16_t addr);
 int smp_cpu_start(uint16_t addr, struct psw psw);
 int smp_cpu_stop(uint16_t addr);
diff --git a/s390x/smp.c b/s390x/smp.c
index 79cdc1f..4450aff 100644
--- a/s390x/smp.c
+++ b/s390x/smp.c
@@ -210,6 +210,20 @@ static void test_emcall(void)
 	report_prefix_pop();
 }
 
+static void test_sense_running(void)
+{
+	report_prefix_push("sense_running");
+	/* we are running */
+	report(smp_sense_running_status(0), "CPU0 sense claims running");
+	/* make sure CPU is stopped to speed up the not running case */
+	smp_cpu_stop(1);
+	/* Make sure to have at least one time with a not running indication */
+	while(smp_sense_running_status(1));
+	report(true, "CPU1 sense claims not running");
+	report_prefix_pop();
+}
+
+
 /* Used to dirty registers of cpu #1 before it is reset */
 static void test_func_initial(void)
 {
@@ -319,6 +333,7 @@ int main(void)
 	test_store_status();
 	test_ecall();
 	test_emcall();
+	test_sense_running();
 	test_reset();
 	test_reset_initial();
 	smp_cpu_destroy(1);
-- 
2.25.1

Re: [kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[David Hildenbrand]

On 02.04.20 17:44, Christian Borntraeger wrote:
> Two minimal tests:
> - our own CPU should be running when we check ourselves
> - a CPU should at least have some times with a not running
> indication. To speed things up we stop CPU1
> 
> Also rename smp_cpu_running to smp_sense_running_status.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  lib/s390x/smp.c |  2 +-
>  lib/s390x/smp.h |  2 +-
>  s390x/smp.c     | 15 +++++++++++++++
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 5ed8b7b..492cb05 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
>  	return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
>  }
>  
> -bool smp_cpu_running(uint16_t addr)
> +bool smp_sense_running_status(uint16_t addr)
>  {
>  	if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
>  		return true;
> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
> index a8b98c0..639ec92 100644
> --- a/lib/s390x/smp.h
> +++ b/lib/s390x/smp.h
> @@ -40,7 +40,7 @@ struct cpu_status {
>  int smp_query_num_cpus(void);
>  struct cpu *smp_cpu_from_addr(uint16_t addr);
>  bool smp_cpu_stopped(uint16_t addr);
> -bool smp_cpu_running(uint16_t addr);
> +bool smp_sense_running_status(uint16_t addr);
>  int smp_cpu_restart(uint16_t addr);
>  int smp_cpu_start(uint16_t addr, struct psw psw);
>  int smp_cpu_stop(uint16_t addr);
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 79cdc1f..4450aff 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -210,6 +210,20 @@ static void test_emcall(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_sense_running(void)
> +{
> +	report_prefix_push("sense_running");
> +	/* we are running */
> +	report(smp_sense_running_status(0), "CPU0 sense claims running");
> +	/* make sure CPU is stopped to speed up the not running case */
> +	smp_cpu_stop(1);
> +	/* Make sure to have at least one time with a not running indication */
> +	while(smp_sense_running_status(1));
> +	report(true, "CPU1 sense claims not running");
> +	report_prefix_pop();
> +}
> +
> +

Did not apply cleanly, (due to comment in front of the next function).
Dropped one of the two empty lines.

Queued to

https://github.com/davidhildenbrand/qemu.git s390-tcg-next

for now. Happy to add acks/rb's ;)

Tested with TCG.

Thanks!

-- 
Thanks,

David / dhildenb

Re: [kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[Cornelia Huck]

On Thu,  2 Apr 2020 11:44:41 -0400
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> Two minimal tests:
> - our own CPU should be running when we check ourselves
> - a CPU should at least have some times with a not running
> indication. To speed things up we stop CPU1
> 
> Also rename smp_cpu_running to smp_sense_running_status.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  lib/s390x/smp.c |  2 +-
>  lib/s390x/smp.h |  2 +-
>  s390x/smp.c     | 15 +++++++++++++++
>  3 files changed, 17 insertions(+), 2 deletions(-)
>

(...)

> diff --git a/s390x/smp.c b/s390x/smp.c
> index 79cdc1f..4450aff 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -210,6 +210,20 @@ static void test_emcall(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_sense_running(void)
> +{
> +	report_prefix_push("sense_running");
> +	/* we are running */

Maybe /* we (CPU0) are running */ ?

> +	report(smp_sense_running_status(0), "CPU0 sense claims running");
> +	/* make sure CPU is stopped to speed up the not running case */

"the target CPU" ?

> +	smp_cpu_stop(1);
> +	/* Make sure to have at least one time with a not running indication */
> +	while(smp_sense_running_status(1));
> +	report(true, "CPU1 sense claims not running");
> +	report_prefix_pop();
> +}
> +
> +
>  /* Used to dirty registers of cpu #1 before it is reset */
>  static void test_func_initial(void)
>  {

(...)

Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Re: [kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[Janosch Frank]

[-- Attachment #1.1: Type: text/plain, Size: 2660 bytes --]

On 4/2/20 5:44 PM, Christian Borntraeger wrote:
> Two minimal tests:
> - our own CPU should be running when we check ourselves
> - a CPU should at least have some times with a not running
> indication. To speed things up we stop CPU1
> 
> Also rename smp_cpu_running to smp_sense_running_status.

Thanks for fixing this, one nit below.

Reviewed-by: Janosch Frank <frankja@linux.ibm.com>


> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  lib/s390x/smp.c |  2 +-
>  lib/s390x/smp.h |  2 +-
>  s390x/smp.c     | 15 +++++++++++++++
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/s390x/smp.c b/lib/s390x/smp.c
> index 5ed8b7b..492cb05 100644
> --- a/lib/s390x/smp.c
> +++ b/lib/s390x/smp.c
> @@ -58,7 +58,7 @@ bool smp_cpu_stopped(uint16_t addr)
>  	return !!(status & (SIGP_STATUS_CHECK_STOP|SIGP_STATUS_STOPPED));
>  }
>  
> -bool smp_cpu_running(uint16_t addr)
> +bool smp_sense_running_status(uint16_t addr)
>  {
>  	if (sigp(addr, SIGP_SENSE_RUNNING, 0, NULL) != SIGP_CC_STATUS_STORED)
>  		return true;
> diff --git a/lib/s390x/smp.h b/lib/s390x/smp.h
> index a8b98c0..639ec92 100644
> --- a/lib/s390x/smp.h
> +++ b/lib/s390x/smp.h
> @@ -40,7 +40,7 @@ struct cpu_status {
>  int smp_query_num_cpus(void);
>  struct cpu *smp_cpu_from_addr(uint16_t addr);
>  bool smp_cpu_stopped(uint16_t addr);
> -bool smp_cpu_running(uint16_t addr);
> +bool smp_sense_running_status(uint16_t addr);
>  int smp_cpu_restart(uint16_t addr);
>  int smp_cpu_start(uint16_t addr, struct psw psw);
>  int smp_cpu_stop(uint16_t addr);
> diff --git a/s390x/smp.c b/s390x/smp.c
> index 79cdc1f..4450aff 100644
> --- a/s390x/smp.c
> +++ b/s390x/smp.c
> @@ -210,6 +210,20 @@ static void test_emcall(void)
>  	report_prefix_pop();
>  }
>  
> +static void test_sense_running(void)
> +{
> +	report_prefix_push("sense_running");
> +	/* we are running */
> +	report(smp_sense_running_status(0), "CPU0 sense claims running");
> +	/* make sure CPU is stopped to speed up the not running case */
> +	smp_cpu_stop(1);
> +	/* Make sure to have at least one time with a not running indication */
> +	while(smp_sense_running_status(1));
> +	report(true, "CPU1 sense claims not running");
> +	report_prefix_pop();
> +}
> +
> +

One \n should be enough

>  /* Used to dirty registers of cpu #1 before it is reset */
>  static void test_func_initial(void)
>  {
> @@ -319,6 +333,7 @@ int main(void)
>  	test_store_status();
>  	test_ecall();
>  	test_emcall();
> +	test_sense_running();
>  	test_reset();
>  	test_reset_initial();
>  	smp_cpu_destroy(1);
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [kvm-unit-tests v3] s390x/smp: add minimal test for sigp sense running status

[David Hildenbrand]

On 02.04.20 18:19, Cornelia Huck wrote:
> On Thu,  2 Apr 2020 11:44:41 -0400
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> Two minimal tests:
>> - our own CPU should be running when we check ourselves
>> - a CPU should at least have some times with a not running
>> indication. To speed things up we stop CPU1
>>
>> Also rename smp_cpu_running to smp_sense_running_status.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>>  lib/s390x/smp.c |  2 +-
>>  lib/s390x/smp.h |  2 +-
>>  s390x/smp.c     | 15 +++++++++++++++
>>  3 files changed, 17 insertions(+), 2 deletions(-)
>>
> 
> (...)
> 
>> diff --git a/s390x/smp.c b/s390x/smp.c
>> index 79cdc1f..4450aff 100644
>> --- a/s390x/smp.c
>> +++ b/s390x/smp.c
>> @@ -210,6 +210,20 @@ static void test_emcall(void)
>>  	report_prefix_pop();
>>  }
>>  
>> +static void test_sense_running(void)
>> +{
>> +	report_prefix_push("sense_running");
>> +	/* we are running */
> 
> Maybe /* we (CPU0) are running */ ?

I fixed that one up

> 
>> +	report(smp_sense_running_status(0), "CPU0 sense claims running");
>> +	/* make sure CPU is stopped to speed up the not running case */
> 
> "the target CPU" ?

I'm using

"stop the target CPU (CPU1) to speed up the not running case"

to make it fit into a single line :)


(Christian, correct me if you disagree with these changes)


-- 
Thanks,

David / dhildenb