| Message ID | 20211002110007.30825-2-mark.cave-ayland@ilande.co.uk |
|---|---|
| State | New |
| Headers | show |
| Series | macfb: fixes for booting MacOS | expand |
On Sat, 2 Oct 2021, 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> > --- > hw/display/macfb.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/display/macfb.c b/hw/display/macfb.c > index 76808b69cc..2b747a8de8 100644 > --- a/hw/display/macfb.c > +++ b/hw/display/macfb.c There's one more in macfb_common_realize() after: memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(s), "macfb-vram", MACFB_VRAM_SIZE, errp); otherwise Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> > @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp) > MacfbState *ms = &s->macfb; > > macfb_common_realize(dev, ms, errp); > + if (*errp) { > + return; > + } > + > sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_ctrl); > sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram); > } > @@ -391,8 +395,15 @@ static void macfb_nubus_realize(DeviceState *dev, Error **errp) > MacfbState *ms = &s->macfb; > > ndc->parent_realize(dev, errp); > + if (*errp) { > + return; > + } > > macfb_common_realize(dev, ms, errp); > + if (*errp) { > + return; > + } > + > memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl); > memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram); > } >
On 10/2/21 12:59, 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> > --- > hw/display/macfb.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/display/macfb.c b/hw/display/macfb.c > index 76808b69cc..2b747a8de8 100644 > --- a/hw/display/macfb.c > +++ b/hw/display/macfb.c > @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp) > MacfbState *ms = &s->macfb; > > macfb_common_realize(dev, ms, errp); > + if (*errp) { > + return; > + } See a related discussion: https://lore.kernel.org/qemu-devel/87bl47ll9l.fsf@dusky.pond.sub.org/
Le 02/10/2021 à 12:59, Mark Cave-Ayland a écrit : > 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> > --- > hw/display/macfb.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/hw/display/macfb.c b/hw/display/macfb.c > index 76808b69cc..2b747a8de8 100644 > --- a/hw/display/macfb.c > +++ b/hw/display/macfb.c > @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp) > MacfbState *ms = &s->macfb; > > macfb_common_realize(dev, ms, errp); > + if (*errp) { > + return; > + } > + > sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_ctrl); > sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram); > } > @@ -391,8 +395,15 @@ static void macfb_nubus_realize(DeviceState *dev, Error **errp) > MacfbState *ms = &s->macfb; > > ndc->parent_realize(dev, errp); > + if (*errp) { > + return; > + } > > macfb_common_realize(dev, ms, errp); > + if (*errp) { > + return; > + } > + > memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl); > memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram); > } > Perhaps as suggested by Philippe you change macfb_common_realize() to return a value and an error? Otherwise: Reviewed-by: Laurent Vivier <laurent@vivier.eu>
On 02/10/2021 12:36, BALATON Zoltan wrote: > On Sat, 2 Oct 2021, 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> >> --- >> hw/display/macfb.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/display/macfb.c b/hw/display/macfb.c >> index 76808b69cc..2b747a8de8 100644 >> --- a/hw/display/macfb.c >> +++ b/hw/display/macfb.c > > There's one more in macfb_common_realize() after: > > memory_region_init_ram_nomigrate(&s->mem_vram, OBJECT(s), "macfb-vram", > MACFB_VRAM_SIZE, errp); > > otherwise > > Reviewed-by: BALATON Zoltan <balaton@eik.bme.hu> Ah yes, IIRC from the last patchset the outcome of the discussion with Markus was that these functions should use &error_abort. I'll make the same change here for v2. ATB, Mark.
On 02/10/2021 14:47, Philippe Mathieu-Daudé wrote: > On 10/2/21 12:59, 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> >> --- >> hw/display/macfb.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/hw/display/macfb.c b/hw/display/macfb.c >> index 76808b69cc..2b747a8de8 100644 >> --- a/hw/display/macfb.c >> +++ b/hw/display/macfb.c >> @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp) >> MacfbState *ms = &s->macfb; >> >> macfb_common_realize(dev, ms, errp); >> + if (*errp) { >> + return; >> + } > > See a related discussion: > https://lore.kernel.org/qemu-devel/87bl47ll9l.fsf@dusky.pond.sub.org/ Interesting, thanks for the link. I will update macfb_common_realize() to return a boolean for v2. ATB, Mark.
diff --git a/hw/display/macfb.c b/hw/display/macfb.c index 76808b69cc..2b747a8de8 100644 --- a/hw/display/macfb.c +++ b/hw/display/macfb.c @@ -379,6 +379,10 @@ static void macfb_sysbus_realize(DeviceState *dev, Error **errp) MacfbState *ms = &s->macfb; macfb_common_realize(dev, ms, errp); + if (*errp) { + return; + } + sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_ctrl); sysbus_init_mmio(SYS_BUS_DEVICE(s), &ms->mem_vram); } @@ -391,8 +395,15 @@ static void macfb_nubus_realize(DeviceState *dev, Error **errp) MacfbState *ms = &s->macfb; ndc->parent_realize(dev, errp); + if (*errp) { + return; + } macfb_common_realize(dev, ms, errp); + if (*errp) { + return; + } + memory_region_add_subregion(&nd->slot_mem, DAFB_BASE, &ms->mem_ctrl); memory_region_add_subregion(&nd->slot_mem, VIDEO_BASE, &ms->mem_vram); }
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> --- hw/display/macfb.c | 11 +++++++++++ 1 file changed, 11 insertions(+)