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

[Claudio Fontana]

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
>