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 <[email protected]> Date: Fri, May 15, 2020 8:13 pm To: [email protected] Cc: "[email protected]" <[email protected]> On Fri, May 15, 2020 at 05:03:30PM -0700, [email protected] 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 > > >
