[PATCH 1/5] s390x: Create include files for s390x IPL definitions

jrossi@linux.ibm.com posted 5 patches 5 months, 4 weeks ago
Maintainers: Thomas Huth <thuth@redhat.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Eric Farman <farman@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>
There is a newer version of this series
[PATCH 1/5] s390x: Create include files for s390x IPL definitions
Posted by jrossi@linux.ibm.com 5 months, 4 weeks ago
From: Jared Rossi <jrossi@linux.ibm.com>

Currently, stuctures defined in both hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h
must be kept in sync, which is prone to error. Instead, create a new directory
at include/hw/s390x/ipl/ to contain the definitions that must be shared.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 hw/s390x/ipl.h              | 113 +-------------------------------
 include/hw/s390x/ipl/qipl.h | 126 ++++++++++++++++++++++++++++++++++++
 pc-bios/s390-ccw/iplb.h     |  84 ++----------------------
 pc-bios/s390-ccw/Makefile   |   2 +-
 4 files changed, 133 insertions(+), 192 deletions(-)
 create mode 100644 include/hw/s390x/ipl/qipl.h

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 57cd125769..b066d9e8e5 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -16,95 +16,11 @@
 #include "cpu.h"
 #include "exec/address-spaces.h"
 #include "hw/qdev-core.h"
+#include "hw/s390x/ipl/qipl.h"
 #include "qom/object.h"
 
-struct IPLBlockPVComp {
-    uint64_t tweak_pref;
-    uint64_t addr;
-    uint64_t size;
-} QEMU_PACKED;
-typedef struct IPLBlockPVComp IPLBlockPVComp;
-
-struct IPLBlockPV {
-    uint8_t  reserved18[87];    /* 0x18 */
-    uint8_t  version;           /* 0x6f */
-    uint32_t reserved70;        /* 0x70 */
-    uint32_t num_comp;          /* 0x74 */
-    uint64_t pv_header_addr;    /* 0x78 */
-    uint64_t pv_header_len;     /* 0x80 */
-    struct IPLBlockPVComp components[0];
-} QEMU_PACKED;
-typedef struct IPLBlockPV IPLBlockPV;
-
-struct IplBlockCcw {
-    uint8_t  reserved0[85];
-    uint8_t  ssid;
-    uint16_t devno;
-    uint8_t  vm_flags;
-    uint8_t  reserved3[3];
-    uint32_t vm_parm_len;
-    uint8_t  nss_name[8];
-    uint8_t  vm_parm[64];
-    uint8_t  reserved4[8];
-} QEMU_PACKED;
-typedef struct IplBlockCcw IplBlockCcw;
-
-struct IplBlockFcp {
-    uint8_t  reserved1[305 - 1];
-    uint8_t  opt;
-    uint8_t  reserved2[3];
-    uint16_t reserved3;
-    uint16_t devno;
-    uint8_t  reserved4[4];
-    uint64_t wwpn;
-    uint64_t lun;
-    uint32_t bootprog;
-    uint8_t  reserved5[12];
-    uint64_t br_lba;
-    uint32_t scp_data_len;
-    uint8_t  reserved6[260];
-    uint8_t  scp_data[0];
-} QEMU_PACKED;
-typedef struct IplBlockFcp IplBlockFcp;
-
-struct IplBlockQemuScsi {
-    uint32_t lun;
-    uint16_t target;
-    uint16_t channel;
-    uint8_t  reserved0[77];
-    uint8_t  ssid;
-    uint16_t devno;
-} QEMU_PACKED;
-typedef struct IplBlockQemuScsi IplBlockQemuScsi;
-
 #define DIAG308_FLAGS_LP_VALID 0x80
 
-union IplParameterBlock {
-    struct {
-        uint32_t len;
-        uint8_t  reserved0[3];
-        uint8_t  version;
-        uint32_t blk0_len;
-        uint8_t  pbt;
-        uint8_t  flags;
-        uint16_t reserved01;
-        uint8_t  loadparm[8];
-        union {
-            IplBlockCcw ccw;
-            IplBlockFcp fcp;
-            IPLBlockPV pv;
-            IplBlockQemuScsi scsi;
-        };
-    } QEMU_PACKED;
-    struct {
-        uint8_t  reserved1[110];
-        uint16_t devno;
-        uint8_t  reserved2[88];
-        uint8_t  reserved_ext[4096 - 200];
-    } QEMU_PACKED;
-} QEMU_PACKED;
-typedef union IplParameterBlock IplParameterBlock;
-
 int s390_ipl_set_loadparm(uint8_t *loadparm);
 void s390_ipl_update_diag308(IplParameterBlock *iplb);
 int s390_ipl_prepare_pv_header(Error **errp);
@@ -125,33 +41,6 @@ void s390_ipl_reset_request(CPUState *cs, enum s390_reset reset_type);
 void s390_ipl_get_reset_request(CPUState **cs, enum s390_reset *reset_type);
 void s390_ipl_clear_reset_request(void);
 
-#define QIPL_ADDRESS  0xcc
-
-/* Boot Menu flags */
-#define QIPL_FLAG_BM_OPTS_CMD   0x80
-#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
-
-/*
- * The QEMU IPL Parameters will be stored at absolute address
- * 204 (0xcc) which means it is 32-bit word aligned but not
- * double-word aligned.
- * Placement of data fields in this area must account for
- * their alignment needs. E.g., netboot_start_address must
- * have an offset of 4 + n * 8 bytes within the struct in order
- * to keep it double-word aligned.
- * The total size of the struct must never exceed 28 bytes.
- * This definition must be kept in sync with the definition
- * in pc-bios/s390-ccw/iplb.h.
- */
-struct QemuIplParameters {
-    uint8_t  qipl_flags;
-    uint8_t  reserved1[3];
-    uint64_t netboot_start_addr;
-    uint32_t boot_menu_timeout;
-    uint8_t  reserved2[12];
-} QEMU_PACKED;
-typedef struct QemuIplParameters QemuIplParameters;
-
 #define TYPE_S390_IPL "s390-ipl"
 OBJECT_DECLARE_SIMPLE_TYPE(S390IPLState, S390_IPL)
 
diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
new file mode 100644
index 0000000000..a6ce6ddfe3
--- /dev/null
+++ b/include/hw/s390x/ipl/qipl.h
@@ -0,0 +1,126 @@
+/*
+ * S/390 boot structures
+ *
+ * Copyright 2024 IBM Corp.
+ * Author(s): Jared Rossi <jrossi@linux.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#ifndef S390X_QIPL_H
+#define S390X_QIPL_H
+
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD   0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
+
+#define QIPL_ADDRESS  0xcc
+#define LOADPARM_LEN    8
+
+/*
+ * The QEMU IPL Parameters will be stored at absolute address
+ * 204 (0xcc) which means it is 32-bit word aligned but not
+ * double-word aligned.
+ * Placement of data fields in this area must account for
+ * their alignment needs. E.g., netboot_start_address must
+ * have an offset of 4 + n * 8 bytes within the struct in order
+ * to keep it double-word aligned.
+ * The total size of the struct must never exceed 28 bytes.
+ */
+struct QemuIplParameters {
+    uint8_t  qipl_flags;
+    uint8_t  reserved1[3];
+    uint64_t netboot_start_addr;
+    uint32_t boot_menu_timeout;
+    uint8_t  reserved2[12];
+}  QEMU_PACKED;
+typedef struct QemuIplParameters QemuIplParameters;
+
+struct IPLBlockPVComp {
+    uint64_t tweak_pref;
+    uint64_t addr;
+    uint64_t size;
+}  QEMU_PACKED;
+typedef struct IPLBlockPVComp IPLBlockPVComp;
+
+struct IPLBlockPV {
+    uint8_t  reserved18[87];    /* 0x18 */
+    uint8_t  version;           /* 0x6f */
+    uint32_t reserved70;        /* 0x70 */
+    uint32_t num_comp;          /* 0x74 */
+    uint64_t pv_header_addr;    /* 0x78 */
+    uint64_t pv_header_len;     /* 0x80 */
+    struct IPLBlockPVComp components[0];
+}  QEMU_PACKED;
+typedef struct IPLBlockPV IPLBlockPV;
+
+struct IplBlockCcw {
+    uint8_t  reserved0[85];
+    uint8_t  ssid;
+    uint16_t devno;
+    uint8_t  vm_flags;
+    uint8_t  reserved3[3];
+    uint32_t vm_parm_len;
+    uint8_t  nss_name[8];
+    uint8_t  vm_parm[64];
+    uint8_t  reserved4[8];
+}  QEMU_PACKED;
+typedef struct IplBlockCcw IplBlockCcw;
+
+struct IplBlockFcp {
+    uint8_t  reserved1[305 - 1];
+    uint8_t  opt;
+    uint8_t  reserved2[3];
+    uint16_t reserved3;
+    uint16_t devno;
+    uint8_t  reserved4[4];
+    uint64_t wwpn;
+    uint64_t lun;
+    uint32_t bootprog;
+    uint8_t  reserved5[12];
+    uint64_t br_lba;
+    uint32_t scp_data_len;
+    uint8_t  reserved6[260];
+    uint8_t  scp_data[0];
+}  QEMU_PACKED;
+typedef struct IplBlockFcp IplBlockFcp;
+
+struct IplBlockQemuScsi {
+    uint32_t lun;
+    uint16_t target;
+    uint16_t channel;
+    uint8_t  reserved0[77];
+    uint8_t  ssid;
+    uint16_t devno;
+}  QEMU_PACKED;
+typedef struct IplBlockQemuScsi IplBlockQemuScsi;
+
+union IplParameterBlock {
+    struct {
+        uint32_t len;
+        uint8_t  reserved0[3];
+        uint8_t  version;
+        uint32_t blk0_len;
+        uint8_t  pbt;
+        uint8_t  flags;
+        uint16_t reserved01;
+        uint8_t  loadparm[LOADPARM_LEN];
+        union {
+            IplBlockCcw ccw;
+            IplBlockFcp fcp;
+            IPLBlockPV pv;
+            IplBlockQemuScsi scsi;
+        };
+    }  QEMU_PACKED;
+    struct {
+        uint8_t  reserved1[110];
+        uint16_t devno;
+        uint8_t  reserved2[88];
+        uint8_t  reserved_ext[4096 - 200];
+    }  QEMU_PACKED;
+}  QEMU_PACKED;
+typedef union IplParameterBlock IplParameterBlock;
+
+#endif
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index cb6ac8a880..16643f5879 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -12,88 +12,14 @@
 #ifndef IPLB_H
 #define IPLB_H
 
-#define LOADPARM_LEN    8
+#ifndef QEMU_PACKED
+#define QEMU_PACKED __attribute__((packed))
+#endif
 
-struct IplBlockCcw {
-    uint8_t  reserved0[85];
-    uint8_t  ssid;
-    uint16_t devno;
-    uint8_t  vm_flags;
-    uint8_t  reserved3[3];
-    uint32_t vm_parm_len;
-    uint8_t  nss_name[8];
-    uint8_t  vm_parm[64];
-    uint8_t  reserved4[8];
-} __attribute__ ((packed));
-typedef struct IplBlockCcw IplBlockCcw;
-
-struct IplBlockFcp {
-    uint8_t  reserved1[305 - 1];
-    uint8_t  opt;
-    uint8_t  reserved2[3];
-    uint16_t reserved3;
-    uint16_t devno;
-    uint8_t  reserved4[4];
-    uint64_t wwpn;
-    uint64_t lun;
-    uint32_t bootprog;
-    uint8_t  reserved5[12];
-    uint64_t br_lba;
-    uint32_t scp_data_len;
-    uint8_t  reserved6[260];
-    uint8_t  scp_data[];
-} __attribute__ ((packed));
-typedef struct IplBlockFcp IplBlockFcp;
-
-struct IplBlockQemuScsi {
-    uint32_t lun;
-    uint16_t target;
-    uint16_t channel;
-    uint8_t  reserved0[77];
-    uint8_t  ssid;
-    uint16_t devno;
-} __attribute__ ((packed));
-typedef struct IplBlockQemuScsi IplBlockQemuScsi;
-
-struct IplParameterBlock {
-    uint32_t len;
-    uint8_t  reserved0[3];
-    uint8_t  version;
-    uint32_t blk0_len;
-    uint8_t  pbt;
-    uint8_t  flags;
-    uint16_t reserved01;
-    uint8_t  loadparm[LOADPARM_LEN];
-    union {
-        IplBlockCcw ccw;
-        IplBlockFcp fcp;
-        IplBlockQemuScsi scsi;
-    };
-} __attribute__ ((packed));
-typedef struct IplParameterBlock IplParameterBlock;
-
-extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
-
-#define QIPL_ADDRESS  0xcc
-
-/* Boot Menu flags */
-#define QIPL_FLAG_BM_OPTS_CMD   0x80
-#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
-
-/*
- * This definition must be kept in sync with the definition
- * in hw/s390x/ipl.h
- */
-struct QemuIplParameters {
-    uint8_t  qipl_flags;
-    uint8_t  reserved1[3];
-    uint64_t netboot_start_addr;
-    uint32_t boot_menu_timeout;
-    uint8_t  reserved2[12];
-} __attribute__ ((packed));
-typedef struct QemuIplParameters QemuIplParameters;
+#include <qipl.h>
 
 extern QemuIplParameters qipl;
+extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
 
 #define S390_IPL_TYPE_FCP 0x00
 #define S390_IPL_TYPE_CCW 0x02
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index acfcd1e71a..a771439acf 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -3,7 +3,7 @@ all: build-all
 	@true
 
 include config-host.mak
-CFLAGS = -O2 -g
+CFLAGS = -O2 -g -I $(SRC_PATH)/../..//include/hw/s390x/ipl
 MAKEFLAGS += -rR
 
 GIT_SUBMODULES = roms/SLOF
-- 
2.45.1
Re: [PATCH 1/5] s390x: Create include files for s390x IPL definitions
Posted by Thomas Huth 5 months, 3 weeks ago
  Hi Jared!

On 29/05/2024 17.43, jrossi@linux.ibm.com wrote:
> From: Jared Rossi <jrossi@linux.ibm.com>
> 
> Currently, stuctures defined in both hw/s390x/ipl.h and pc-bios/s390-ccw/iplb.h

Typo: s/stuctures/structures/

> must be kept in sync, which is prone to error. Instead, create a new directory
> at include/hw/s390x/ipl/ to contain the definitions that must be shared.
> 
> Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
> ---
>   hw/s390x/ipl.h              | 113 +-------------------------------
>   include/hw/s390x/ipl/qipl.h | 126 ++++++++++++++++++++++++++++++++++++
>   pc-bios/s390-ccw/iplb.h     |  84 ++----------------------
>   pc-bios/s390-ccw/Makefile   |   2 +-
>   4 files changed, 133 insertions(+), 192 deletions(-)
>   create mode 100644 include/hw/s390x/ipl/qipl.h
...
> diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
> index acfcd1e71a..a771439acf 100644
> --- a/pc-bios/s390-ccw/Makefile
> +++ b/pc-bios/s390-ccw/Makefile
> @@ -3,7 +3,7 @@ all: build-all
>   	@true
>   
>   include config-host.mak
> -CFLAGS = -O2 -g
> +CFLAGS = -O2 -g -I $(SRC_PATH)/../..//include/hw/s390x/ipl

Duplicate slash -----------------------^

Apart from these two nits, patch looks fine to me.

  Thomas