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

[David Hildenbrand]

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