| Message ID | 20200528110444.20456-19-armbru@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes around device realization | expand |
Just adding Huacai, the original author and the new maintainer for Fuloong 2E machine. 1:04 PM Čet, 28.05.2020. Markus Armbruster <armbru@redhat.com> је написао/ла: > > sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but > neglect to realize it. Affects machines sam460ex, shix, r2d, and > fulong2e. > > In theory, a device becomes real only on realize. In practice, the > transition from unreal to real is a fuzzy one. The work to make a > device real can be spread between realize methods (fine), > instance_init methods (wrong), and board code wiring up the device > (fine as long as it effectively happens on realize). Depending on > what exactly is done where, a device can work even when we neglect > to realize it. > > This one appears to work. Nevertheless, it's a clear misuse of the > interface. Even when it works today (more or less by chance), it can > break tomorrow. > > Fix by realizing it right away. Visible in "info qom-tree"; here's > the change for sam460ex: > > /machine (sam460ex-machine) > [...] > /unattached (container) > [...] > - /device[14] (sii3112) > + /device[14] (i2c-ddc) > + /device[15] (sii3112) > [rest of device[*] renumbered...] > > Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a > Fixes: c82c7336de58876862e6b4dccbda29e9240fd388 > Cc: BALATON Zoltan <balaton@eik.bme.hu> > Cc: qemu-ppc@nongnu.org > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> > Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Tested-by: BALATON Zoltan <balaton@eik.bme.hu> > --- > hw/display/ati.c | 2 ++ > hw/display/sm501.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/hw/display/ati.c b/hw/display/ati.c > index 065f197678..5c71e5f295 100644 > --- a/hw/display/ati.c > +++ b/hw/display/ati.c > @@ -929,6 +929,8 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) > bitbang_i2c_init(&s->bbi2c, i2cbus); > I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); > i2c_set_slave_address(i2cddc, 0x50); > + object_property_set_bool(OBJECT(i2cddc), true, "realized", > + &error_abort); > > /* mmio register space */ > memory_region_init_io(&s->mm, OBJECT(s), &ati_mm_ops, s, > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index acc692531a..fbedc56715 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -1816,6 +1816,8 @@ static void sm501_init(SM501State *s, DeviceState *dev, > /* ddc */ > I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); > i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); > + object_property_set_bool(OBJECT(ddc), true, "realized", > + &error_abort); > > /* mmio */ > memory_region_init(&s->mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE); > -- > 2.21.3 >
On 5/28/20 1:04 PM, Markus Armbruster wrote: > sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but > neglect to realize it. Affects machines sam460ex, shix, r2d, and > fulong2e. > > In theory, a device becomes real only on realize. In practice, the > transition from unreal to real is a fuzzy one. The work to make a > device real can be spread between realize methods (fine), > instance_init methods (wrong), and board code wiring up the device > (fine as long as it effectively happens on realize). Depending on > what exactly is done where, a device can work even when we neglect > to realize it. > > This one appears to work. Nevertheless, it's a clear misuse of the > interface. Even when it works today (more or less by chance), it can > break tomorrow. > > Fix by realizing it right away. Visible in "info qom-tree"; here's > the change for sam460ex: > > /machine (sam460ex-machine) > [...] > /unattached (container) > [...] > - /device[14] (sii3112) > + /device[14] (i2c-ddc) > + /device[15] (sii3112) > [rest of device[*] renumbered...] > > Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a > Fixes: c82c7336de58876862e6b4dccbda29e9240fd388 > Cc: BALATON Zoltan <balaton@eik.bme.hu> > Cc: qemu-ppc@nongnu.org > Cc: Magnus Damm <magnus.damm@gmail.com> > Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> > Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Tested-by: BALATON Zoltan <balaton@eik.bme.hu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > --- > hw/display/ati.c | 2 ++ > hw/display/sm501.c | 2 ++ > 2 files changed, 4 insertions(+) > > diff --git a/hw/display/ati.c b/hw/display/ati.c > index 065f197678..5c71e5f295 100644 > --- a/hw/display/ati.c > +++ b/hw/display/ati.c > @@ -929,6 +929,8 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) > bitbang_i2c_init(&s->bbi2c, i2cbus); > I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); > i2c_set_slave_address(i2cddc, 0x50); > + object_property_set_bool(OBJECT(i2cddc), true, "realized", > + &error_abort); > > /* mmio register space */ > memory_region_init_io(&s->mm, OBJECT(s), &ati_mm_ops, s, > diff --git a/hw/display/sm501.c b/hw/display/sm501.c > index acc692531a..fbedc56715 100644 > --- a/hw/display/sm501.c > +++ b/hw/display/sm501.c > @@ -1816,6 +1816,8 @@ static void sm501_init(SM501State *s, DeviceState *dev, > /* ddc */ > I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); > i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); > + object_property_set_bool(OBJECT(ddc), true, "realized", > + &error_abort); > > /* mmio */ > memory_region_init(&s->mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE); >
diff --git a/hw/display/ati.c b/hw/display/ati.c index 065f197678..5c71e5f295 100644 --- a/hw/display/ati.c +++ b/hw/display/ati.c @@ -929,6 +929,8 @@ static void ati_vga_realize(PCIDevice *dev, Error **errp) bitbang_i2c_init(&s->bbi2c, i2cbus); I2CSlave *i2cddc = I2C_SLAVE(qdev_create(BUS(i2cbus), TYPE_I2CDDC)); i2c_set_slave_address(i2cddc, 0x50); + object_property_set_bool(OBJECT(i2cddc), true, "realized", + &error_abort); /* mmio register space */ memory_region_init_io(&s->mm, OBJECT(s), &ati_mm_ops, s, diff --git a/hw/display/sm501.c b/hw/display/sm501.c index acc692531a..fbedc56715 100644 --- a/hw/display/sm501.c +++ b/hw/display/sm501.c @@ -1816,6 +1816,8 @@ static void sm501_init(SM501State *s, DeviceState *dev, /* ddc */ I2CDDCState *ddc = I2CDDC(qdev_create(BUS(s->i2c_bus), TYPE_I2CDDC)); i2c_set_slave_address(I2C_SLAVE(ddc), 0x50); + object_property_set_bool(OBJECT(ddc), true, "realized", + &error_abort); /* mmio */ memory_region_init(&s->mmio_region, OBJECT(dev), "sm501.mmio", MMIO_SIZE);
sm501_init() and ati_vga_realize() create an "i2c-ddc" device, but neglect to realize it. Affects machines sam460ex, shix, r2d, and fulong2e. In theory, a device becomes real only on realize. In practice, the transition from unreal to real is a fuzzy one. The work to make a device real can be spread between realize methods (fine), instance_init methods (wrong), and board code wiring up the device (fine as long as it effectively happens on realize). Depending on what exactly is done where, a device can work even when we neglect to realize it. This one appears to work. Nevertheless, it's a clear misuse of the interface. Even when it works today (more or less by chance), it can break tomorrow. Fix by realizing it right away. Visible in "info qom-tree"; here's the change for sam460ex: /machine (sam460ex-machine) [...] /unattached (container) [...] - /device[14] (sii3112) + /device[14] (i2c-ddc) + /device[15] (sii3112) [rest of device[*] renumbered...] Fixes: 4a1f253adb45ac6019971193d5077c4d5d55886a Fixes: c82c7336de58876862e6b4dccbda29e9240fd388 Cc: BALATON Zoltan <balaton@eik.bme.hu> Cc: qemu-ppc@nongnu.org Cc: Magnus Damm <magnus.damm@gmail.com> Cc: Philippe Mathieu-Daudé <f4bug@amsat.org> Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Tested-by: BALATON Zoltan <balaton@eik.bme.hu> --- hw/display/ati.c | 2 ++ hw/display/sm501.c | 2 ++ 2 files changed, 4 insertions(+)