[PATCH 0/2] print some errors to the screen
Gerd Hoffmann (2): add hwerr_printf function for threads display error message for blocksizes != 512 src/output.h | 5 +++++ src/hw/blockcmd.c | 2 +- src/hw/virtio-blk.c | 4 ++-- src/output.c | 17 +++++++++++++++++ src/post.c | 4 ++++ 5 files changed, 29 insertions(+), 3 deletions(-) -- 2.40.1
Printing to the screen from threads doesn't work, because that involves a switch to real mode for using int10h services. Add a string buffer and hwerr_printf() helper functions to store error messages. Print the buffer later, after device initialization, from main thread in case it is not empty. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/output.h | 5 +++++ src/output.c | 17 +++++++++++++++++ src/post.c | 4 ++++ 3 files changed, 26 insertions(+) diff --git a/src/output.h b/src/output.h index 14288cf505d8..4548d2d4abc2 100644 --- a/src/output.h +++ b/src/output.h @@ -16,6 +16,11 @@ char * znprintf(size_t size, const char *fmt, ...) __attribute__ ((format (printf, 2, 3))); void __dprintf(const char *fmt, ...) __attribute__ ((format (printf, 1, 2))); + +extern char hwerror_str[512]; +void hwerr_printf(const char *fmt, ...) + __attribute__ ((format (printf, 1, 2))); + struct bregs; void __debug_enter(struct bregs *regs, const char *fname); void __debug_isr(const char *fname); diff --git a/src/output.c b/src/output.c index 0184444c8f21..8c9d6b8fb1e0 100644 --- a/src/output.c +++ b/src/output.c @@ -419,6 +419,23 @@ snprintf(char *str, size_t size, const char *fmt, ...) return end - str; } +char hwerror_str[512]; +struct snprintfinfo hwerror_info = { + .info = { putc_str }, + .str = hwerror_str, + .end = hwerror_str + sizeof(hwerror_str) - 1, +}; + +void +hwerr_printf(const char *fmt, ...) +{ + ASSERT32FLAT(); + va_list args; + va_start(args, fmt); + bvprintf(&hwerror_info.info, fmt, args); + va_end(args); +} + // Build a formatted string - malloc'ing the memory. char * znprintf(size_t size, const char *fmt, ...) diff --git a/src/post.c b/src/post.c index f93106a1c9c9..3e85da43060f 100644 --- a/src/post.c +++ b/src/post.c @@ -216,6 +216,10 @@ maininit(void) device_hardware_setup(); wait_threads(); } + if (hwerror_str[0]) + printf("\n" + "hardware setup errors:\n" + "%s", hwerror_str); // Run option roms optionrom_setup(); -- 2.40.1
This actually happens in case users try to use 4k sectors with seabios. Printing the error to the screen instead of only the debug log helps users to figure why their guest doesn't boot. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- src/hw/blockcmd.c | 2 +- src/hw/virtio-blk.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hw/blockcmd.c b/src/hw/blockcmd.c index 6b6fea970748..ff88680b16b6 100644 --- a/src/hw/blockcmd.c +++ b/src/hw/blockcmd.c @@ -336,7 +336,7 @@ scsi_drive_setup(struct drive_s *drive, const char *s, int prio) // 64-bit LBA anyway. drive->blksize = be32_to_cpu(capdata.blksize); if (drive->blksize != DISK_SECTOR_SIZE) { - dprintf(1, "%s: unsupported block size %d\n", s, drive->blksize); + hwerr_printf("%s: unsupported block size %d\n", s, drive->blksize); return -1; } drive->sectors = (u64)be32_to_cpu(capdata.sectors) + 1; diff --git a/src/hw/virtio-blk.c b/src/hw/virtio-blk.c index e087fe4fb0e3..9d2bcb0f38f4 100644 --- a/src/hw/virtio-blk.c +++ b/src/hw/virtio-blk.c @@ -193,8 +193,8 @@ init_virtio_blk(void *data) vdrive->drive.blksize = DISK_SECTOR_SIZE; } if (vdrive->drive.blksize != DISK_SECTOR_SIZE) { - dprintf(1, "virtio-blk %pP block size %d is unsupported\n", - pci, vdrive->drive.blksize); + hwerr_printf("virtio-blk %pP block size %d is unsupported\n", + pci, vdrive->drive.blksize); goto fail; } dprintf(3, "virtio-blk %pP blksize=%d sectors=%u size_max=%u " -- 2.40.1
For what it is worth, I'm not sure sending debug/error messages to the vga screen is a good long-term plan. I fear it will be more frustration than help to users. Sure, having the VM not start isn't a fun experience, but I'm not sure a weird cryptic error message that flashes on the screen for a split second is an improvement. Enabling the seabios log from qemu is an annoying step to have to take, but I suspect taking that step may ultimately be necessary in many cases. Cheers, -Kevin On Wed, May 03, 2023 at 11:21:59AM +0200, Gerd Hoffmann wrote:
Gerd Hoffmann (2): add hwerr_printf function for threads display error message for blocksizes != 512
src/output.h | 5 +++++ src/hw/blockcmd.c | 2 +- src/hw/virtio-blk.c | 4 ++-- src/output.c | 17 +++++++++++++++++ src/post.c | 4 ++++ 5 files changed, 29 insertions(+), 3 deletions(-)
-- 2.40.1
_______________________________________________ SeaBIOS mailing list -- seabios@seabios.org To unsubscribe send an email to seabios-leave@seabios.org
On Thu, May 04, 2023 at 02:28:05PM -0400, Kevin O'Connor wrote:
For what it is worth, I'm not sure sending debug/error messages to the vga screen is a good long-term plan. I fear it will be more frustration than help to users. Sure, having the VM not start isn't a fun experience, but I'm not sure a weird cryptic error message that flashes on the screen for a split second is an improvement.
It surely doesn't make sense for most errors, patch #2 switches over only the errors for unsupported blocksizes. That actually happens in practice. And when the VM doesn't boot the message will typically continue to be displayed ... take care, Gerd
On Fri, May 05, 2023 at 09:04:41AM +0200, Gerd Hoffmann wrote:
On Thu, May 04, 2023 at 02:28:05PM -0400, Kevin O'Connor wrote:
For what it is worth, I'm not sure sending debug/error messages to the vga screen is a good long-term plan. I fear it will be more frustration than help to users. Sure, having the VM not start isn't a fun experience, but I'm not sure a weird cryptic error message that flashes on the screen for a split second is an improvement.
It surely doesn't make sense for most errors, patch #2 switches over only the errors for unsupported blocksizes. That actually happens in practice. And when the VM doesn't boot the message will typically continue to be displayed ...
I understand. For what it is worth, I'm still unsure this is a good long term plan though. -Kevin
participants (2)
-
Gerd Hoffmann -
Kevin O'Connor