hw/misc/macio: Fix crash when listing device properties of macio device
PATCH 1/1 hw/misc/macio: Fix crash when listing device properties of macio device
The macio-newworld device can currently be used to abort QEMU unexpectedly:
$ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio
{"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2},
"package": "build-all"}, "capabilities": []}}
{ 'execute': 'qmp_capabilities' }
{"return": {}}
{ 'execute': 'device-list-properties',
'arguments': {'typename': 'macio-newworld'}}
Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222:
Device 'serial0' is in use
Aborted (core dumped)qdev properties should be set during realize(), not during instance_init(), so move the related code there to fix this problem.
Signed-off-by Thomas Huth <[email protected]>
--- hw/misc/macio/macio.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
hw/misc/macio/macio.c +7 -7
@@ -115,6 +115,13 @@ static void macio_common_realize(PCIDevice *d, Error **errp)memory_region_add_subregion (&s->bar,0x16000 ,sysbus_mmio_get_region (sysbus_dev,0 )); +qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); +qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); +qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);object_property_set_bool (OBJECT (&s->escc), true,"realized" , &err);if (err) {error_propagate (errp, err);
@@ -341,13 +348,6 @@ static void macio_instance_init(Object *obj)object_property_add_child (obj,"dbdma" ,OBJECT (&s->dbdma),NULL );object_initialize (&s->escc,sizeof (s->escc),TYPE_ESCC ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); -qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); -qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);qdev_set_parent_bus (DEVICE (&s->escc),sysbus_get_default ());object_property_add_child (obj,"escc" ,OBJECT (&s->escc),NULL ); }
Signature · 1 line
1.8.3.1
Quoted 13 lines · Thomas Huth
The macio-newworld device can currently be used to abort QEMU unexpectedly:
$ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped)
Quoted 2 lines
qdev properties should be set during
realize(), not duringinstance_init(), so move the related code there to fix this problem.
Oops I missed that.
Quoted 2 lines
Signed-off-by Thomas Huth <[email protected]>
Reviewed-by Philippe Mathieu-Daudé <[email protected]>
Quoted 37 lines
--- hw/misc/macio/macio.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)hw/misc/macio/macio.c +7
-7@@ -115,6 +115,13 @@ static void macio_common_realize(PCIDevice *d, Error **errp)memory_region_add_subregion (&s->bar,0x16000 ,sysbus_mmio_get_region (sysbus_dev,0 )); +qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); +qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); +qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);object_property_set_bool (OBJECT (&s->escc), true,"realized" , &err);if (err) {error_propagate (errp, err);@@ -341,13 +348,6 @@ static void macio_instance_init(Object *obj)object_property_add_child (obj,"dbdma" ,OBJECT (&s->dbdma),NULL );object_initialize (&s->escc,sizeof (s->escc),TYPE_ESCC ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); -qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); -qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);qdev_set_parent_bus (DEVICE (&s->escc),sysbus_get_default ());object_property_add_child (obj,"escc" ,OBJECT (&s->escc),NULL ); }
David Gibson
AppliedQuoted 13 lines · Thomas Huth
The macio-newworld device can currently be used to abort QEMU unexpectedly:
$ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped)
Quoted 4 lines
qdev properties should be set during
realize(), not duringinstance_init(), so move the related code there to fix this problem.Signed-off-by Thomas Huth <[email protected]>
Applied, thanks.
Quoted 36 lines
--- hw/misc/macio/macio.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)hw/misc/macio/macio.c +7
-7@@ -115,6 +115,13 @@ static void macio_common_realize(PCIDevice *d, Error **errp)memory_region_add_subregion (&s->bar,0x16000 ,sysbus_mmio_get_region (sysbus_dev,0 )); +qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); +qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); +qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);object_property_set_bool (OBJECT (&s->escc), true,"realized" , &err);if (err) {error_propagate (errp, err);@@ -341,13 +348,6 @@ static void macio_instance_init(Object *obj)object_property_add_child (obj,"dbdma" ,OBJECT (&s->dbdma),NULL );object_initialize (&s->escc,sizeof (s->escc),TYPE_ESCC ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); -qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); -qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);qdev_set_parent_bus (DEVICE (&s->escc),sysbus_get_default ());object_property_add_child (obj,"escc" ,OBJECT (&s->escc),NULL ); }
Signature · 4 lines
David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Quoted 13 lines · Thomas Huth
The macio-newworld device can currently be used to abort QEMU unexpectedly:
$ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped)
Quoted 2 lines
qdev properties should be set during
realize(), not duringinstance_init(), so move the related code there to fix this problem.
Ah right, presumably this is because of the reference to serial_hds again? The patch looks good, although given that it affects macio_instance_init() and macio_common_realize() then I would have expected this to have failed on the macio-oldworld device too (or perhaps you were just unlucky that this was the first macio-*world device enumerated).
Quoted 28 lines
Signed-off-by Thomas Huth <[email protected]>--- hw/misc/macio/macio.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)hw/misc/macio/macio.c +7
-0@@ -115,6 +115,13 @@ static void macio_common_realize(PCIDevice *d, Error **errp)memory_region_add_subregion (&s->bar,0x16000 ,sysbus_mmio_get_region (sysbus_dev,0 )); +qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); +qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); +qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); +qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); +qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);object_property_set_bool (OBJECT (&s->escc), true,"realized" , &err);if (err) {error_propagate (errp, err);@@ -341,13 +348,6 @@ static void macio_instance_init(Object *obj)object_property_add_child (obj,"dbdma" ,OBJECT (&s->dbdma),NULL );object_initialize (&s->escc,sizeof (s->escc),TYPE_ESCC );
hw/misc/macio/macio.c static void macio_instance_init(Object *obj) · line 353
Quoted 10 lines
hw/misc/macio/macio.c +0
-7-qdev_prop_set_uint32 (DEVICE (&s->escc),"disabled" ,0 ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"frequency" ,ESCC_CLOCK ); -qdev_prop_set_uint32 (DEVICE (&s->escc),"it_shift" ,4 ); -qdev_prop_set_chr (DEVICE (&s->escc),"chrA" , serial_hds[0 ]); -qdev_prop_set_chr (DEVICE (&s->escc),"chrB" , serial_hds[1 ]); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnBtype" , escc_serial); -qdev_prop_set_uint32 (DEVICE (&s->escc),"chnAtype" , escc_serial);qdev_set_parent_bus (DEVICE (&s->escc),sysbus_get_default ());object_property_add_child (obj,"escc" ,OBJECT (&s->escc),NULL ); }
Acked-by Mark Cave-Ayland <[email protected]>
ATB,
Mark.
Quoted 16 lines · Mark Cave-Ayland
Quoted 14 lines · Thomas Huth
The macio-newworld device can currently be used to abort QEMU unexpectedly:
$ ppc-softmmu/qemu-system-ppc -S -M ref405ep,accel=qtest -qmp stdio {"QMP": {"version": {"qemu": {"micro": 50, "minor": 11, "major": 2}, "package": "build-all"}, "capabilities": []}} { 'execute': 'qmp_capabilities' } {"return": {}} { 'execute': 'device-list-properties', 'arguments': {'typename': 'macio-newworld'}} Unexpected error in qemu_chr_fe_init() at chardev/char-fe.c:222: Device 'serial0' is in use Aborted (core dumped)
Quoted 6 lines
Quoted 3 lines
qdev properties should be set during
realize(), not duringinstance_init(),
so move the related code there to fix this problem.Ah right, presumably this is because of the reference to
serial_hdsagain?
Right.
Quoted 5 lines
The patch looks good, although given that it affects
macio_instance_init()andmacio_common_realize()then I would have expected this to have failed on the macio-oldworld device too (or perhaps you were just unlucky that this was the first macio-*world device enumerated).
Yes, the bug triggers also with the macio-oldworld device. I just hit it with the newworld device first.
Thomas