On 27/09/2024 02.51, jro...@linux.ibm.com wrote:
From: Jared Rossi <jro...@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 <jro...@linux.ibm.com>
---
pc-bios/s390-ccw/bootmap.h | 17 +++++++---
pc-bios/s390-ccw/s390-ccw.h | 1 +
pc-bios/s390-ccw/bootmap.c | 64 ++++++++++++++++++++++++-------------
3 files changed, 55 insertions(+), 27 deletions(-)
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index bbe2c132aa..cb5346829b 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -385,17 +385,24 @@ 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,
+static inline int read_iso_sector(uint32_t block_offset, void *buf,
const char *errmsg)
{
- IPL_assert(virtio_read_many(block_offset, buf, 1) == 0, errmsg);
+ if (virtio_read(block_offset, buf)) {
+ puts(errmsg);
+ return 1;
+ }
+ return 0;
}
-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)) {
That "!" looks wrong here? Or do I misunderstood the original IPL_assert()
condition?
+ 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 0ed7eb8ade..cbd92f3671 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..31cf0f6d97 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 (read_iso_sector(bswap32(s->load_rba), magic_sec,
+ "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);
@@ -706,14 +708,18 @@ static inline uint32_t iso_get_file_size(uint32_t
load_rba)
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;
+ }
Any reasons for switching from read_iso_sector() directly to virtio_read() here?
Apart from that, patch looks fine for me, thanks for doing this clean-up work!
Thomas
