| Message ID | 20200326152438.6218-27-alexandru.elisei@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add reassignable BARs and PCIE 1.1 support | expand |
On 26/03/2020 15:24, Alexandru Elisei wrote: > A vesa device is used by the SDL, GTK or VNC framebuffers. Don't allow the > user to specify more than one of these options because kvmtool will create > identical devices and bad things will happen: > > $ ./lkvm run -c2 -m2048 -k bzImage --sdl --gtk > # lkvm run -k bzImage -m 2048 -c 2 --name guest-10159 > Error: device region [d0000000-d012bfff] would overlap device region [d0000000-d012bfff] > *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** > *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** > *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** > ======= Backtrace: ========= > ======= Backtrace: ========= > /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] > ======= Backtrace: ========= > /lib/x86_64-linux-gnu/libc.so.6/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] > (+0x777e5)[0x7fae0ed447e5] > /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] > /lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] > /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7fae0ed5153c] > *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** > /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] > /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] > /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] > /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] > ======= Backtrace: ========= > Aborted (core dumped) > > The vesa device is explicitly created during the initialization phase of > the above framebuffers. Remove the superfluous check for their existence. > Not really happy about this pointer comparison, but I don't see a better way, and it's surely good enough for that purpose. > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> Reviewed-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre > --- > hw/vesa.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/hw/vesa.c b/hw/vesa.c > index dd59a112330b..8071ad153f27 100644 > --- a/hw/vesa.c > +++ b/hw/vesa.c > @@ -61,8 +61,11 @@ struct framebuffer *vesa__init(struct kvm *kvm) > BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); > BUILD_BUG_ON(VESA_MEM_SIZE < VESA_BPP/8 * VESA_WIDTH * VESA_HEIGHT); > > - if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) > - return NULL; > + if (device__find_dev(vesa_device.bus_type, vesa_device.dev_num) == &vesa_device) { > + r = -EEXIST; > + goto out_error; > + } > + > vesa_base_addr = pci_get_io_port_block(PCI_IO_SIZE); > r = ioport__register(kvm, vesa_base_addr, &vesa_io_ops, PCI_IO_SIZE, NULL); > if (r < 0) >
Hi, On 4/2/20 9:58 AM, André Przywara wrote: > On 26/03/2020 15:24, Alexandru Elisei wrote: >> A vesa device is used by the SDL, GTK or VNC framebuffers. Don't allow the >> user to specify more than one of these options because kvmtool will create >> identical devices and bad things will happen: >> >> $ ./lkvm run -c2 -m2048 -k bzImage --sdl --gtk >> # lkvm run -k bzImage -m 2048 -c 2 --name guest-10159 >> Error: device region [d0000000-d012bfff] would overlap device region [d0000000-d012bfff] >> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** >> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** >> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** >> ======= Backtrace: ========= >> ======= Backtrace: ========= >> /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] >> ======= Backtrace: ========= >> /lib/x86_64-linux-gnu/libc.so.6/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] >> (+0x777e5)[0x7fae0ed447e5] >> /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] >> /lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] >> /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7fae0ed5153c] >> *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** >> /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] >> /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] >> /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] >> /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] >> ======= Backtrace: ========= >> Aborted (core dumped) >> >> The vesa device is explicitly created during the initialization phase of >> the above framebuffers. Remove the superfluous check for their existence. >> > Not really happy about this pointer comparison, but I don't see a better > way, and it's surely good enough for that purpose. I've been giving this patch some thought. I think the fix is wrong. The problem is that the user is allowed to specify more than one of --gtk, --sdl and --vnc. I'll rewrite the patch to fix that, and print an error message that is actually useful for the user. Thanks, Alex > >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > > Cheers, > Andre > >> --- >> hw/vesa.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/hw/vesa.c b/hw/vesa.c >> index dd59a112330b..8071ad153f27 100644 >> --- a/hw/vesa.c >> +++ b/hw/vesa.c >> @@ -61,8 +61,11 @@ struct framebuffer *vesa__init(struct kvm *kvm) >> BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); >> BUILD_BUG_ON(VESA_MEM_SIZE < VESA_BPP/8 * VESA_WIDTH * VESA_HEIGHT); >> >> - if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) >> - return NULL; >> + if (device__find_dev(vesa_device.bus_type, vesa_device.dev_num) == &vesa_device) { >> + r = -EEXIST; >> + goto out_error; >> + } >> + >> vesa_base_addr = pci_get_io_port_block(PCI_IO_SIZE); >> r = ioport__register(kvm, vesa_base_addr, &vesa_io_ops, PCI_IO_SIZE, NULL); >> if (r < 0) >>
diff --git a/hw/vesa.c b/hw/vesa.c index dd59a112330b..8071ad153f27 100644 --- a/hw/vesa.c +++ b/hw/vesa.c @@ -61,8 +61,11 @@ struct framebuffer *vesa__init(struct kvm *kvm) BUILD_BUG_ON(!is_power_of_two(VESA_MEM_SIZE)); BUILD_BUG_ON(VESA_MEM_SIZE < VESA_BPP/8 * VESA_WIDTH * VESA_HEIGHT); - if (!kvm->cfg.vnc && !kvm->cfg.sdl && !kvm->cfg.gtk) - return NULL; + if (device__find_dev(vesa_device.bus_type, vesa_device.dev_num) == &vesa_device) { + r = -EEXIST; + goto out_error; + } + vesa_base_addr = pci_get_io_port_block(PCI_IO_SIZE); r = ioport__register(kvm, vesa_base_addr, &vesa_io_ops, PCI_IO_SIZE, NULL); if (r < 0)
A vesa device is used by the SDL, GTK or VNC framebuffers. Don't allow the user to specify more than one of these options because kvmtool will create identical devices and bad things will happen: $ ./lkvm run -c2 -m2048 -k bzImage --sdl --gtk # lkvm run -k bzImage -m 2048 -c 2 --name guest-10159 Error: device region [d0000000-d012bfff] would overlap device region [d0000000-d012bfff] *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** ======= Backtrace: ========= ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6/lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] (+0x777e5)[0x7fae0ed447e5] /lib/x86_64-linux-gnu/libc.so.6(+0x777e5)[0x7fae0ed447e5] /lib/x86_64-linux-gnu/libc.so.6(+0x8037a)[0x7fae0ed4d37a] /lib/x86_64-linux-gnu/libc.so.6(cfree+0x4c)[0x7fae0ed5153c] *** Error in `./lkvm': free(): invalid pointer: 0x00007fad78002e40 *** /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] /lib/x86_64-linux-gnu/libglib-2.0.so.0(g_string_free+0x3b)[0x7fae0f814dab] /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] /usr/lib/x86_64-linux-gnu/libgtk-3.so.0(+0x21121c)[0x7fae1023321c] ======= Backtrace: ========= Aborted (core dumped) The vesa device is explicitly created during the initialization phase of the above framebuffers. Remove the superfluous check for their existence. Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com> --- hw/vesa.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)