Hi Michal, exactly ... that was what I tried to explain! :-) Gave the patch a go and it works just fine - thanks!
Cheers, Martin On 11.02.2019 12:09, Michal Necasek wrote: > > 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 >> > > > _______________________________________________ > 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
