Re: [PATCH 02/10] pc-bios/s390-ccw: Store boot device type and bus separately

2026-01-07 Thread Thomas Huth 

On 10/12/2025 21.54, [email protected] wrote:

From: Jared Rossi 

Store both the device type (e.g. block) and device bus (e.g. CCW) to determine
IPL format rather than assume all devices can be identified by CCW specific
sense data.

Signed-off-by: Jared Rossi 
---

...
> diff --git a/include/hw/s390x/ipl/qipl.h b/include/hw/s390x/ipl/qipl.h
> index 682439..8199b839f0 100644
> --- a/include/hw/s390x/ipl/qipl.h
> +++ b/include/hw/s390x/ipl/qipl.h
> @@ -20,6 +20,13 @@
>   #define LOADPARM_LEN8
>   #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
>
> +#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 QEMU_DEFAULT_IPL S390_IPL_TYPE_CCW

Would it make sense to turn this into an enum as well?

>   /*
>* 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/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 4b819dd80f..f40a9407c2 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c

...

@@ -129,6 +129,8 @@ void virtio_assume_iso9660(void)
  case VIRTIO_ID_SCSI:
  vdev->scsi_block_size = VIRTIO_ISO_BLOCK_SIZE;
  break;
+default:
+return;
  }
  }
  
@@ -139,13 +141,15 @@ void virtio_assume_eckd(void)

  vdev->guessed_disk_nature = VIRTIO_GDN_DASD;
  vdev->blk_factor = 1;
  vdev->config.blk.physical_block_exp = 0;
-switch (vdev->senseid.cu_model) {
+switch (vdev->dev_type) {
  case VIRTIO_ID_BLOCK:
  vdev->config.blk.blk_size = VIRTIO_DASD_DEFAULT_BLOCK_SIZE;
  break;
  case VIRTIO_ID_SCSI:
  vdev->config.blk.blk_size = vdev->scsi_block_size;
  break;
+default:
+return;


This looks like errors could silently be ignored here. Maybe rather panic() 
here instead of returning? Or use "break" to maintain the old behavior?



  }
  vdev->config.blk.geometry.heads = 15;
  vdev->config.blk.geometry.sectors =


 Thomas

Re: [PATCH 02/10] pc-bios/s390-ccw: Store boot device type and bus separately

2026-01-06 Thread Eric Farman 
On Wed, 2025-12-10 at 15:54 -0500, [email protected] wrote:
> From: Jared Rossi 
> 
> Store both the device type (e.g. block) and device bus (e.g. CCW) to determine
> IPL format rather than assume all devices can be identified by CCW specific
> sense data.
> 
> Signed-off-by: Jared Rossi 
> ---
>  hw/s390x/ipl.h   |  5 
>  include/hw/s390x/ipl/qipl.h  |  7 ++
>  pc-bios/s390-ccw/iplb.h  |  4 
>  pc-bios/s390-ccw/virtio.h|  2 ++
>  pc-bios/s390-ccw/main.c  | 10 +---
>  pc-bios/s390-ccw/virtio-blkdev.c | 39 +++-
>  pc-bios/s390-ccw/virtio.c| 13 +++
>  7 files changed, 48 insertions(+), 32 deletions(-)

I might've suggested that this be broken up even further, to distinguish 
between the vdev->ipl_type
(nee iplb.pbt) and the vdev->dev_type (nee vdev->senseid.cu_model) changes, and 
maybe even the
consolidation of the S390_IPL_TYPE_* defines, to make it more obvious that 
things are just moving
around without any actual change. But, this is honestly fine.

Reviewed-by: Eric Farman

[PATCH 02/10] pc-bios/s390-ccw: Store boot device type and bus separately

2025-12-10 Thread jrossi 
From: Jared Rossi 

Store both the device type (e.g. block) and device bus (e.g. CCW) to determine
IPL format rather than assume all devices can be identified by CCW specific
sense data.

Signed-off-by: Jared Rossi 
---
 hw/s390x/ipl.h   |  5 
 include/hw/s390x/ipl/qipl.h  |  7 ++
 pc-bios/s390-ccw/iplb.h  |  4 
 pc-bios/s390-ccw/virtio.h|  2 ++
 pc-bios/s390-ccw/main.c  | 10 +---
 pc-bios/s390-ccw/virtio-blkdev.c | 39 +++-
 pc-bios/s390-ccw/virtio.c| 13 +++
 7 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 505cded490..aec2244321 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_STORE9
 #define DIAG308_PV_START10
 
-#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 682439..8199b839f0 100644
--- a/include/hw/s390x/ipl/qipl.h
+++ b/include/hw/s390x/ipl/qipl.h
@@ -20,6 +20,13 @@
 #define LOADPARM_LEN8
 #define NO_LOADPARM "\0\0\0\0\0\0\0\0"
 
+#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 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/virtio.h b/pc-bios/s390-ccw/virtio.h
index 597bd42358..d557a4a90e 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -239,6 +239,8 @@ struct VDev {
 VirtioGDN guessed_disk_nature;
 SubChannelId schid;
 SenseId senseid;
+VirtioDevType dev_type;
+int ipl_type;
 union {
 VirtioBlkConfig blk;
 VirtioScsiConfig scsi;
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 76bf743900..fef192c934 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -162,7 +162,7 @@ static void menu_setup(void)
 return;
 }
 
-switch (iplb.pbt) {
+switch (virtio_get_device()->ipl_type) {
 case S390_IPL_TYPE_CCW:
 case S390_IPL_TYPE_QEMU_SCSI:
 menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
@@ -190,6 +190,7 @@ static void css_setup(void)
 static void boot_setup(void)
 {
 char lpmsg[] = "LOADPARM=[]\n";
+VDev *vdev = virtio_get_device();
 
 if (have_iplb && memcmp(iplb.loadparm, NO_LOADPARM, LOADPARM_LEN) != 0) {
 ebcdic_to_ascii((char *) iplb.loadparm, loadparm_str, LOADPARM_LEN);
@@ -198,7 +199,10 @@ static void boot_setup(void)
 }
 
 if (have_iplb) {
+vdev->ipl_type = iplb.pbt;
 menu_setup();
+} else {
+vdev->ipl_type = QEMU_DEFAULT_IPL;
 }
 
 memcpy(lpmsg + 10, loadparm_str, 8);
@@ -216,7 +220,7 @@ static bool find_boot_device(void)
 VDev *vdev = virtio_get_device();
 bool found = false;
 
-switch (iplb.pbt) {
+switch (vdev->ipl_type) {
 case S390_IPL_TYPE_CCW:
 vdev->scsi_device_selected = false;
 debug_print_int("device no. ", iplb.ccw.devno);
@@ -245,7 +249,7 @@ static int virtio_setup(void)
 vdev->is_cdrom = false;
 int ret;
 
-switch (vdev->senseid.cu_model) {
+switch (vdev->dev_type) {
 case VIRTIO_ID_NET:
 puts("Network boot device detected");
 return 0;
diff --git a/pc-bios/s390-ccw/virtio-blkdev.c b/pc-bios/s390-ccw/virtio-blkdev.c
index 4b819dd80f..f40a9407c2 100644
--- a/pc-bios/s390-ccw/virtio-blkdev.c
+++ b/pc-bios/s390-ccw/virtio-blkdev.c
@@ -53,14 +53,14 @@ int virtio_read_many(unsigned long sector, void *load_addr, 
int sec_num)
 {
 VDev *vdev = virtio_get_device();
 
-switch (vdev->senseid.cu_model) {
+switch (vdev->dev_type) {
 case VIRTIO_ID_BLOCK:
 return virtio_blk_read_many(vdev, sector, load_addr, sec_num);
 case VIRTIO_ID_SCSI:
 return virtio_scsi_read_many(vdev, sector, load_addr, sec_num);
+default:
+return -1;
 }
-
-return -1;
 }
 
 unsigned long virtio_load_direct(unsigned long rec_list1, unsigned long