qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: jrossi@linux.ibm.com
To: qemu-devel@nongnu.org, qemu-s390x@nongnu.org, thuth@redhat.com
Cc: frankja@linux.ibm.com, jrossi@linux.ibm.com
Subject: [PATCH v5 07/19] pc-bios/s390-ccw: Remove panics from ISO IPL path
Date: Sat, 19 Oct 2024 21:29:41 -0400	[thread overview]
Message-ID: <20241020012953.1380075-8-jrossi@linux.ibm.com> (raw)
In-Reply-To: <20241020012953.1380075-1-jrossi@linux.ibm.com>

From: Jared Rossi <jrossi@linux.ibm.com>

Remove panic-on-error from IPL ISO El Torito specific functions so that error
recovery may be possible in the future.

Functions that would previously panic now provide a return code.

Signed-off-by: Jared Rossi <jrossi@linux.ibm.com>
---
 pc-bios/s390-ccw/bootmap.h  | 15 +++----
 pc-bios/s390-ccw/s390-ccw.h |  1 +
 pc-bios/s390-ccw/bootmap.c  | 87 ++++++++++++++++++++++++-------------
 3 files changed, 65 insertions(+), 38 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4a7d8a91f1..3cb573b86b 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -385,17 +385,14 @@ static inline uint32_t iso_733_to_u32(uint64_t x)
 
 #define ISO_PRIMARY_VD_SECTOR 16
 
-static inline void read_iso_sector(uint32_t block_offset, void *buf,
-                                   const char *errmsg)
-{
-    IPL_assert(virtio_read_many(block_offset, buf, 1) == 0, errmsg);
-}
-
-static inline void read_iso_boot_image(uint32_t block_offset, void *load_addr,
+static inline int read_iso_boot_image(uint32_t block_offset, void *load_addr,
                                        uint32_t blks_to_load)
 {
-    IPL_assert(virtio_read_many(block_offset, load_addr, blks_to_load) == 0,
-               "Failed to read boot image!");
+    if (virtio_read_many(block_offset, load_addr, blks_to_load)) {
+        puts("Failed to read boot image!");
+        return -1;
+    }
+    return 0;
 }
 
 #define ISO9660_MAX_DIR_DEPTH 8
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 6abb34e563..3e844abd71 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -30,6 +30,7 @@ typedef unsigned long long u64;
 #define EIO     1
 #define EBUSY   2
 #define ENODEV  3
+#define EINVAL  4
 
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 414c3f1b47..af73254acb 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -678,8 +678,10 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s)
     if (s->unused || !s->sector_count) {
         return false;
     }
-    read_iso_sector(bswap32(s->load_rba), magic_sec,
-                    "Failed to read image sector 0");
+    if (virtio_read(bswap32(s->load_rba), magic_sec)) {
+        puts("Failed to read image sector 0");
+        return false;
+    }
 
     /* Checking bytes 8 - 32 for S390 Linux magic */
     return !memcmp(magic_sec + 8, linux_s390_magic, 24);
@@ -692,28 +694,35 @@ static uint32_t sec_offset[ISO9660_MAX_DIR_DEPTH];
 /* Remained directory space in bytes */
 static uint32_t dir_rem[ISO9660_MAX_DIR_DEPTH];
 
-static inline uint32_t iso_get_file_size(uint32_t load_rba)
+static inline long iso_get_file_size(uint32_t load_rba)
 {
     IsoVolDesc *vd = (IsoVolDesc *)sec;
     IsoDirHdr *cur_record = &vd->vd.primary.rootdir;
     uint8_t *temp = sec + ISO_SECTOR_SIZE;
     int level = 0;
 
-    read_iso_sector(ISO_PRIMARY_VD_SECTOR, sec,
-                    "Failed to read ISO primary descriptor");
+    if (virtio_read(ISO_PRIMARY_VD_SECTOR, sec)) {
+        puts("Failed to read ISO primary descriptor");
+        return -EIO;
+    }
+
     sec_loc[0] = iso_733_to_u32(cur_record->ext_loc);
     dir_rem[0] = 0;
     sec_offset[0] = 0;
 
     while (level >= 0) {
-        IPL_assert(sec_offset[level] <= ISO_SECTOR_SIZE,
-                   "Directory tree structure violation");
+        if (sec_offset[level] > ISO_SECTOR_SIZE) {
+            puts("Directory tree structure violation");
+            return -EIO;
+        }
 
         cur_record = (IsoDirHdr *)(temp + sec_offset[level]);
 
         if (sec_offset[level] == 0) {
-            read_iso_sector(sec_loc[level], temp,
-                            "Failed to read ISO directory");
+            if (virtio_read(sec_loc[level], temp)) {
+                puts("Failed to read ISO directory");
+                return -EIO;
+            }
             if (dir_rem[level] == 0) {
                 /* Skip self and parent records */
                 dir_rem[level] = iso_733_to_u32(cur_record->data_len) -
@@ -758,8 +767,10 @@ static inline uint32_t iso_get_file_size(uint32_t load_rba)
         if (dir_rem[level] == 0) {
             /* Nothing remaining */
             level--;
-            read_iso_sector(sec_loc[level], temp,
-                            "Failed to read ISO directory");
+            if (virtio_read(sec_loc[level], temp)) {
+                puts("Failed to read ISO directory");
+                return -EIO;
+            }
         }
     }
 
@@ -774,19 +785,24 @@ static void load_iso_bc_entry(IsoBcSection *load)
      * is padded and ISO_SECTOR_SIZE bytes aligned
      */
     uint32_t blks_to_load = bswap16(s.sector_count) >> ET_SECTOR_SHIFT;
-    uint32_t real_size = iso_get_file_size(bswap32(s.load_rba));
+    long real_size = iso_get_file_size(bswap32(s.load_rba));
 
-    if (real_size) {
+    if (real_size > 0) {
         /* Round up blocks to load */
         blks_to_load = (real_size + ISO_SECTOR_SIZE - 1) / ISO_SECTOR_SIZE;
         puts("ISO boot image size verified");
     } else {
         puts("ISO boot image size could not be verified");
+        if (real_size < 0) {
+            return;
+        }
     }
 
-    read_iso_boot_image(bswap32(s.load_rba),
+    if (read_iso_boot_image(bswap32(s.load_rba),
                         (void *)((uint64_t)bswap16(s.load_segment)),
-                        blks_to_load);
+                        blks_to_load)) {
+        return;
+    }
 
     jump_to_low_kernel();
 }
@@ -809,17 +825,18 @@ static uint32_t find_iso_bc(void)
                 return bswap32(et->bc_offset);
             }
         }
-        read_iso_sector(block_num++, sec,
-                        "Failed to read ISO volume descriptor");
+        if (virtio_read(block_num++, sec)) {
+            puts("Failed to read ISO volume descriptor");
+            return 0;
+        }
     }
 
     return 0;
 }
 
-static IsoBcSection *find_iso_bc_entry(void)
+static IsoBcSection *find_iso_bc_entry(uint32_t offset)
 {
     IsoBcEntry *e = (IsoBcEntry *)sec;
-    uint32_t offset = find_iso_bc();
     int i;
     unsigned int loadparm = get_loadparm_index();
 
@@ -827,11 +844,13 @@ static IsoBcSection *find_iso_bc_entry(void)
         return NULL;
     }
 
-    read_iso_sector(offset, sec, "Failed to read El Torito boot catalog");
+    if (virtio_read(offset, sec)) {
+        puts("Failed to read El Torito boot catalog");
+        return NULL;
+    }
 
     if (!is_iso_bc_valid(e)) {
         /* The validation entry is mandatory */
-        panic("No valid boot catalog found!\n");
         return NULL;
     }
 
@@ -851,19 +870,25 @@ static IsoBcSection *find_iso_bc_entry(void)
         }
     }
 
-    panic("No suitable boot entry found on ISO-9660 media!\n");
-
     return NULL;
 }
 
-static void ipl_iso_el_torito(void)
+static int ipl_iso_el_torito(void)
 {
-    IsoBcSection *s = find_iso_bc_entry();
+    uint32_t offset = find_iso_bc();
+    if (!offset) {
+        return 0;
+    }
+
+    IsoBcSection *s = find_iso_bc_entry(offset);
 
     if (s) {
-        load_iso_bc_entry(s);
-        /* no return */
+        load_iso_bc_entry(s); /* only return in error */
+        return -1;
     }
+
+    puts("No suitable boot entry found on ISO-9660 media!");
+    return -EIO;
 }
 
 /**
@@ -893,7 +918,9 @@ static void zipl_load_vblk(void)
         if (blksize != VIRTIO_ISO_BLOCK_SIZE) {
             virtio_assume_iso9660();
         }
-        ipl_iso_el_torito();
+        if (ipl_iso_el_torito()) {
+            return;
+        }
     }
 
     if (blksize != VIRTIO_DASD_DEFAULT_BLOCK_SIZE) {
@@ -907,7 +934,9 @@ static void zipl_load_vscsi(void)
 {
     if (virtio_get_block_size() == VIRTIO_ISO_BLOCK_SIZE) {
         /* Is it an ISO image in non-CD drive? */
-        ipl_iso_el_torito();
+        if (ipl_iso_el_torito()) {
+            return;
+        }
     }
 
     puts("Using guessed DASD geometry.");
-- 
2.45.1



  parent reply	other threads:[~2024-10-20  1:34 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-20  1:29 [PATCH v5 00/19] s390x: Add Full Boot Order Support jrossi
2024-10-20  1:29 ` [PATCH v5 01/19] hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware jrossi
2024-10-20  1:29 ` [PATCH v5 02/19] pc-bios/s390-ccw: Use the libc from SLOF and remove sclp prints jrossi
2024-10-22 17:36   ` Thomas Huth
2024-10-22 20:12     ` Jared Rossi
2024-10-23  4:51       ` Thomas Huth
2024-10-23 10:30         ` Jared Rossi
2024-10-20  1:29 ` [PATCH v5 03/19] pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img binary jrossi
2024-10-20  1:29 ` [PATCH v5 04/19] hw/s390x: Remove the possibility to load the s390-netboot.img binary jrossi
2024-10-20  1:29 ` [PATCH v5 05/19] pc-bios/s390-ccw: Merge netboot.mak into the main Makefile jrossi
2024-10-20  1:29 ` [PATCH v5 06/19] docs/system/s390x/bootdevices: Update the documentation about network booting jrossi
2024-10-20  1:29 ` jrossi [this message]
2024-10-21  8:41   ` [PATCH v5 07/19] pc-bios/s390-ccw: Remove panics from ISO IPL path Thomas Huth
2024-10-20  1:29 ` [PATCH v5 08/19] pc-bios/s390-ccw: Remove panics from ECKD " jrossi
2024-10-21  9:12   ` Thomas Huth
2024-10-20  1:29 ` [PATCH v5 09/19] pc-bios/s390-ccw: Remove panics from SCSI " jrossi
2024-10-21  9:26   ` Thomas Huth
2024-10-20  1:29 ` [PATCH v5 10/19] pc-bios/s390-ccw: Remove panics from DASD " jrossi
2024-10-20  1:29 ` [PATCH v5 11/19] pc-bios/s390-ccw: Remove panics from Netboot " jrossi
2024-10-21  9:30   ` Thomas Huth
2024-10-20  1:29 ` [PATCH v5 12/19] pc-bios/s390-ccw: Enable failed IPL to return after error jrossi
2024-10-20  1:29 ` [PATCH v5 13/19] include/hw/s390x: Add include files for common IPL structs jrossi
2024-10-20  1:29 ` [PATCH v5 14/19] s390x: Add individual loadparm assignment to CCW device jrossi
2024-10-21 12:26   ` Thomas Huth
2024-10-31  8:45   ` Thomas Huth
2024-11-05 20:22     ` Jared Rossi
2024-10-20  1:29 ` [PATCH v5 15/19] hw/s390x: Build an IPLB for each boot device jrossi
2024-10-21 12:43   ` Thomas Huth
2024-10-22 13:13   ` Thomas Huth
2024-10-22 14:25     ` Jared Rossi
2024-10-20  1:29 ` [PATCH v5 16/19] s390x: Rebuild IPLB for SCSI device directly from DIAG308 jrossi
2024-10-20  1:29 ` [PATCH v5 17/19] pc-bios/s390x: Enable multi-device boot loop jrossi
2024-10-22  7:33   ` Thomas Huth
2024-10-20  1:29 ` [PATCH v5 18/19] docs/system: Update documentation for s390x IPL jrossi
2024-10-20  1:29 ` [PATCH v5 19/19] tests/qtest: Add s390x boot order tests to cdrom-test.c jrossi
2024-10-21 14:52   ` Thomas Huth
2024-10-31 15:50 ` [PATCH v5 00/19] s390x: Add Full Boot Order Support Thomas Huth
2024-11-05 16:42   ` Jared Rossi
2024-11-06 11:10     ` Thomas Huth
2024-11-07 20:42       ` Jared Rossi
2024-11-08 14:37         ` Thomas Huth
2024-11-11 12:23           ` Thomas Huth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241020012953.1380075-8-jrossi@linux.ibm.com \
    --to=jrossi@linux.ibm.com \
    --cc=frankja@linux.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-s390x@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).