* [PATCH v2] vga: don't abort when adding a duplicate isa-vga device
@ 2021-08-16 12:36 Jose R. Ziviani
2021-08-16 13:01 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 3+ messages in thread
From: Jose R. Ziviani @ 2021-08-16 12:36 UTC (permalink / raw)
To: qemu-devel; +Cc: thuth, kraxel, Jose R. Ziviani
If users try to add an isa-vga device that was already registered,
still in command line, qemu will crash:
$ qemu-system-mips64el -M pica61 -device isa-vga
RAMBlock "vga.vram" already registered, abort!
Aborted (core dumped)
That particular board registers the device automaticaly, so it's
not obvious that a VGA device already exists. This patch changes
this behavior by displaying a message and exiting without crashing.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44
Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
---
hw/display/vga-isa.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
index 90851e730b..1fba33b22b 100644
--- a/hw/display/vga-isa.c
+++ b/hw/display/vga-isa.c
@@ -33,6 +33,7 @@
#include "hw/loader.h"
#include "hw/qdev-properties.h"
#include "qom/object.h"
+#include "qapi/error.h"
#define TYPE_ISA_VGA "isa-vga"
OBJECT_DECLARE_SIMPLE_TYPE(ISAVGAState, ISA_VGA)
@@ -61,6 +62,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
MemoryRegion *vga_io_memory;
const MemoryRegionPortio *vga_ports, *vbe_ports;
+ /*
+ * make sure this device is not being added twice, if so
+ * exit without crashing qemu
+ */
+ if (qemu_ram_block_by_name("vga.vram")) {
+ error_setg(errp, "vga.vram is already registered");
+ return;
+ }
+
s->global_vmstate = true;
vga_common_init(s, OBJECT(dev));
s->legacy_address_space = isa_address_space(isadev);
--
2.32.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vga: don't abort when adding a duplicate isa-vga device
2021-08-16 12:36 [PATCH v2] vga: don't abort when adding a duplicate isa-vga device Jose R. Ziviani
@ 2021-08-16 13:01 ` Philippe Mathieu-Daudé
2021-08-16 13:53 ` Jose Ricardo Ziviani
0 siblings, 1 reply; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-08-16 13:01 UTC (permalink / raw)
To: Jose R. Ziviani, qemu-devel; +Cc: thuth, kraxel
On 8/16/21 2:36 PM, Jose R. Ziviani wrote:
> If users try to add an isa-vga device that was already registered,
> still in command line, qemu will crash:
>
> $ qemu-system-mips64el -M pica61 -device isa-vga
> RAMBlock "vga.vram" already registered, abort!
> Aborted (core dumped)
>
> That particular board registers the device automaticaly, so it's
> not obvious that a VGA device already exists. This patch changes
> this behavior by displaying a message and exiting without crashing.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44
> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
> ---
> hw/display/vga-isa.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
> index 90851e730b..1fba33b22b 100644
> --- a/hw/display/vga-isa.c
> +++ b/hw/display/vga-isa.c
> @@ -33,6 +33,7 @@
> #include "hw/loader.h"
> #include "hw/qdev-properties.h"
> #include "qom/object.h"
> +#include "qapi/error.h"
>
> #define TYPE_ISA_VGA "isa-vga"
> OBJECT_DECLARE_SIMPLE_TYPE(ISAVGAState, ISA_VGA)
> @@ -61,6 +62,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
> MemoryRegion *vga_io_memory;
> const MemoryRegionPortio *vga_ports, *vbe_ports;
>
> + /*
> + * make sure this device is not being added twice, if so
> + * exit without crashing qemu
> + */
> + if (qemu_ram_block_by_name("vga.vram")) {
> + error_setg(errp, "vga.vram is already registered");
Maybe "'isa-vga' device already registered" is easier to understand?
(vga.vram is about the device model internal).
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> + return;
> + }
> +
> s->global_vmstate = true;
> vga_common_init(s, OBJECT(dev));
> s->legacy_address_space = isa_address_space(isadev);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] vga: don't abort when adding a duplicate isa-vga device
2021-08-16 13:01 ` Philippe Mathieu-Daudé
@ 2021-08-16 13:53 ` Jose Ricardo Ziviani
0 siblings, 0 replies; 3+ messages in thread
From: Jose Ricardo Ziviani @ 2021-08-16 13:53 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel; +Cc: thuth, kraxel
Hello Philippe!
On 16/08/2021 10:01, Philippe Mathieu-Daudé wrote:
> On 8/16/21 2:36 PM, Jose R. Ziviani wrote:
>> If users try to add an isa-vga device that was already registered,
>> still in command line, qemu will crash:
>>
>> $ qemu-system-mips64el -M pica61 -device isa-vga
>> RAMBlock "vga.vram" already registered, abort!
>> Aborted (core dumped)
>>
>> That particular board registers the device automaticaly, so it's
>> not obvious that a VGA device already exists. This patch changes
>> this behavior by displaying a message and exiting without crashing.
>>
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/44
>> Signed-off-by: Jose R. Ziviani <jziviani@suse.de>
>> ---
>> hw/display/vga-isa.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/hw/display/vga-isa.c b/hw/display/vga-isa.c
>> index 90851e730b..1fba33b22b 100644
>> --- a/hw/display/vga-isa.c
>> +++ b/hw/display/vga-isa.c
>> @@ -33,6 +33,7 @@
>> #include "hw/loader.h"
>> #include "hw/qdev-properties.h"
>> #include "qom/object.h"
>> +#include "qapi/error.h"
>>
>> #define TYPE_ISA_VGA "isa-vga"
>> OBJECT_DECLARE_SIMPLE_TYPE(ISAVGAState, ISA_VGA)
>> @@ -61,6 +62,15 @@ static void vga_isa_realizefn(DeviceState *dev, Error **errp)
>> MemoryRegion *vga_io_memory;
>> const MemoryRegionPortio *vga_ports, *vbe_ports;
>>
>> + /*
>> + * make sure this device is not being added twice, if so
>> + * exit without crashing qemu
>> + */
>> + if (qemu_ram_block_by_name("vga.vram")) {
>> + error_setg(errp, "vga.vram is already registered");
> Maybe "'isa-vga' device already registered" is easier to understand?
> (vga.vram is about the device model internal).
Much better message, sending a v3 with this improvement.
Thank you!
>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>> + return;
>> + }
>> +
>> s->global_vmstate = true;
>> vga_common_init(s, OBJECT(dev));
>> s->legacy_address_space = isa_address_space(isadev);
>>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-16 13:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-16 12:36 [PATCH v2] vga: don't abort when adding a duplicate isa-vga device Jose R. Ziviani
2021-08-16 13:01 ` Philippe Mathieu-Daudé
2021-08-16 13:53 ` Jose Ricardo Ziviani
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.