Re: [RFC v6 04/13] hw/s390x: tod: make explicit checks for accelerators when initializing

[Al Cho via]

ok, will swap 03 and 04 in next version.

Thanks,
AL

From: Thomas Huth 
Sent: Thursday, July 1, 2021 6:32 PM
To: Al Cho ; qemu-devel@nongnu.org ; 
qemu-s3...@nongnu.org 
Cc: Claudio Fontana ; José Ricardo Ziviani 
; Claudio Fontana ; David Hildenbrand 
; Cornelia Huck 
Subject: Re: [RFC v6 04/13] hw/s390x: tod: make explicit checks for 
accelerators when initializing

On 29/06/2021 16.19, Cho, Yu-Chen wrote:
> replace general "else" with specific checks for each possible accelerator.
>
> Handle qtest as a NOP, and error out for an unknown accelerator used in
> combination with tod.
>
> Signed-off-by: Claudio Fontana 
> Reviewed-by: David Hildenbrand 
> Reviewed-by: Cornelia Huck 
> Signed-off-by: Cho, Yu-Chen 
> ---
>   hw/s390x/tod.c | 9 -
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
> index 3c2979175e..fd5a36bf24 100644
> --- a/hw/s390x/tod.c
> +++ b/hw/s390x/tod.c
> @@ -14,6 +14,8 @@
>   #include "qemu/error-report.h"
>   #include "qemu/module.h"
>   #include "sysemu/kvm.h"
> +#include "sysemu/tcg.h"
> +#include "sysemu/qtest.h"
>   #include "migration/qemu-file-types.h"
>   #include "migration/register.h"
>
> @@ -23,8 +25,13 @@ void s390_init_tod(void)
>
>   if (kvm_enabled()) {
>   obj = object_new(TYPE_KVM_S390_TOD);
> -} else {
> +} else if (tcg_enabled()) {
>   obj = object_new(TYPE_QEMU_S390_TOD);
> +} else if (qtest_enabled()) {
> +return;
> +} else {
> +error_report("current accelerator not handled in s390_init_tod!");
> +abort();
>   }
>   object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>   object_unref(obj);
>

I think it might be better to swap the order of patch 03 and 04, to avoid
that the qtests might break during bisecting later.

For this patch itself:
Reviewed-by: Thomas Huth 

Re: [RFC v6 04/13] hw/s390x: tod: make explicit checks for accelerators when initializing

[Thomas Huth]

On 29/06/2021 16.19, Cho, Yu-Chen wrote:

replace general "else" with specific checks for each possible accelerator.

Handle qtest as a NOP, and error out for an unknown accelerator used in
combination with tod.

Signed-off-by: Claudio Fontana 
Reviewed-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
Signed-off-by: Cho, Yu-Chen 
---
  hw/s390x/tod.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
index 3c2979175e..fd5a36bf24 100644
--- a/hw/s390x/tod.c
+++ b/hw/s390x/tod.c
@@ -14,6 +14,8 @@
  #include "qemu/error-report.h"
  #include "qemu/module.h"
  #include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
  #include "migration/qemu-file-types.h"
  #include "migration/register.h"
  
@@ -23,8 +25,13 @@ void s390_init_tod(void)
  
  if (kvm_enabled()) {

  obj = object_new(TYPE_KVM_S390_TOD);
-} else {
+} else if (tcg_enabled()) {
  obj = object_new(TYPE_QEMU_S390_TOD);
+} else if (qtest_enabled()) {
+return;
+} else {
+error_report("current accelerator not handled in s390_init_tod!");
+abort();
  }
  object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
  object_unref(obj);



I think it might be better to swap the order of patch 03 and 04, to avoid 
that the qtests might break during bisecting later.


For this patch itself:
Reviewed-by: Thomas Huth 

[RFC v6 04/13] hw/s390x: tod: make explicit checks for accelerators when initializing

[Cho, Yu-Chen]

replace general "else" with specific checks for each possible accelerator.

Handle qtest as a NOP, and error out for an unknown accelerator used in
combination with tod.

Signed-off-by: Claudio Fontana 
Reviewed-by: David Hildenbrand 
Reviewed-by: Cornelia Huck 
Signed-off-by: Cho, Yu-Chen 
---
 hw/s390x/tod.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
index 3c2979175e..fd5a36bf24 100644
--- a/hw/s390x/tod.c
+++ b/hw/s390x/tod.c
@@ -14,6 +14,8 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "sysemu/kvm.h"
+#include "sysemu/tcg.h"
+#include "sysemu/qtest.h"
 #include "migration/qemu-file-types.h"
 #include "migration/register.h"
 
@@ -23,8 +25,13 @@ void s390_init_tod(void)
 
 if (kvm_enabled()) {
 obj = object_new(TYPE_KVM_S390_TOD);
-} else {
+} else if (tcg_enabled()) {
 obj = object_new(TYPE_QEMU_S390_TOD);
+} else if (qtest_enabled()) {
+return;
+} else {
+error_report("current accelerator not handled in s390_init_tod!");
+abort();
 }
 object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
 object_unref(obj);
-- 
2.32.0