1. Patchew
  2. QEMU
  3. [RFC v1 0/5] s390x cleanup
  4. View patch

[RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build

Claudio Fontana posted 5 patches 4 years, 10 months ago
There is a newer version of this series
[RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Claudio Fontana 4 years, 10 months ago 
this allows to remove unneeded stubs for target/s390x.

Signed-off-by: Claudio Fontana <cfontana@suse.de>
---
 hw/s390x/tod.c       | 9 ++++++++-
 hw/s390x/meson.build | 5 ++++-
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
index 3c2979175e..322732d7fd 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 {
+        warn_report("current accelerator not handled in s390_init_tod!");
+        return;
     }
     object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
     object_unref(obj);
diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 91495b5631..7f31f9e5d5 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -16,7 +16,6 @@ s390x_ss.add(files(
   'sclp.c',
   'sclpcpu.c',
   'sclpquiesce.c',
-  'tod-qemu.c',
   'tod.c',
 ))
 s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
@@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
   's390-stattrib-kvm.c',
   'pv.c',
 ))
+s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
+  'tod-qemu.c',
+))
+
 s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
 s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
 s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
-- 
2.26.2
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Cornelia Huck 4 years, 10 months ago 
On Mon, 22 Mar 2021 20:15:47 +0100
Claudio Fontana <cfontana@suse.de> wrote:

> this allows to remove unneeded stubs for target/s390x.

This patch doesn't seem to remove any, though?

> 
> Signed-off-by: Claudio Fontana <cfontana@suse.de>
> ---
>  hw/s390x/tod.c       | 9 ++++++++-
>  hw/s390x/meson.build | 5 ++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
> index 3c2979175e..322732d7fd 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 {
> +        warn_report("current accelerator not handled in s390_init_tod!");
> +        return;

I'm wondering whether this should be a fatal error.

>      }
>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>      object_unref(obj);
> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
> index 91495b5631..7f31f9e5d5 100644
> --- a/hw/s390x/meson.build
> +++ b/hw/s390x/meson.build
> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>    'sclp.c',
>    'sclpcpu.c',
>    'sclpquiesce.c',
> -  'tod-qemu.c',
>    'tod.c',
>  ))
>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>    's390-stattrib-kvm.c',
>    'pv.c',
>  ))
> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> +  'tod-qemu.c',

Should we rename this to tod-tcg.c?

> +))
> +
>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Claudio Fontana 4 years, 9 months ago 
Hi Cornelia,

On 3/31/21 1:07 PM, Cornelia Huck wrote:
> On Mon, 22 Mar 2021 20:15:47 +0100
> Claudio Fontana <cfontana@suse.de> wrote:
> 
>> this allows to remove unneeded stubs for target/s390x.
> 
> This patch doesn't seem to remove any, though?

The next patch does... I'll split more the patches so it becomes clearer.

> 
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  hw/s390x/tod.c       | 9 ++++++++-
>>  hw/s390x/meson.build | 5 ++++-
>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
>> index 3c2979175e..322732d7fd 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 {
>> +        warn_report("current accelerator not handled in s390_init_tod!");
>> +        return;
> 
> I'm wondering whether this should be a fatal error.

I would agree with that.

> 
>>      }
>>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>>      object_unref(obj);
>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
>> index 91495b5631..7f31f9e5d5 100644
>> --- a/hw/s390x/meson.build
>> +++ b/hw/s390x/meson.build
>> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>>    'sclp.c',
>>    'sclpcpu.c',
>>    'sclpquiesce.c',
>> -  'tod-qemu.c',
>>    'tod.c',
>>  ))
>>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>    's390-stattrib-kvm.c',
>>    'pv.c',
>>  ))
>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>> +  'tod-qemu.c',
> 
> Should we rename this to tod-tcg.c?

I think so.

> 
>> +))
>> +
>>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
> 
> 

Will prepare a new version,

Thanks,

Claudio
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Claudio Fontana 4 years, 9 months ago 
On 4/19/21 11:11 AM, Claudio Fontana wrote:
> Hi Cornelia,
> 
> On 3/31/21 1:07 PM, Cornelia Huck wrote:
>> On Mon, 22 Mar 2021 20:15:47 +0100
>> Claudio Fontana <cfontana@suse.de> wrote:
>>
>>> this allows to remove unneeded stubs for target/s390x.
>>
>> This patch doesn't seem to remove any, though?
> 
> The next patch does... I'll split more the patches so it becomes clearer.
> 
>>
>>>
>>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>>> ---
>>>  hw/s390x/tod.c       | 9 ++++++++-
>>>  hw/s390x/meson.build | 5 ++++-
>>>  2 files changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/s390x/tod.c b/hw/s390x/tod.c
>>> index 3c2979175e..322732d7fd 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 {
>>> +        warn_report("current accelerator not handled in s390_init_tod!");
>>> +        return;
>>
>> I'm wondering whether this should be a fatal error.
> 
> I would agree with that.
> 
>>
>>>      }
>>>      object_property_add_child(qdev_get_machine(), TYPE_S390_TOD, obj);
>>>      object_unref(obj);
>>> diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
>>> index 91495b5631..7f31f9e5d5 100644
>>> --- a/hw/s390x/meson.build
>>> +++ b/hw/s390x/meson.build
>>> @@ -16,7 +16,6 @@ s390x_ss.add(files(
>>>    'sclp.c',
>>>    'sclpcpu.c',
>>>    'sclpquiesce.c',
>>> -  'tod-qemu.c',
>>>    'tod.c',
>>>  ))
>>>  s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>>    's390-stattrib-kvm.c',
>>>    'pv.c',
>>>  ))
>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>>> +  'tod-qemu.c',
>>
>> Should we rename this to tod-tcg.c?
> 
> I think so.

Here we are a bit limited though by the fact that the object is currently called:

include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"

So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:

#define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"

What do you think?


> 
>>
>>> +))
>>> +
>>>  s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
>>>  s390x_ss.add(when: 'CONFIG_TERMINAL3270', if_true: files('3270-ccw.c'))
>>>  s390x_ss.add(when: 'CONFIG_VFIO', if_true: files('s390-pci-vfio.c'))
>>
>>
> 
> Will prepare a new version,
> 
> Thanks,
> 
> Claudio
>
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Cornelia Huck 4 years, 9 months ago 
On Mon, 19 Apr 2021 18:12:48 +0200
Claudio Fontana <cfontana@suse.de> wrote:

> On 4/19/21 11:11 AM, Claudio Fontana wrote:
> > Hi Cornelia,
> > 
> > On 3/31/21 1:07 PM, Cornelia Huck wrote:  
> >> On Mon, 22 Mar 2021 20:15:47 +0100
> >> Claudio Fontana <cfontana@suse.de> wrote:

> >>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> >>>    's390-stattrib-kvm.c',
> >>>    'pv.c',
> >>>  ))
> >>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> >>> +  'tod-qemu.c',  
> >>
> >> Should we rename this to tod-tcg.c?  
> > 
> > I think so.  
> 
> Here we are a bit limited though by the fact that the object is currently called:
> 
> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
> 
> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
> 
> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
> 
> What do you think?

How visible is this? I don't think the TOD objects are instantiable by
the user.
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Claudio Fontana 4 years, 9 months ago 
On 4/19/21 6:20 PM, Cornelia Huck wrote:
> On Mon, 19 Apr 2021 18:12:48 +0200
> Claudio Fontana <cfontana@suse.de> wrote:
> 
>> On 4/19/21 11:11 AM, Claudio Fontana wrote:
>>> Hi Cornelia,
>>>
>>> On 3/31/21 1:07 PM, Cornelia Huck wrote:  
>>>> On Mon, 22 Mar 2021 20:15:47 +0100
>>>> Claudio Fontana <cfontana@suse.de> wrote:
> 
>>>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
>>>>>    's390-stattrib-kvm.c',
>>>>>    'pv.c',
>>>>>  ))
>>>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
>>>>> +  'tod-qemu.c',  
>>>>
>>>> Should we rename this to tod-tcg.c?  
>>>
>>> I think so.  
>>
>> Here we are a bit limited though by the fact that the object is currently called:
>>
>> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
>>
>> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
>>
>> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
>>
>> What do you think?
> 
> How visible is this? I don't think the TOD objects are instantiable by
> the user.
> 

I just remember we were very conservative with the object hierarchy on x86, personally I am fine with the change.
I will add this change then, I'd ask for people with concerns about this to speak up:

(Paolo?) 

Ciao,

CLaudio
Re: [RFC v1 1/5] hw/s390x: only build qemu-tod from the CONFIG_TCG build
Posted by Cornelia Huck 4 years, 9 months ago 
On Mon, 19 Apr 2021 18:24:34 +0200
Claudio Fontana <cfontana@suse.de> wrote:

> On 4/19/21 6:20 PM, Cornelia Huck wrote:
> > On Mon, 19 Apr 2021 18:12:48 +0200
> > Claudio Fontana <cfontana@suse.de> wrote:
> >   
> >> On 4/19/21 11:11 AM, Claudio Fontana wrote:  
> >>> Hi Cornelia,
> >>>
> >>> On 3/31/21 1:07 PM, Cornelia Huck wrote:    
> >>>> On Mon, 22 Mar 2021 20:15:47 +0100
> >>>> Claudio Fontana <cfontana@suse.de> wrote:  
> >   
> >>>>> @@ -25,6 +24,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
> >>>>>    's390-stattrib-kvm.c',
> >>>>>    'pv.c',
> >>>>>  ))
> >>>>> +s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
> >>>>> +  'tod-qemu.c',    
> >>>>
> >>>> Should we rename this to tod-tcg.c?    
> >>>
> >>> I think so.    
> >>
> >> Here we are a bit limited though by the fact that the object is currently called:
> >>
> >> include/hw/s390x/tod.h:26:#define TYPE_QEMU_S390_TOD TYPE_S390_TOD "-qemu"
> >>
> >> So there might be a compatibility issue in trying to make this consistent, which would mean to replace this with:
> >>
> >> #define TYPE_TCG_S390_TOD TYPE_S390_TOD "-tcg"
> >>
> >> What do you think?  
> > 
> > How visible is this? I don't think the TOD objects are instantiable by
> > the user.
> >   
> 
> I just remember we were very conservative with the object hierarchy on x86, personally I am fine with the change.
> I will add this change then, I'd ask for people with concerns about this to speak up:
> 
> (Paolo?) 
> 
> Ciao,
> 
> CLaudio
> 

It was more an argument against changing it, because most people won't
see it anyway :)

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.