Corrupted text background color on VirtualBox

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

Corrupted text background color on VirtualBox

johnc
One of the first things I noticed when I tried OpenBSD in VirtualBox
was that doing ctrl-alt-2, then ctrl-alt-1 to switch back and forth
between virtual screens left the text mode background a dim red
color instead of black.

Here is someone else noting it, ten years ago:
http://daemonforums.org/showthread.php?t=4704

It doesn't happen on a real (nvidia) VGA, but unexpected behavior on
a virtual machine can be a sign that code is doing undefined things
on the hardware, which is worth investigating.

The call to vga_enable() at the end of vga_restore_palette() is what
triggers the behavior.

This appears to be truly ancient code, but I don't think it was ever
correct.

#define vga_enable(vh) \
        vga_raw_write(vh, 0, 0x20);

After resetting the attribute flip-flop, this is just setting the VGA
attribute index to an illegal register value -- there aren't 32
attribute registers.

The palette code doesn't even touch the attributes, so it doesn't
have to reset the flip-flop, either.

The vga_enable() was also used, along with another unnecessary flip
flop reset (it toggles between address and data, so after writing
an address and data, it is back where it started), in the
vga_attr_read and write functions.

Removing all this appears to work fine on both VirtualBox and Nvidia.

Perhaps there was errant code that wrote to the io port register at
some point, and having it dissapear into an "invalid" register hid
another bug?

Index: vga.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/vga.c,v
retrieving revision 1.69
diff -u -p -r1.69 vga.c
--- vga.c 17 Jun 2017 19:20:30 -0000 1.69
+++ vga.c 15 May 2020 23:38:32 -0000
@@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
  vga_raw_write(vh, VGA_DAC_READ, 0x00);
  for (i = 0; i < 3 * 256; i++)
  *palette++ = vga_raw_read(vh, VGA_DAC_DATA);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
 }
 
 void
@@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
  vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
  for (i = 0; i < 3 * 256; i++)
  vga_raw_write(vh, VGA_DAC_DATA, *palette++);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
-
- vga_enable(vh);
 }
 
 void
Index: vgavar.h
===================================================================
RCS file: /cvs/src/sys/dev/ic/vgavar.h,v
retrieving revision 1.13
diff -u -p -r1.13 vgavar.h
--- vgavar.h 26 Jul 2015 03:17:07 -0000 1.13
+++ vgavar.h 15 May 2020 23:38:32 -0000
@@ -96,9 +96,6 @@ static inline void _vga_gdc_write(struct
 #define vga_raw_write(vh, reg, value) \
  bus_space_write_1(vh->vh_iot, vh->vh_ioh_vga, reg, value)
 
-#define vga_enable(vh) \
- vga_raw_write(vh, 0, 0x20);
-
 static inline u_int8_t
 _vga_attr_read(struct vga_handle *vh, int reg)
 {
@@ -110,11 +107,6 @@ _vga_attr_read(struct vga_handle *vh, in
  vga_raw_write(vh, VGA_ATC_INDEX, reg);
  res = vga_raw_read(vh, VGA_ATC_DATAR);
 
- /* reset state XXX unneeded? */
- (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
-
- vga_enable(vh);
-
  return (res);
 }
 
@@ -126,11 +118,6 @@ _vga_attr_write(struct vga_handle *vh, i
 
  vga_raw_write(vh, VGA_ATC_INDEX, reg);
  vga_raw_write(vh, VGA_ATC_DATAW, val);
-
- /* reset state XXX unneeded? */
- (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
-
- vga_enable(vh);
 }
 
 static inline u_int8_t


Reply | Threaded
Open this post in threaded view
|

Re: Corrupted text background color on VirtualBox

Jonathan Gray-11
On Fri, May 15, 2020 at 05:03:30PM -0700, [hidden email] wrote:

> One of the first things I noticed when I tried OpenBSD in VirtualBox
> was that doing ctrl-alt-2, then ctrl-alt-1 to switch back and forth
> between virtual screens left the text mode background a dim red
> color instead of black.
>
> Here is someone else noting it, ten years ago:
> http://daemonforums.org/showthread.php?t=4704
>
> It doesn't happen on a real (nvidia) VGA, but unexpected behavior on
> a virtual machine can be a sign that code is doing undefined things
> on the hardware, which is worth investigating.
>
> The call to vga_enable() at the end of vga_restore_palette() is what
> triggers the behavior.
>
> This appears to be truly ancient code, but I don't think it was ever
> correct.
>
> #define vga_enable(vh) \
> vga_raw_write(vh, 0, 0x20);
>
> After resetting the attribute flip-flop, this is just setting the VGA
> attribute index to an illegal register value -- there aren't 32
> attribute registers.

bits 0-4 are the index register, bit 5 is some kind of an enable bit.

To quote Abrash

"bit 5 of the AC Index register should be set to 1 whenever palette RAM
is not being set (which is to say, all the time in your code, because
palette RAM should normally be set via the BIOS). When bit 5 is 0, video
data from display memory is no longer sent to palette RAM, and the
screen becomes a solid color—not normally a desirable state of affairs."

from
http://www.jagregory.com/abrash-black-book/#notes-on-setting-and-reading-registers

The Matrox G400 datasheet describes bit 5 as

"This bit controls use of the internal palette. If pas = 0, the host
CPU can read and write the palette, and the display is forced to the
overscan color. If pas = 1, the palette is used normally by the video
stream to translate color indices (CPU writes are inhibited and reads
return all `1's). Normally, the internal palette is loaded during the
blank time, since loading inhibits video translation."

from
http://www.bitsavers.org/pdf/matrox/G400SPEC_Jun1999.PDF
3-289 -> 3-290

>
> The palette code doesn't even touch the attributes, so it doesn't
> have to reset the flip-flop, either.
>
> The vga_enable() was also used, along with another unnecessary flip
> flop reset (it toggles between address and data, so after writing
> an address and data, it is back where it started), in the
> vga_attr_read and write functions.
>
> Removing all this appears to work fine on both VirtualBox and Nvidia.
>
> Perhaps there was errant code that wrote to the io port register at
> some point, and having it dissapear into an "invalid" register hid
> another bug?
>
> Index: vga.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/vga.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 vga.c
> --- vga.c 17 Jun 2017 19:20:30 -0000 1.69
> +++ vga.c 15 May 2020 23:38:32 -0000
> @@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
>   vga_raw_write(vh, VGA_DAC_READ, 0x00);
>   for (i = 0; i < 3 * 256; i++)
>   *palette++ = vga_raw_read(vh, VGA_DAC_DATA);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
>  }
>  
>  void
> @@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
>   vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
>   for (i = 0; i < 3 * 256; i++)
>   vga_raw_write(vh, VGA_DAC_DATA, *palette++);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
> -
> - vga_enable(vh);
>  }
>  
>  void
> Index: vgavar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/vgavar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 vgavar.h
> --- vgavar.h 26 Jul 2015 03:17:07 -0000 1.13
> +++ vgavar.h 15 May 2020 23:38:32 -0000
> @@ -96,9 +96,6 @@ static inline void _vga_gdc_write(struct
>  #define vga_raw_write(vh, reg, value) \
>   bus_space_write_1(vh->vh_iot, vh->vh_ioh_vga, reg, value)
>  
> -#define vga_enable(vh) \
> - vga_raw_write(vh, 0, 0x20);
> -
>  static inline u_int8_t
>  _vga_attr_read(struct vga_handle *vh, int reg)
>  {
> @@ -110,11 +107,6 @@ _vga_attr_read(struct vga_handle *vh, in
>   vga_raw_write(vh, VGA_ATC_INDEX, reg);
>   res = vga_raw_read(vh, VGA_ATC_DATAR);
>  
> - /* reset state XXX unneeded? */
> - (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
> -
> - vga_enable(vh);
> -
>   return (res);
>  }
>  
> @@ -126,11 +118,6 @@ _vga_attr_write(struct vga_handle *vh, i
>  
>   vga_raw_write(vh, VGA_ATC_INDEX, reg);
>   vga_raw_write(vh, VGA_ATC_DATAW, val);
> -
> - /* reset state XXX unneeded? */
> - (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
> -
> - vga_enable(vh);
>  }
>  
>  static inline u_int8_t
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Corrupted text background color on VirtualBox

johnc
In reply to this post by johnc
Ok, new theory:

VirtualBox ignores the pas bit, so writes to the attribute data register
after vga_enable() is called go ahead and modify the attribute palette
index 0, but on a real vga the write is ignored.

This implies something is writing to VGA_ATC_DATAW without resetting the
flip flop.

If I change vga_enable() to write 0x3f instead of 0x20, which is an
actual illegal register + pas instead of text palette 0 + pas, the text
background doesn't change.

If I explicitly write a 0 to VGA_ATC_DATAW after the vga_enable(), the
problem also doesn't appear; the background stays black.

If I explicitly write a 3 to VGA_ATC_DATAW after the vga_enable(), the
text background becomes cyan after the restore, which confirms that
VirtualBox is allowing the palette registers to be modified even when
pas is set.

Oddly, if I reset the flip-flop again after vga_enable(), which should
make the next write just an address change instead of doing anything,
the problem still happens.

So, I would propose:

Removing the flip flop reset and vga_reset() from vga_restore_palette()
since setting the 256 color VGA palette doesn't touch the attribute
registers at all.  This fixes VirtualBox.

Leave the vga_enable() code in vga_var.h for the vga_attr_read/write()
calls, where it probably is still needed, at least for old cards.

It looks like the only place those are ever called from is
vga_save_state() / vga_restore_state(), which only happens on ACPI
events.  That might still cause a problem on VirtualBox, but I don't
see a way to simulate a hibernate event, so it probably doesn't matter.

Alternately, vga_enable() could explicitly write a 0 after setting 0x20
which would work around the VirtualBox issue and not harm anything
else.


-------- Original Message --------
Subject: Re: Corrupted text background color on VirtualBox
From: Jonathan Gray <[hidden email]>
Date: Fri, May 15, 2020 8:13 pm
To: [hidden email]
Cc: "[hidden email]" <[hidden email]>

On Fri, May 15, 2020 at 05:03:30PM -0700, [hidden email]
wrote:

> One of the first things I noticed when I tried OpenBSD in VirtualBox
> was that doing ctrl-alt-2, then ctrl-alt-1 to switch back and forth
> between virtual screens left the text mode background a dim red
> color instead of black.
>
> Here is someone else noting it, ten years ago:
> http://daemonforums.org/showthread.php?t=4704
>
> It doesn't happen on a real (nvidia) VGA, but unexpected behavior on
> a virtual machine can be a sign that code is doing undefined things
> on the hardware, which is worth investigating.
>
> The call to vga_enable() at the end of vga_restore_palette() is what
> triggers the behavior.
>
> This appears to be truly ancient code, but I don't think it was ever
> correct.
>
> #define vga_enable(vh) \
> vga_raw_write(vh, 0, 0x20);
>
> After resetting the attribute flip-flop, this is just setting the VGA
> attribute index to an illegal register value -- there aren't 32
> attribute registers.

bits 0-4 are the index register, bit 5 is some kind of an enable bit.

To quote Abrash

"bit 5 of the AC Index register should be set to 1 whenever palette RAM
is not being set (which is to say, all the time in your code, because
palette RAM should normally be set via the BIOS). When bit 5 is 0, video
data from display memory is no longer sent to palette RAM, and the
screen becomes a solid color—not normally a desirable state of
affairs."

from
http://www.jagregory.com/abrash-black-book/#notes-on-setting-and-reading-registers

The Matrox G400 datasheet describes bit 5 as

"This bit controls use of the internal palette. If pas = 0, the host
CPU can read and write the palette, and the display is forced to the
overscan color. If pas = 1, the palette is used normally by the video
stream to translate color indices (CPU writes are inhibited and reads
return all `1's). Normally, the internal palette is loaded during the
blank time, since loading inhibits video translation."

from
http://www.bitsavers.org/pdf/matrox/G400SPEC_Jun1999.PDF
3-289 -> 3-290

>
> The palette code doesn't even touch the attributes, so it doesn't
> have to reset the flip-flop, either.
>
> The vga_enable() was also used, along with another unnecessary flip
> flop reset (it toggles between address and data, so after writing
> an address and data, it is back where it started), in the
> vga_attr_read and write functions.
>
> Removing all this appears to work fine on both VirtualBox and Nvidia.
>
> Perhaps there was errant code that wrote to the io port register at
> some point, and having it dissapear into an "invalid" register hid
> another bug?
>
> Index: vga.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/vga.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 vga.c
> --- vga.c 17 Jun 2017 19:20:30 -0000 1.69
> +++ vga.c 15 May 2020 23:38:32 -0000
> @@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
> vga_raw_write(vh, VGA_DAC_READ, 0x00);
> for (i = 0; i < 3 * 256; i++)
> *palette++ = vga_raw_read(vh, VGA_DAC_DATA);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
> }
>
> void
> @@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
> vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
> for (i = 0; i < 3 * 256; i++)
> vga_raw_write(vh, VGA_DAC_DATA, *palette++);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
> -
> - vga_enable(vh);
> }
>
> void
> Index: vgavar.h
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/vgavar.h,v
> retrieving revision 1.13
> diff -u -p -r1.13 vgavar.h
> --- vgavar.h 26 Jul 2015 03:17:07 -0000 1.13
> +++ vgavar.h 15 May 2020 23:38:32 -0000
> @@ -96,9 +96,6 @@ static inline void _vga_gdc_write(struct
> #define vga_raw_write(vh, reg, value) \
> bus_space_write_1(vh->vh_iot, vh->vh_ioh_vga, reg, value)
>
> -#define vga_enable(vh) \
> - vga_raw_write(vh, 0, 0x20);
> -
> static inline u_int8_t
> _vga_attr_read(struct vga_handle *vh, int reg)
> {
> @@ -110,11 +107,6 @@ _vga_attr_read(struct vga_handle *vh, in
> vga_raw_write(vh, VGA_ATC_INDEX, reg);
> res = vga_raw_read(vh, VGA_ATC_DATAR);
>
> - /* reset state XXX unneeded? */
> - (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
> -
> - vga_enable(vh);
> -
> return (res);
> }
>
> @@ -126,11 +118,6 @@ _vga_attr_write(struct vga_handle *vh, i
>
> vga_raw_write(vh, VGA_ATC_INDEX, reg);
> vga_raw_write(vh, VGA_ATC_DATAW, val);
> -
> - /* reset state XXX unneeded? */
> - (void) bus_space_read_1(vh->vh_iot, vh->vh_ioh_6845, 10);
> -
> - vga_enable(vh);
> }
>
> static inline u_int8_t
>
>
>

Reply | Threaded
Open this post in threaded view
|

Re: Corrupted text background color on VirtualBox

Jonathan Gray-11
On Fri, May 15, 2020 at 10:02:22PM -0700, [hidden email] wrote:

> Ok, new theory:
>
> VirtualBox ignores the pas bit, so writes to the attribute data register
> after vga_enable() is called go ahead and modify the attribute palette
> index 0, but on a real vga the write is ignored.
>
> This implies something is writing to VGA_ATC_DATAW without resetting the
> flip flop.
>
> If I change vga_enable() to write 0x3f instead of 0x20, which is an
> actual illegal register + pas instead of text palette 0 + pas, the text
> background doesn't change.
>
> If I explicitly write a 0 to VGA_ATC_DATAW after the vga_enable(), the
> problem also doesn't appear; the background stays black.
>
> If I explicitly write a 3 to VGA_ATC_DATAW after the vga_enable(), the
> text background becomes cyan after the restore, which confirms that
> VirtualBox is allowing the palette registers to be modified even when
> pas is set.
>
> Oddly, if I reset the flip-flop again after vga_enable(), which should
> make the next write just an address change instead of doing anything,
> the problem still happens.
>
> So, I would propose:
>
> Removing the flip flop reset and vga_reset() from vga_restore_palette()
> since setting the 256 color VGA palette doesn't touch the attribute
> registers at all.  This fixes VirtualBox.
>
> Leave the vga_enable() code in vga_var.h for the vga_attr_read/write()
> calls, where it probably is still needed, at least for old cards.
>
> It looks like the only place those are ever called from is
> vga_save_state() / vga_restore_state(), which only happens on ACPI
> events.  That might still cause a problem on VirtualBox, but I don't
> see a way to simulate a hibernate event, so it probably doesn't matter.
>
> Alternately, vga_enable() could explicitly write a 0 after setting 0x20
> which would work around the VirtualBox issue and not harm anything
> else.

It turns out the flip-flop reset in the palette functions is using the
wrong address.  _vga_attr_read()/_vga_attr_write() get it right.

ioh_vga 0x3c0
vh_ioh_6845 0x3d0 (unless mono)

vga_raw_read() uses ioh_vga so incorrectly reads 0x3ca to reset
instead of 0x3da.

This was a mistake made in importing code from FreeBSD

----------------------------
revision 1.48
date: 2009/02/01 14:37:22;  author: miod;  state: Exp;  lines: +78 -3;
Save the text mode color palette upon startup, and restore it when
switching consoles or when X11 exits. Almost all other operating systems
do this, and thus do not suffer from palette bugs in some X11 drivers.

From FreeBSD.
----------------------------

In FreeBSD they have

inb(adp->va_crtc_addr + 6)

where va_crtc_addr is set to COLOR_CRTC for non-mono

#define COLOR_CRTC (IO_CGA + 0x04)
#define IO_CGA 0x3D0

I agree the attempted flip-flop reset and pas setting can go:

Index: vga.c
===================================================================
RCS file: /cvs/src/sys/dev/ic/vga.c,v
retrieving revision 1.69
diff -u -p -r1.69 vga.c
--- vga.c 17 Jun 2017 19:20:30 -0000 1.69
+++ vga.c 16 May 2020 07:21:19 -0000
@@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
  vga_raw_write(vh, VGA_DAC_READ, 0x00);
  for (i = 0; i < 3 * 256; i++)
  *palette++ = vga_raw_read(vh, VGA_DAC_DATA);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
 }
 
 void
@@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
  vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
  for (i = 0; i < 3 * 256; i++)
  vga_raw_write(vh, VGA_DAC_DATA, *palette++);
-
- vga_raw_read(vh, 0x0a); /* reset flip/flop */
-
- vga_enable(vh);
 }
 
 void

Reply | Threaded
Open this post in threaded view
|

Re: Corrupted text background color on VirtualBox

Matthieu Herrb-3
On Sat, May 16, 2020 at 05:59:06PM +1000, Jonathan Gray wrote:
>

That fixes the console background issue for me on a KVM / virt-manager
virtual machine.

> Index: vga.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ic/vga.c,v
> retrieving revision 1.69
> diff -u -p -r1.69 vga.c
> --- vga.c 17 Jun 2017 19:20:30 -0000 1.69
> +++ vga.c 16 May 2020 07:21:19 -0000
> @@ -1213,8 +1213,6 @@ vga_save_palette(struct vga_config *vc)
>   vga_raw_write(vh, VGA_DAC_READ, 0x00);
>   for (i = 0; i < 3 * 256; i++)
>   *palette++ = vga_raw_read(vh, VGA_DAC_DATA);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
>  }
>  
>  void
> @@ -1231,10 +1229,6 @@ vga_restore_palette(struct vga_config *v
>   vga_raw_write(vh, VGA_DAC_WRITE, 0x00);
>   for (i = 0; i < 3 * 256; i++)
>   vga_raw_write(vh, VGA_DAC_DATA, *palette++);
> -
> - vga_raw_read(vh, 0x0a); /* reset flip/flop */
> -
> - vga_enable(vh);
>  }
>  
>  void

--
Matthieu Herrb

Reply | Threaded
Open this post in threaded view
|

Re: Corrupted text background color on VirtualBox

Stuart Henderson
In reply to this post by Jonathan Gray-11
Nice find! This change also fixes

vga1 at pci3 dev 0 function 0 "ASPEED Technology AST2000" rev 0x30

as seen on many server boards from smaller manufacturers (especially
common on Supermicro).