On 04/03/2026 03.59, [email protected] wrote:
From: Jared Rossi <[email protected]>

Remove the duplicate definitions from hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h
and add a shared definition.  The new definition is an enum to enforce default
handling in switches.

Because the IPL type is determined by the IPLB, and because an IPLB is not
strictly necessary, the IPL type is set to a default value if not otherwise
specified.  A default IPL type is required so future functionality may add
IPL new bus and/or device types that dictate specific behavior during IPL.

Signed-off-by: Jared Rossi <[email protected]>
---
  hw/s390x/ipl.h              |  5 -----
  include/hw/s390x/ipl/qipl.h | 10 ++++++++++
  pc-bios/s390-ccw/iplb.h     |  4 ----
  pc-bios/s390-ccw/main.c     |  9 +++++++--
  pc-bios/s390-ccw/virtio.h   |  1 +
  5 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 086e57681c..c542d30ce2 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -103,11 +103,6 @@ QEMU_BUILD_BUG_MSG(offsetof(S390IPLState, iplb) & 3, "alignment 
of iplb wrong");
  #define DIAG308_PV_STORE            9
  #define DIAG308_PV_START            10
-#define S390_IPL_TYPE_FCP 0x00
-#define S390_IPL_TYPE_CCW 0x02
-#define S390_IPL_TYPE_PV 0x05
-#define S390_IPL_TYPE_QEMU_SCSI 0xff
-
  #define S390_IPLB_HEADER_LEN 8
  #define S390_IPLB_MIN_PV_LEN 148
  #define S390_IPLB_MIN_CCW_LEN 200
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
index 6824391111..6dc12dd859 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -20,6 +20,16 @@
  #define LOADPARM_LEN    8
  #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
+enum S390IplType {
+    S390_IPL_TYPE_FCP = 0x00,
+    S390_IPL_TYPE_CCW = 0x02,
+    S390_IPL_TYPE_PV = 0x05,
+    S390_IPL_TYPE_QEMU_SCSI = 0xff
+};
+typedef enum S390IplType S390IplType;
+
+#define QEMU_DEFAULT_IPL S390_IPL_TYPE_CCW
+
  /*
   * The QEMU IPL Parameters will be stored at absolute address
   * 204 (0xcc) which means it is 32-bit word aligned but not
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 08f259ff31..926e8eed5d 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -23,10 +23,6 @@ extern QemuIplParameters qipl;
  extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
  extern bool have_iplb;
-#define S390_IPL_TYPE_FCP 0x00
-#define S390_IPL_TYPE_CCW 0x02
-#define S390_IPL_TYPE_QEMU_SCSI 0xff
-
  static inline bool manage_iplb(IplParameterBlock *iplb, bool store)
  {
      register unsigned long addr asm("0") = (unsigned long) iplb;
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 8e2c99bee1..ef5acc1985 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -162,11 +162,12 @@ static void menu_setup(void)
          return;
      }
- switch (iplb.pbt) {
+    switch (virtio_get_device()->ipl_type) {

Cosmetics: Since you added a "vdev = virtio_get_device()" to boot_setup() and boot_setup() is the only caller of menu_setup(), you could also pass the vdev via parameter to menu_setup() now and get rid of the virtio_get_device() call here.

      case S390_IPL_TYPE_CCW:
      case S390_IPL_TYPE_QEMU_SCSI:
          menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
                         qipl.boot_menu_timeout);

I'm a little bit surprised that this does not trigger any "-Wimplicit-fallthrough" warnings ... ah, we only enable "-Wall" in the Makefile, but not "-Wimplicit-fallthrough" is not included in that switch.

Anyway, I'd suggest to either add another "return;" statement here, or a "/* fallthrough */" comment.

+    default:
          return;
      }
  }

With the nits fixed:
Reviewed-by: Thomas Huth <[email protected]>


Reply via email to