VirtualBox

Ticket #15787 (closed defect: fixed)

Opened 16 months ago

Last modified 15 months ago

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

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

Description

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.

Attachments

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

Change History

comment:1 Changed 16 months 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 16 months 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 16 months ago by michael (previous) (diff)

comment:3 Changed 16 months 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 16 months 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 16 months 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 16 months 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 15 months 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 15 months 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 15 months 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 15 months ago by ErikB

comment:10 Changed 15 months 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 15 months ago by ErikB (previous) (diff)

comment:11 Changed 15 months 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 15 months 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 15 months ago by ErikB

Changed 15 months ago by ErikB

comment:13 Changed 15 months 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 15 months ago by ErikB (previous) (diff)

comment:14 Changed 15 months 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.

www.oracle.com
ContactPrivacy policyTerms of Use