diff mbox series

[v2,01/12] macfb: handle errors that occur during realize

Message ID 20211004211928.15803-2-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series macfb: fixes for booting MacOS | expand

Commit Message

Mark Cave-Ayland Oct. 4, 2021, 9:19 p.m. UTC 
Make sure any errors that occur within the macfb realize chain are detected
and handled correctly to prevent crashes and to ensure that error messages are
reported back to the user.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
---
 hw/display/macfb.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 5, 2021, 3:34 p.m. UTC | #1 
On 10/4/21 23:19, Mark Cave-Ayland wrote:
> Make sure any errors that occur within the macfb realize chain are detected
> and handled correctly to prevent crashes and to ensure that error messages are
> reported back to the user.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu>
> Reviewed-by: Laurent Vivier <laurent@vivier.eu>
> ---
>  hw/display/macfb.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)

> @@ -391,8 +396,14 @@ static void macfb_nubus_realize(DeviceState *dev, Error **errp)
>      MacfbState *ms = &s->macfb;
>  
>      ndc->parent_realize(dev, errp);
> +    if (*errp) {
> +        return;
> +    }

Sorry for being picky, but this is one fix (one patch),

> +    if (!macfb_common_realize(dev, ms, errp)) {
> +        return;
> +    }

and this is another fix (another patch, including the bool).

>  
> -    macfb_common_realize(dev, ms, errp);
>      memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl);
>      memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram);
>  }
> 

I'd rather split this in 2, but I'd be OK if you just reword
the description, either split or reworded:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff mbox series

Patch

diff --git a/hw/display/macfb.c b/hw/display/macfb.c
index 76808b69cc..2ec25c5d6f 100644
--- a/hw/display/macfb.c
+++ b/hw/display/macfb.c
@@ -343,14 +343,14 @@  static const GraphicHwOps macfb_ops = {
     .gfx_update = macfb_update_display,
 };
 
-static void macfb_common_realize(DeviceState *dev, MacfbState *s, Error **errp)
+static bool macfb_common_realize(DeviceState *dev, MacfbState *s, Error **errp)
 {
     DisplaySurface *surface;
 
     if (s->depth != 1 && s->depth != 2 && s->depth != 4 && s->depth != 8 &&
         s->depth != 16 && s->depth != 24) {
         error_setg(errp, "unknown guest depth %d", s->depth);
-        return;
+        return false;
     }
 
     s->con = graphic_console_init(dev, 0, &macfb_ops, s);
@@ -359,18 +359,20 @@  static void macfb_common_realize(DeviceState *dev, MacfbState *s, Error **errp)
     if (surface_bits_per_pixel(surface) != 32) {
         error_setg(errp, "unknown host depth %d",
                    surface_bits_per_pixel(surface));
-        return;
+        return false;
     }
 
     memory_region_init_io(&s->mem_ctrl, OBJECT(dev), &macfb_ctrl_ops, s,
                           "macfb-ctrl", 0x1000);
 
     memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(s), "macfb-vram",
-                                     MACFB_VRAM_SIZE, errp);
+                                     MACFB_VRAM_SIZE, &error_abort);
     s->vram = memory_region_get_ram_ptr(&s->mem_vram);
     s->vram_bit_mask = MACFB_VRAM_SIZE - 1;
     vmstate_register_ram(&s->mem_vram, dev);
     memory_region_set_coalescing(&s->mem_vram);
+
+    return true;
 }
 
 static void macfb_sysbus_realize(DeviceState *dev, Error **errp)
@@ -378,7 +380,10 @@  static void macfb_sysbus_realize(DeviceState *dev, Error **errp)
     MacfbSysBusState *s = MACFB(dev);
     MacfbState *ms = &s->macfb;
 
-    macfb_common_realize(dev, ms, errp);
+    if (!macfb_common_realize(dev, ms, errp)) {
+        return;
+    }
+
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_ctrl);
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram);
 }
@@ -391,8 +396,14 @@  static void macfb_nubus_realize(DeviceState *dev, Error **errp)
     MacfbState *ms = &s->macfb;
 
     ndc->parent_realize(dev, errp);
+    if (*errp) {
+        return;
+    }
+
+    if (!macfb_common_realize(dev, ms, errp)) {
+        return;
+    }
 
-    macfb_common_realize(dev, ms, errp);
     memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl);
     memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram);
 }