Hi Martin,
I think I now finally understand what's happening. The checksum
calculation code in DevPcBios.cpp is wrong... *but* it actually delivers
the correct result, as long as the BIOS image was run through 'biossums'
first!
Here's why: biossums calculates the checksum of the _DMI_ header and
the _SM_ header (at that point not containing final values), and then
calculates and updates the checksum of the entire BIOS image. Now, the
8-bit checksum of the _DMI_ header is 0, and the checksum of the entire
BIOS image is also 0. Which means that when you calculate the checksum
of the 15 _DMI_ header bytes, you'll get some value, and when you add
the checksum of the rest of the BIOS image, the result won't change.
You probably do not have the correct checksum of the entire BIOS
image, and that is why the _DMI_ checksum calculation produces a bogus
result for you. I have the checksum right, so the obviously wrong code
still gets the right answer.
I decided to throw the checksumming code in DevPcBios.cpp out
completely and replace it with the already existing
FwCommonPlantSmbiosAndDmiHdrs() function. That should produce the right
checksums regardless of whether the existing checksums in the BIOS image
are correct or not. Patch is attached, please let me know if it works
for you.
Cheers,
Michal
On 2/5/2019 5:57 PM, Martin Fleisz wrote:
Hi Michal,
when you look at the source of dmidecode that is not really the case.
You are right that dmidecode first looks for the _SM_ header. But then
it will check the _SM_ checksum, the _DMI_ signature and the _DMI_
checksum (see
https://github.com/mirror/dmidecode/blob/master/dmidecode.c#L5379). If
any of these is wrong/missing it will fail with the error message from
my previous mail.
In both our cases the _DMI_ checksum is wrong (as you stated the code
obviously calculates the wrong checksum).
The _SM_ checksum is just wrong because of the incorrect value for the
_DMI_ checksum. If the _DMI_ checksum was calculated correctly, the _SM_
checksum is correct as it is now.
I hope that also makes sense ... maybe we even mean the same thing
anyways ... :-)
Cheers, Martin
On 05.02.2019 17:37, Michal Necasek wrote:
Hi Martin,
That's not what I'm saying. What I'm saying is that because my BIOS
image was run through biossums, I didn't see the dmidecode problem
because although the _DMI_ header checksum is wrong (incorrect
calculation in DevPcBios.cpp), it does not matter in practice because
dmidecode finds the _SM_ header first.
In other words, for you both _SM_ and _DMI_ headers had bad
checksums, but for me only _DMI_ did. After your fix (which is
correct), the _SM_ header you have probably still has a bad checksum
but dmidecode does not care because it finds the valid _DMI_ header.
I hope that made sense.
- Michal
On 2/5/2019 5:27 PM, Martin Fleisz wrote:
Hi Michal,
I don't think that biossums is the way you can fix this issue because in
DevPcBios.cpp the _DMI_ table is patched dynamically (see
https://www.virtualbox.org/browser/vbox/trunk/src/VBox/Devices/PC/DevPcBios.cpp#L1594
where the number and size of the DMI tables is updated) and therefore
the checksum must be re-calculated. The _SM_ table checksum is not
affected by these changes (as long as the _DMI_ checksum is correctly).
Cheers, Martin
On 05.02.2019 17:02, Michal Necasek wrote:
Hi Martin,
Now it's making sense. The '_DMI_' table header checksum code is
wrong in DevPCBios.cpp. But dmidecode normally does not get that far,
because it finds the '_SM_' marker first, and that one has the right
checksum if the BIOS image got run through the 'biossums' tool. I
suspect that's not happening for you, and that is something which is
probably missing from our makefiles.
I can't see into your repository on github. But you're probably right
and it's not doing any harm. Anyway, now I see how to reproduce the
problem, thanks.
- Michal
On 2/5/2019 4:09 PM, Martin Fleisz wrote:
Hi Michal,
I compile the OSE version (with some very small modifications in
VBoxBiosAlternative386.asm
<https://github.com/thincast/VirtualBox-RDP-Server/commit/98e7959e484a0145e39619dc6dbc40d3b9282520#diff-9c7472aa47554e70acbb1a4298b673e4>
but that shouldn't do any harm to my understanding) and when running
dmidecode on Ubuntu 18.04 it reports "# No SMBIOS nor DMI entry point
found, sorry.". After applying the patch everything worked just fine.
I checked with VirtualBox 6.0.4 and it works just fine out of the box
... so I guess either the checksum is somehow valid by accident or
there
is some code missing/different in the OSE repository.
Best regards, Martin
On 05.02.2019 15:48, Michal Necasek wrote:
Hi Martin,
The patch looks good, but I'm struggling to reproduce the problem.
The dmidecode utility in Ubuntu 18.04.1 and 18.10 (dmidecode version
3.1 in both) does not generate any complaints. So what exactly does?
Regards,
Michal
On 2/4/2019 12:16 PM, Martin Fleisz wrote:
When using legacy BIOS the Intermediate Checksum of the SMBIOS Entry
Point Structure is calculated incorrectly.
The calculation starts at the wrong offset (at the beginning of
pu8PcBios rather than pu8PcBios[i]) and creates the sum over the
whole pu8PcBios data instead of just the next 15 bytes.
The patch below fixes checksum calculation and makes applications
like dmidecode work again in the guest.
The patch is contributed under the terms of the MIT license.
Cheers, Martin
diff --git src/VBox/Devices/PC/DevPcBios.cpp
src/VBox/Devices/PC/DevPcBios.cpp
index e967adff2f..7e56cc18fe 100644
--- src/VBox/Devices/PC/DevPcBios.cpp
+++ src/VBox/Devices/PC/DevPcBios.cpp
@@ -1594,9 +1594,9 @@ static DECLCALLBACK(int)
pcbiosConstruct(PPDMDEVINS pDevIns, int iInstance, PCF
*(uint16_t*)&pThis->pu8PcBios[i + 0x06] =
RT_H2LE_U16(cbDmiTables);
*(uint16_t*)&pThis->pu8PcBios[i + 0x0C] =
RT_H2LE_U16(cNumDmiTables);
uint8_t u8Sum = 0;
- for (unsigned j = 0; j < pThis->cbPcBios; j++)
- if (j != i + 0x05)
- u8Sum += pThis->pu8PcBios[j];
+ for (unsigned j = 0; j < 0x0F; j++)
+ if (j != 0x05)
+ u8Sum += pThis->pu8PcBios[i + j];
pThis->pu8PcBios[i + 0x05] = -u8Sum;
break;
}
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
Index: DevPcBios.cpp
===================================================================
--- DevPcBios.cpp (revision 128759)
+++ DevPcBios.cpp (working copy)
@@ -1581,23 +1581,20 @@
if (RT_FAILURE(rc))
return rc;
- for (unsigned i = 0; i < pThis->cbPcBios; i += 16)
+ /* Look for _SM_/_DMI_ anchor strings within the BIOS and replace the
table headers. */
+ for (unsigned i = 0; i < (pThis->cbPcBios - 16); i += 16)
{
- /* If the DMI table is located at the expected place, patch the DMI
table length and the checksum. */
if ( pThis->pu8PcBios[i + 0x00] == '_'
- && pThis->pu8PcBios[i + 0x01] == 'D'
+ && pThis->pu8PcBios[i + 0x01] == 'S'
&& pThis->pu8PcBios[i + 0x02] == 'M'
- && pThis->pu8PcBios[i + 0x03] == 'I'
- && pThis->pu8PcBios[i + 0x04] == '_'
- && *(uint16_t*)&pThis->pu8PcBios[i + 0x06] == 0)
+ && pThis->pu8PcBios[i + 0x03] == '_'
+ && pThis->pu8PcBios[i + 0x10] == '_'
+ && pThis->pu8PcBios[i + 0x11] == 'D'
+ && pThis->pu8PcBios[i + 0x12] == 'M'
+ && pThis->pu8PcBios[i + 0x13] == 'I'
+ && pThis->pu8PcBios[i + 0x14] == '_')
{
- *(uint16_t*)&pThis->pu8PcBios[i + 0x06] = RT_H2LE_U16(cbDmiTables);
- *(uint16_t*)&pThis->pu8PcBios[i + 0x0C] =
RT_H2LE_U16(cNumDmiTables);
- uint8_t u8Sum = 0;
- for (unsigned j = 0; j < pThis->cbPcBios; j++)
- if (j != i + 0x05)
- u8Sum += pThis->pu8PcBios[j];
- pThis->pu8PcBios[i + 0x05] = -u8Sum;
+ FwCommonPlantSmbiosAndDmiHdrs(pDevIns, pThis->pu8PcBios + i,
cbDmiTables, cNumDmiTables);
break;
}
}
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
