The QEMU model for the Bochs display has no VGA memory section at offset
0x400 [1]. By writing to this register Linux can create a write to
unassigned memory which depending on machine and architecture can result
in a store fault.
I don't see any reference to this address at OSDev [2] or in the Bochs
source code.
Removing this write still allows graphics to work inside QEMU with
the bochs-display.
1: https://gitlab.com/qemu-project/qemu/-/blob/master/hw/display/bochs-display.c#L264
2. https://wiki.osdev.org/Bochs_VBE_Extensions
Reported-by: Khem Raj <[email protected]>
Signed-off-by: Alistair Francis <[email protected]>
---
drivers/gpu/drm/bochs/bochs_hw.c | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/drivers/gpu/drm/bochs/bochs_hw.c b/drivers/gpu/drm/bochs/bochs_hw.c
index b615b7dfdd9d..dfb2a5363c62 100644
--- a/drivers/gpu/drm/bochs/bochs_hw.c
+++ b/drivers/gpu/drm/bochs/bochs_hw.c
@@ -10,19 +10,6 @@
/* ---------------------------------------------------------------------- */
-static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
-{
- if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df))
- return;
-
- if (bochs->mmio) {
- int offset = ioport - 0x3c0 + 0x400;
- writeb(val, bochs->mmio + offset);
- } else {
- outb(val, ioport);
- }
-}
-
static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg)
{
u16 ret = 0;
@@ -217,8 +204,6 @@ void bochs_hw_setmode(struct bochs_device *bochs,
bochs->xres, bochs->yres, bochs->bpp,
bochs->yres_virtual);
- bochs_vga_writeb(bochs, 0x3c0, 0x20); /* unblank */
-
bochs_dispi_write(bochs, VBE_DISPI_INDEX_ENABLE, 0);
bochs_dispi_write(bochs, VBE_DISPI_INDEX_BPP, bochs->bpp);
bochs_dispi_write(bochs, VBE_DISPI_INDEX_XRES, bochs->xres);
--
2.25.0
On Thu, Feb 27, 2020 at 01:04:54PM -0800, Alistair Francis wrote:
> The QEMU model for the Bochs display has no VGA memory section at offset
> 0x400 [1]. By writing to this register Linux can create a write to
> unassigned memory which depending on machine and architecture can result
> in a store fault.
>
> I don't see any reference to this address at OSDev [2] or in the Bochs
> source code.
>
> Removing this write still allows graphics to work inside QEMU with
> the bochs-display.
It's not that simple. The driver also handles the qemu stdvga (-device
VGA, -device secondary-vga) which *does* need the vga port write.
There is no way for the guest to figure whenever the device is
secondary-vga or bochs-display.
So how about fixing things on the host side? Does qemu patch below
help?
Maybe another possible approach is to enable/disable vga access per
arch. On x86 this doesn't cause any problems. I guess you are on
risc-v?
cheers,
Gerd
diff --git a/hw/display/bochs-display.c b/hw/display/bochs-display.c
index 62085f9fc063..e93e838243b8 100644
--- a/hw/display/bochs-display.c
+++ b/hw/display/bochs-display.c
@@ -151,6 +151,26 @@ static const MemoryRegionOps bochs_display_qext_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
+static uint64_t dummy_read(void *ptr, hwaddr addr, unsigned size)
+{
+ return -1;
+}
+
+static void dummy_write(void *ptr, hwaddr addr,
+ uint64_t val, unsigned size)
+{
+}
+
+static const MemoryRegionOps dummy_ops = {
+ .read = dummy_read,
+ .write = dummy_write,
+ .valid.min_access_size = 1,
+ .valid.max_access_size = 4,
+ .impl.min_access_size = 1,
+ .impl.max_access_size = 1,
+ .endianness = DEVICE_LITTLE_ENDIAN,
+};
+
static int bochs_display_get_mode(BochsDisplayState *s,
BochsDisplayMode *mode)
{
@@ -284,8 +304,8 @@ static void bochs_display_realize(PCIDevice *dev, Error **errp)
memory_region_init_io(&s->qext, obj, &bochs_display_qext_ops, s,
"qemu extended regs", PCI_VGA_QEXT_SIZE);
- memory_region_init(&s->mmio, obj, "bochs-display-mmio",
- PCI_VGA_MMIO_SIZE);
+ memory_region_init_io(&s->mmio, obj, &dummy_ops, NULL,
+ "bochs-display-mmio", PCI_VGA_MMIO_SIZE);
memory_region_add_subregion(&s->mmio, PCI_VGA_BOCHS_OFFSET, &s->vbe);
memory_region_add_subregion(&s->mmio, PCI_VGA_QEXT_OFFSET, &s->qext);
On Fri, Feb 28, 2020 at 1:57 AM Gerd Hoffmann <[email protected]> wrote:
>
> On Thu, Feb 27, 2020 at 01:04:54PM -0800, Alistair Francis wrote:
> > The QEMU model for the Bochs display has no VGA memory section at offset
> > 0x400 [1]. By writing to this register Linux can create a write to
> > unassigned memory which depending on machine and architecture can result
> > in a store fault.
> >
> > I don't see any reference to this address at OSDev [2] or in the Bochs
> > source code.
> >
> > Removing this write still allows graphics to work inside QEMU with
> > the bochs-display.
>
> It's not that simple. The driver also handles the qemu stdvga (-device
> VGA, -device secondary-vga) which *does* need the vga port write.
> There is no way for the guest to figure whenever the device is
> secondary-vga or bochs-display.
>
> So how about fixing things on the host side? Does qemu patch below
> help?
That patch looks like it will fix the problem, but it doesn't seem
like the correct fix. I would rather avoid adding a large chunk of
dummy I/O to handle the two devices.
>
> Maybe another possible approach is to enable/disable vga access per
> arch. On x86 this doesn't cause any problems. I guess you are on
> risc-v?
I would prefer this option. I do see this on RISC-V, but I suspect the
issue will appear on other architectures (although how they handle I/O
failures in QEMU is a different story).
Can I just do the VGA write if x86?
Alistair
>
> cheers,
> Gerd
>
> diff --git a/hw/display/bochs-display.c b/hw/display/bochs-display.c
> index 62085f9fc063..e93e838243b8 100644
> --- a/hw/display/bochs-display.c
> +++ b/hw/display/bochs-display.c
> @@ -151,6 +151,26 @@ static const MemoryRegionOps bochs_display_qext_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> +static uint64_t dummy_read(void *ptr, hwaddr addr, unsigned size)
> +{
> + return -1;
> +}
> +
> +static void dummy_write(void *ptr, hwaddr addr,
> + uint64_t val, unsigned size)
> +{
> +}
> +
> +static const MemoryRegionOps dummy_ops = {
> + .read = dummy_read,
> + .write = dummy_write,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 4,
> + .impl.min_access_size = 1,
> + .impl.max_access_size = 1,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> static int bochs_display_get_mode(BochsDisplayState *s,
> BochsDisplayMode *mode)
> {
> @@ -284,8 +304,8 @@ static void bochs_display_realize(PCIDevice *dev, Error **errp)
> memory_region_init_io(&s->qext, obj, &bochs_display_qext_ops, s,
> "qemu extended regs", PCI_VGA_QEXT_SIZE);
>
> - memory_region_init(&s->mmio, obj, "bochs-display-mmio",
> - PCI_VGA_MMIO_SIZE);
> + memory_region_init_io(&s->mmio, obj, &dummy_ops, NULL,
> + "bochs-display-mmio", PCI_VGA_MMIO_SIZE);
> memory_region_add_subregion(&s->mmio, PCI_VGA_BOCHS_OFFSET, &s->vbe);
> memory_region_add_subregion(&s->mmio, PCI_VGA_QEXT_OFFSET, &s->qext);
>
>
On Mon, Mar 02, 2020 at 02:14:02PM -0800, Alistair Francis wrote:
> On Fri, Feb 28, 2020 at 1:57 AM Gerd Hoffmann <[email protected]> wrote:
> >
> > On Thu, Feb 27, 2020 at 01:04:54PM -0800, Alistair Francis wrote:
> > > The QEMU model for the Bochs display has no VGA memory section at offset
> > > 0x400 [1]. By writing to this register Linux can create a write to
> > > unassigned memory which depending on machine and architecture can result
> > > in a store fault.
> > >
> > > I don't see any reference to this address at OSDev [2] or in the Bochs
> > > source code.
> > >
> > > Removing this write still allows graphics to work inside QEMU with
> > > the bochs-display.
> >
> > It's not that simple. The driver also handles the qemu stdvga (-device
> > VGA, -device secondary-vga) which *does* need the vga port write.
> > There is no way for the guest to figure whenever the device is
> > secondary-vga or bochs-display.
> >
> > So how about fixing things on the host side? Does qemu patch below
> > help?
>
> That patch looks like it will fix the problem, but it doesn't seem
> like the correct fix. I would rather avoid adding a large chunk of
> dummy I/O to handle the two devices.
It's just a single handler for the parent mmio region, so we have a
defined default action instead of undefined behavior.
Patch just posted to qemu-devel, lets see what others think ...
> > Maybe another possible approach is to enable/disable vga access per
> > arch. On x86 this doesn't cause any problems. I guess you are on
> > risc-v?
>
> I would prefer this option. I do see this on RISC-V, but I suspect the
> issue will appear on other architectures (although how they handle I/O
> failures in QEMU is a different story).
>
> Can I just do the VGA write if x86?
I know ppc needs it too. Not sure about other architectures. I'd
suggest to do it the other way around: blacklist known-problematic
archs.
cheers,
Gerd
On Mon, Mar 2, 2020 at 10:24 PM Gerd Hoffmann <[email protected]> wrote:
>
> On Mon, Mar 02, 2020 at 02:14:02PM -0800, Alistair Francis wrote:
> > On Fri, Feb 28, 2020 at 1:57 AM Gerd Hoffmann <[email protected]> wrote:
> > >
> > > On Thu, Feb 27, 2020 at 01:04:54PM -0800, Alistair Francis wrote:
> > > > The QEMU model for the Bochs display has no VGA memory section at offset
> > > > 0x400 [1]. By writing to this register Linux can create a write to
> > > > unassigned memory which depending on machine and architecture can result
> > > > in a store fault.
> > > >
> > > > I don't see any reference to this address at OSDev [2] or in the Bochs
> > > > source code.
> > > >
> > > > Removing this write still allows graphics to work inside QEMU with
> > > > the bochs-display.
> > >
> > > It's not that simple. The driver also handles the qemu stdvga (-device
> > > VGA, -device secondary-vga) which *does* need the vga port write.
> > > There is no way for the guest to figure whenever the device is
> > > secondary-vga or bochs-display.
> > >
> > > So how about fixing things on the host side? Does qemu patch below
> > > help?
> >
> > That patch looks like it will fix the problem, but it doesn't seem
> > like the correct fix. I would rather avoid adding a large chunk of
> > dummy I/O to handle the two devices.
>
> It's just a single handler for the parent mmio region, so we have a
> defined default action instead of undefined behavior.
>
> Patch just posted to qemu-devel, lets see what others think ...
Thanks, let's wait and see what happens.
>
> > > Maybe another possible approach is to enable/disable vga access per
> > > arch. On x86 this doesn't cause any problems. I guess you are on
> > > risc-v?
> >
> > I would prefer this option. I do see this on RISC-V, but I suspect the
> > issue will appear on other architectures (although how they handle I/O
> > failures in QEMU is a different story).
> >
> > Can I just do the VGA write if x86?
>
> I know ppc needs it too. Not sure about other architectures. I'd
> suggest to do it the other way around: blacklist known-problematic
> archs.
Argh, that is a little uglier. Let's circle back after receiving
feedback on the QEMU patch.
Alistair
>
> cheers,
> Gerd
>