[PATCH 15/18] target/s390x: Un-inline s390_is_pv()

Philippe Mathieu-Daudé posted 18 patches 1 month ago
Maintainers: Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Farhan Ali <alifm@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>
There is a newer version of this series
[PATCH 15/18] target/s390x: Un-inline s390_is_pv()
Posted by Philippe Mathieu-Daudé 1 month ago
Inlining a method which use a static variable is really a
bad idea, as it totally defeats the point of both concepts.

Currently we have 12 + 4 = 16 static 'ccw' variables...:

  $ git grep -wl target/s390x/kvm/pv.h | fgrep .h
  hw/s390x/ipl.h
  $ git grep -wl target/s390x/kvm/pv.h | fgrep .c | wc -l
        12
  $ git grep -wl hw/s390x/ipl.h | fgrep .c | wc -l
         4

Fixes: c3347ed0d2e ("s390x: protvirt: Support unpack facility")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/s390x/kvm/pv.h | 24 +-----------------------
 target/s390x/kvm/pv.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 23 deletions(-)

diff --git a/target/s390x/kvm/pv.h b/target/s390x/kvm/pv.h
index 94e885e9335..e266fc3d545 100644
--- a/target/s390x/kvm/pv.h
+++ b/target/s390x/kvm/pv.h
@@ -12,8 +12,6 @@
 #ifndef HW_S390_PV_H
 #define HW_S390_PV_H
 
-#include "qapi/error.h"
-#include "system/kvm.h"
 #include "hw/s390x/s390-virtio-ccw.h"
 
 struct S390PVResponse {
@@ -23,27 +21,7 @@ struct S390PVResponse {
 };
 
 #ifdef CONFIG_KVM
-#include "cpu.h"
-
-static inline bool s390_is_pv(void)
-{
-    static S390CcwMachineState *ccw;
-    Object *obj;
-
-    if (ccw) {
-        return ccw->pv;
-    }
-
-    /* we have to bail out for the "none" machine */
-    obj = object_dynamic_cast(qdev_get_machine(),
-                              TYPE_S390_CCW_MACHINE);
-    if (!obj) {
-        return false;
-    }
-    ccw = S390_CCW_MACHINE(obj);
-    return ccw->pv;
-}
-
+bool s390_is_pv(void);
 int s390_pv_query_info(void);
 int s390_pv_vm_enable(void);
 void s390_pv_vm_disable(void);
diff --git a/target/s390x/kvm/pv.c b/target/s390x/kvm/pv.c
index 2bc916a5455..3d508165f34 100644
--- a/target/s390x/kvm/pv.c
+++ b/target/s390x/kvm/pv.c
@@ -25,6 +25,24 @@
 #include "target/s390x/kvm/kvm_s390x.h"
 #include "target/s390x/kvm/pv.h"
 
+bool s390_is_pv(void)
+{
+    static S390CcwMachineState *ccw;
+    Object *obj;
+
+    if (ccw) {
+        return ccw->pv;
+    }
+
+    /* we have to bail out for the "none" machine */
+    obj = object_dynamic_cast(qdev_get_machine(), TYPE_S390_CCW_MACHINE);
+    if (!obj) {
+        return false;
+    }
+    ccw = S390_CCW_MACHINE(obj);
+    return ccw->pv;
+}
+
 static bool info_valid;
 static struct kvm_s390_pv_info_vm info_vm;
 static struct kvm_s390_pv_info_dump info_dump;
-- 
2.52.0


Re: [PATCH 15/18] target/s390x: Un-inline s390_is_pv()
Posted by Thomas Huth 1 month ago
On 07/01/2026 14.08, Philippe Mathieu-Daudé wrote:
> Inlining a method which use a static variable is really a
> bad idea, as it totally defeats the point of both concepts.
> 
> Currently we have 12 + 4 = 16 static 'ccw' variables...:
> 
>    $ git grep -wl target/s390x/kvm/pv.h | fgrep .h
>    hw/s390x/ipl.h
>    $ git grep -wl target/s390x/kvm/pv.h | fgrep .c | wc -l
>          12
>    $ git grep -wl hw/s390x/ipl.h | fgrep .c | wc -l
>           4
> 
> Fixes: c3347ed0d2e ("s390x: protvirt: Support unpack facility")
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/s390x/kvm/pv.h | 24 +-----------------------
>   target/s390x/kvm/pv.c | 18 ++++++++++++++++++
>   2 files changed, 19 insertions(+), 23 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>