Ticket #15787 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

VBE function 4F06 subfuction BL = 02 BUG in 16 colour modes

Reported by: ErikB Owned by:
Component: other Version: VirtualBox 5.1.2
Keywords: Cc:
Guest type: all Host type: all


Please find attached ‘VirtualBox VBE 4F06_02 BUG in 16 colour mode.png’ with a solution to a problem I found in the VBE of VirtualBox. When in 16 colour mode (102h, 104h and 106h), the sub function BL = 02 is using the CX value as if it was a BL = 00 call.


VirtualBox VBE 4F06_02 BUG in 16 colour mode.png Download (86.4 KB) - added by ErikB 6 years ago.
TEST_VBE.COM Download (512 bytes) - added by ErikB 6 years ago.
VirtualBox_Dos_Debug_VBE_4F06_02_BUG.png Download (28.3 KB) - added by ErikB 6 years ago.

Change History

comment:1 Changed 6 years ago by michael

Thanks for the report. Could you please provide a quick and simple to reproduce test case, involving publicly available software, which shows the problem and which should work correctly after applying the change? And since it has been a long time since I went near VBE, some similar test cases which are known to use that code which should work correctly before and after the change would be welcome too.

comment:2 Changed 6 years ago by michael

By the way, if you want to do the tests yourself, we can provide a build with the patch for you.

Edit: Please still document your findings and your tests here. Thanks!

Last edited 6 years ago by michael (previous) (diff)

comment:3 Changed 6 years ago by ErikB

More than 15 years ago, I developed new display drivers for Orcad SDT and PCB with 1024x768 and 1280x1024 resolution in 16 color mode, using the VBE standard.

The drivers are running on a variety of machines (Dell Inspiron among others). The drivers have never failed in more than 15 years of use.

I believe, a lot of programs use this required VBE function to speed up panning of graphical displays.

It is possible for me to modify the drivers, to use pixel count rather than byte count, and use the sub function BL=00h instead, but as I mention above, the BL=02h is a required function of the VBE standard.

I have actually already tested this modification with a patch to the drivers, and it runs perfectly on any OS guest using VirtualBox.

By the way, the bug is quite obvious:

What the function is supposed to do in 16 color modes, is to use the CX value multiplied by 8 (with the shl ax,3), converting the value from byte count to pixel count, before calling 'dispi_set_virt_width' in line 622.

First mistake: The CX value from entry is no longer in AX when reaching line 610. Here we have the return value from calling 'dispi_get_bpp' in line 604.

Second mistake: The value in AX after being multiplied by 8, is lost again before calling 'dispi_set_virt_width', by the 'pop ax' in line 619.

With the additional 'pop ax' and 'push ax' I suggest, you can correct both mistakes.

I would very much appreciate a test build from you, to verify the solution for you.

For your information: VirtualBox is already way ahead of Windows Virtual PC, that does not implement the 'Logical Scan Line Length' feature at all.

comment:4 Changed 6 years ago by michaln

OK, so in other words, you are not able/willing to provide or name any software that we could get which would show this problem?

The trouble is that 16-color VESA mode usage is not all that common. Yes, those functions are used for panning, but VBE + 16 colors + panning is not something done very often.

comment:5 Changed 6 years ago by michaln

BTW instead of adding a pop and push, why not move the existing 'pop ax' just before the 'jnz no_4bpp_1' line?

comment:6 Changed 6 years ago by ErikB

Moving the 'pop ax' from line 619 and inserting it between line 607 and 608 is a very good idea. Why didn't I think of that myself ?

The fact, that VBE + 16 colors + panning is not done very often, is probably the reason that this bug was never discovered before.

comment:7 Changed 6 years ago by michael

I wonder if this bug could be reproduced with a Linux guest using the VESA X server driver and a 16 colour mode with a large virtual resolution. Perhaps you could try that?

comment:8 Changed 6 years ago by ErikB

I don't know, why you want to spend more time to reproduce this problem. As we already agreed, it is quite obvious just by reading the assembler code, that the bug is there.

It is just as obvious, that moving the 'pop ax', as you suggested, will solve the problem. That is really all there is to it. Move a single line of code a few lines up.

comment:9 Changed 6 years ago by michael

I am not one of the people who maintains the VBE code, and in fact no one touches it more than very occasionally. But at least in code I maintain I am used to the fact that code changes which are obviously right sometimes cause unexpected and not immediately obvious problems. I do not expect this to be different in a section of little-used x86 assembly, hence the polite request for some tests to persuade us that what is obviously correct really is. Since you are the only person I am aware of in many years who has expressed interest in this is seems reasonable to ask you to provide the tests. I suspect that discussing whether or not they are needed could easily end up actually taking more time.

Changed 6 years ago by ErikB

comment:10 Changed 6 years ago by ErikB

I replaced the attachment file with a new version to show the modification as discussed in comment 5 and 6.

In comment 2, you offer to provide a build with the patch for me, to test the modification for you. I still consider this to be the easiest solution to prove the modification is correct.

Last edited 6 years ago by ErikB (previous) (diff)

comment:11 Changed 6 years ago by ErikB

Meanwhile I have made a temporary patch to the VirtualBox VideoBios, using the VirtualBox debugger on start-up.

The patch is working perfectly, both with BL=00 where CX=pixel count, and with BL=02 where CX is byte count.

The BIOS patch I made exactly as described, moving the 'pop ax' up between line 607 and 608.

comment:12 Changed 6 years ago by michaln

Is there some reason you're so so unwilling to provide something, anything, that exhibits the problem? Is there something secret? Is it just too hard?

Changed 6 years ago by ErikB

Changed 6 years ago by ErikB

comment:13 Changed 6 years ago by ErikB

I have made a small test program ('TEST_VBE.COM'). You can run it with DEBUG from a DOS command line.

It will capture the return values from INT 10, 4F06 BL=02, and store the values at DS:0210~0217

It clearly shows, that the return values are wrong until I move the VBE BIOS 'pop ax'. After moving the 'pop ax', I get the correct return values, and the function operates properly.

Have a look at 'VirtualBox_Dos_Debug_VBE_4F06_02_BUG.png' where you can see the difference.

Last edited 6 years ago by ErikB (previous) (diff)

comment:14 Changed 6 years ago by frank

  • Status changed from new to closed
  • Resolution set to fixed

Fix is part of VBox 5.1.6.

Note: See TracTickets for help on using tickets.
ContactPrivacy policyTerms of Use