VirtualBox

Ticket #8244 (closed defect: fixed)

Opened 3 years ago

Last modified 3 months ago

BIOS32 PCI service wrong length

Reported by: JdeBP Owned by:
Priority: major Component: other
Version: VirtualBox 4.0.2 Keywords:
Cc: Guest type: other
Host type: other

Description (last modified by frank) (diff)

File: source:trunk/src/VBox/Devices/PC/BIOS/rombios.c

This is the same bug as bug ID #3170157 registered at the Bochs bug database on SourceForge. The equivalent Bochs source file is bochs/bios/rombios.c .

Just before the bios32_end label in rombios.c we have

mov ecx, #0

According to the specification this should be the length of the code/data segment used to access the service. It's definitely not supposed to be zero length. Given that the base specified in the line above is 0xF0000, this will provide a reasonable length value for now:

mov ecx, #0x10000

Change History

comment:1 follow-up: ↓ 2 Changed 3 years ago by michaln

Why is this marked as "major"? What problems does it cause in practice? Presumably some serious issues, so we'd like to know the details...

It's true that according to the specs, such as they are, the returned length shouldn't be zero. But who actually uses it? In other words, how do we validate that the proposed change does anything useful?

comment:2 in reply to: ↑ 1 Changed 3 years ago by JdeBP

Replying to michaln:

Why is this marked as "major"? What problems does it cause in practice? Presumably some serious issues, so we'd like to know the details...

The details are fairly obvious. The entire PCI BIOS API is unusable in protected mode. That's fairly major.

Replying to michaln:

It's true that according to the specs, such as they are, the returned length shouldn't be zero.

So fix it so that the program behaves according to specification. There's no "such as they are" here. The specification is clear and unambiguous as to what ECX should contain and what use is made of that value.

Replying to michaln:

But who actually uses it?

I do, for one. My program tries to call the PCI BIOS in protected mode. It sets up code and data segment descriptions of the length that it's told to supply for use, and — blam! — instant general protection fault because they're both zero length. Anyone who calls anything in the PCI BIOS from protected mode, setting up the segment descriptors according to the specification, will hit this.

comment:3 follow-up: ↓ 4 Changed 3 years ago by michaln

Which specification is clear and unambiguous as to what ECX should contain and what use is made of that value? Certainly not the Phoenix BIOS32 Service Directory proposal, which only says that "the meaning of the EBX, ECX, and EDX registers is dependent on the particular 32-bit BIOS service specification." But the PCI BIOS specification is hardly unambiguous - it defines neither how the "length of the BIOS service" should be determined nor how it is to be used. It only contains one somewhat vague statement, "The CS and DS descriptors must be setup to encompass the physical addresses specified by the Base and Length parameters returned by the BIOS32 Service Directory." You must have some better specs than we have. How can we obtain them? With clear and unambiguous specs, you should be able to do a lot better than "provide a reasonable length value for now"...

Are you suggesting that no actual operating system in widespread use (such as Windows, Linux, Solaris, BSD, etc.) is using the BIOS32 PCI services? That is the clear implication if it's entirely broken. But if that's the case, we'd be better off removing it entirely as it's just wasting valuable ROM space.

Or maybe you're wrong and real operating systems somehow magically manage to make the 32-bit PCI BIOS services to work? Perhaps by using selectors which cover the entire 4GB address space, or at least the entire first megabyte?

Again, a defect which affects a single user hardly qualifies as major.

comment:4 in reply to: ↑ 3 Changed 3 years ago by JdeBP

Replying to michaln:

Which specification is clear and unambiguous as to what ECX should contain and what use is made of that value? Certainly not the Phoenix BIOS32 Service Directory proposal, which only says that "the meaning of the EBX, ECX, and EDX registers is dependent on the particular 32-bit BIOS service specification."

The actual PCI BIOS Specification, version 2.1 dated 1994-08-26, from the PCI SIG. Section 3 is quite clear on what ECX should contain and what it is used for. Work from the actual standard, not a proposal.

Replying to michaln:

But the PCI BIOS specification ![...]

All of that lengthy quibbling over doing a very simple thing that (a) is the right thing to do according to the specification, (b) matches what real machines do (I've looked at the values. They're not zero and are set up properly.), and (c) is obviously correct (How could protected mode API users set up the correct segment descriptors if they aren't given the length?) is a house of cards predicated on an error in those five first words. You're not using the PCI BIOS specification. You're using a proposal from a year earlier. Read and make VirtualBox adhere to the actual PCI BIOS specification from the PCI SIG. It's quite clear on this part of the interface contract that rombios.c must adhere to.

Replying to michaln:

You must have some better specs than we have. How can we obtain them?

Easily: By going to  the PCI SIG's WWW site and downloading it. (Oracle is a member. Indeed, Wesley Shao of Sun/Oracle is on the board of directors.) By putting "PCI BIOS Specification" into one's favourite WWW search engine. By downloading bios21.pdf or pci_bios_21.pdf from various hobbyist WWW sites.

Replying to michaln:

With clear and unambiguous specs, you should be able to do a lot better than "provide a reasonable length value for now"...

That's blame-shifting nonsense. It's not up to us, the API users, to give you that. It's up to you, the firmware writers, to decide what portion of the ROM image in your firmware should be available in the code and data segments. That is, after all, the point of the API. 0x10000 simply makes all of it available. It's reasonable for now until you decide what actual window onto the ROM image you want to have. (For what it's worth, if memory serves at least one real firmware writer takes the same approach of just requesting that the whole 64KiB image be mapped.)

Replying to michaln:

Are you suggesting that no actual operating system in widespread use (such as Windows, Linux, Solaris, BSD, etc.) is using the BIOS32 PCI services?

No, that's you suggesting that. I'm telling you that the PCI BIOS API isn't usable in its entirety in protected mode with the base and length that the firmware asks for mapped as requested. You can possibly deduce that such operating systems only use the real mode services, or that they have VirtualBox-inspired bodges, but you're on somewhat shaky ground trying to do so, given how many outstanding "the operating system crashes" bugs VirtualBox currently has. Stop trying to pull the "You're the only person who has seen this." excuse. I'm the only person who has tracked down the exact line of assembly language that's wrong and handed the fix to you on a platter, perhaps. But you know no more than that.

comment:5 follow-up: ↓ 6 Changed 3 years ago by michaln

I was quoting the published PCI BIOS 2.1 spec. I don't know what gave you the idea that it was a draft PCI spec.

If you are seriously suggesting that operating systems like Windows XP - which predate VirtualBox - include some "VirtualBox-inspired bodges", I'm not sure how you expect to be taken seriously.

To the best of my knowledge, Linux kernels (at least 2.4 and older 2.6 series) can be made to use the 32-bit PCI BIOS services with the 'pci=bios' kernel argument, although they don't use the PCI BIOS by default. They certainly don't use (and AFAIK never did) the 16-bit PCI BIOS services. If you can find a Linux kernel that fails to boot because of the bochs/VirtualBox 32-bit BIOS services implementation, we'll have a good testcase. If you can't find one, perhaps this isn't such a major defect, after all?

comment:6 in reply to: ↑ 5 Changed 3 years ago by JdeBP

Replying to michaln:

I was quoting the published PCI BIOS 2.1 spec. I don't know what gave you the idea that it was a draft PCI spec.

False. You quoted page 7 of the Standard BIOS 32-bit Service Directory Proposal, revision 0.4 dated 1993-06-22. I have copies of both documents, and I can check what you're quoting, even if it weren't for the fact that you originally stated that that was what you quoted. What you quoted isn't in the actual specification. You should have both documents too. Go and read the actual specification and follow it.

Instead of doing so, you've bizarrely questioned what the concept of length is (when it's amply clear that you, the firmware writer, have to work out what length you require for your firmware to operate as coded, so that clients of the API can fulfil their part of the interface contract for you — at the moment you're saying that you require zero bytes of code and data space, and your firmware fails when that's what you are supplied with) and you're now resorting to falsifying which document you're reading and quoting from, all to avoid a simple 1-line fix that's in accordance with the specification and fixes an obviously incorrect zero value. You should be ashamed.

Replying to michaln:

If you are seriously suggesting that operating systems like Windows XP - which predate VirtualBox - include some "VirtualBox-inspired bodges", I'm not sure how you expect to be taken seriously.

No, that's once again you suggesting that. You're blame shifting, working to a 1993 draft instead of to the 1994 specification, and inventing straw men about Windows XP (which I made zero mention of) all to avoid addressing fixing something that's obviously incorrect on its face, not in agreement with the PCI BIOS Specification, and simple to fix. Shame on you! This is terrible bug-report dodging on your part.

Replying to michaln:

If you can find a Linux kernel that fails to boot because of the bochs/VirtualBox 32-bit BIOS services implementation, we'll have a good testcase.

Again you play games to dodge a bug report. I have software that fails in this scenario; I don't need to find more. Yet here you are trying to dodge a bug report by demanding that the reporter, who has diagnosed the bug and has kindly even identified the exact line of code that's wrong and given you a reasonable fix, go and do a lot more work to write a unit test for you as well. Do your own job and go and write your own unit test. Don't demand that people who report bugs and who hand you the fix on a platter go and do all of your job for you.

comment:7 Changed 3 years ago by frank

JdeBP, calm down. The only question michaln raised was if this issue is a major bug or not. We think this is a minor issue as most guests don't care (and especially the officially supported guests). You obviously demand that this is a major bug. Thank you for this bug report. We will fix it sooner or later.

comment:8 Changed 3 months ago by frank

  • Status changed from new to closed
  • Resolution set to fixed
  • Description modified (diff)

This was fixed in VBox 4.3.x.

Note: See TracTickets for help on using tickets.

www.oracle.com
ContactPrivacy policyTerms of Use