Re: [PULL 00/15] Host Memory Backends and Memory devices queue 2024-12-18

[David Hildenbrand <david@redhat.com>]

On 19.12.24 13:20, Thomas Huth wrote:
> On 19/12/2024 12.57, David Hildenbrand wrote:
>> On 19.12.24 12:43, Christian Borntraeger wrote:
>>> Am 19.12.24 um 12:18 schrieb David Hildenbrand:
>>>> The following on top seems to make everything happy. I wish the
>>>> CONFIG_S390_CCW_VIRTIO stuff would't have to be so complicated, just to
>>>> handle odd configs that don't really make sense.
>>>
>>> WOuld it be possible to rid of this config?
>>
>> I was asking myself the same: when does it make sense to build for s390x
>> system without CONFIG_S390_CCW_VIRTIO. But other archs that also have a
>> single machine seem to be doing the same thing.
>>
>> We wouldn't want to have the option to disable it, but "bool" gives you the
>> option to do that.
> 
> Since a while, (almost) all targets can be compiled without any machine
> except for the "none" machine, so I think we should not diverge in the s390x
> just for the sake of it.

Well, okay, although such a qemu-system-s390x is of questionable use. :)

At least hypercalls/skeys/stattrib only applies to the CONFIG_S390_CCW_VIRTIO
machine, so maybe more files can be moved under CONFIG_S390_CCW_VIRTIO later.

Anyhow, sounds like a bigger cleanup. The following on top seems to
make gitlab CI happy, without messing with config options:


diff --git a/hw/s390x/meson.build b/hw/s390x/meson.build
index 094435cd3b..3bbebfd817 100644
--- a/hw/s390x/meson.build
+++ b/hw/s390x/meson.build
@@ -12,7 +12,6 @@ s390x_ss.add(files(
    's390-pci-inst.c',
    's390-skeys.c',
    's390-stattrib.c',
-  's390-hypercall.c',
    'sclp.c',
    'sclpcpu.c',
    'sclpquiesce.c',
@@ -28,7 +27,10 @@ s390x_ss.add(when: 'CONFIG_KVM', if_true: files(
  s390x_ss.add(when: 'CONFIG_TCG', if_true: files(
    'tod-tcg.c',
  ))
-s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files('s390-virtio-ccw.c'))
+s390x_ss.add(when: 'CONFIG_S390_CCW_VIRTIO', if_true: files(
+  's390-virtio-ccw.c',
+  's390-hypercall.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'))
  
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 248566f8dc..097ec78826 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -127,7 +127,7 @@ static void subsystem_reset(void)
  static void s390_set_memory_limit(S390CcwMachineState *s390ms,
                                    uint64_t new_limit)
  {
-    uint64_t hw_limit;
+    uint64_t hw_limit = 0;
      int ret = 0;
  
      assert(!s390ms->memory_limit && new_limit);
@@ -145,13 +145,6 @@ static void s390_set_memory_limit(S390CcwMachineState *s390ms,
      s390ms->memory_limit = new_limit;
  }
  
-uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms)
-{
-    /* We expect to be called only after the limit was set. */
-    assert(s390ms->memory_limit);
-    return s390ms->memory_limit;
-}
-
  static void s390_set_max_pagesize(S390CcwMachineState *s390ms,
                                    uint64_t pagesize)
  {
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index 5a730f5d07..599740a998 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -35,7 +35,12 @@ struct S390CcwMachineState {
      SCLPDevice *sclp;
  };
  
-uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms);
+static inline uint64_t s390_get_memory_limit(S390CcwMachineState *s390ms)
+{
+    /* We expect to be called only after the limit was set. */
+    assert(s390ms->memory_limit);
+    return s390ms->memory_limit;
+}
  
  #define S390_PTF_REASON_NONE (0x00 << 8)
  #define S390_PTF_REASON_DONE (0x01 << 8)
diff --git a/target/s390x/tcg/misc_helper.c b/target/s390x/tcg/misc_helper.c
index 3732d79185..8002b1e2d0 100644
--- a/target/s390x/tcg/misc_helper.c
+++ b/target/s390x/tcg/misc_helper.c
@@ -43,6 +43,7 @@
  #include "hw/s390x/s390-pci-inst.h"
  #include "hw/boards.h"
  #include "hw/s390x/tod.h"
+#include CONFIG_DEVICES
  #endif
  
  /* #define DEBUG_HELPER */
@@ -117,12 +118,14 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
  
      switch (num) {
+#ifdef CONFIG_S390_CCW_VIRTIO
      case 0x500:
          /* QEMU/KVM hypercall */
          bql_lock();
          handle_diag_500(env_archcpu(env), GETPC());
          bql_unlock();
          r = 0;
          break;
+#endif /* CONFIG_S390_CCW_VIRTIO */
      case 0x44:
          /* yield */
          r = 0;

-- 
Cheers,

David / dhildenb