IPMI get device ID returns wrong revision leading to ipmitool mc info displaying wrong firmware revision

[marthauk]

Setting
busctl set-property xyz.openbmc_project.Software.BMC.Updater /xyz/openbmc_project/software/<software> xyz.openbmc_project.Software.Version Version s "v11.10-4-gd8cd888-dirty"

as in #89, leads to:

ipmitool -I lanplus -H <bmc> -U <user>-P <pass> raw 6 1
00 00 11 16 02 00 cf c2 00 00 00 00 00 00 00

and

ipmitool -I lanplus -H <bmc> -U <user>-P <pass> mc info
Device ID                 : 0
Device Revision           : 0
Firmware Revision         : 17.16
IPMI Version              : 2.0
....

Expected raw response is 0b 10, not 11 16. Suspicion is that a type/conversion fault happened when moving from apphandler.C (old) to apphandler.cpp (new).

At the time #89 was written, major and minor were defined in this struct (apphandler.C):

typedef struct
{
    char major;
    char minor;
    uint16_t d[2];
} rev_t;

In apphandler.cpp now has major and minor as uint8_t, and that the conversions for major and minor still cast to char.
rev.minor = static_cast<char>(std::stoi(s.substr(0, location), 0, 16));
I see the actual encoding to binary encoding (major) and BCD encoding(minor) remains the same as it was in the apphandler.C

 if (r >= 0)
        {
            // bit7 identifies if the device is available
            // 0=normal operation
            // 1=device firmware, SDR update,
            // or self-initialization in progress.
            // The availability may change in run time, so mask here
            // and initialize later.
            
            devId.fw[0] = rev.major & ipmiDevIdFw1Mask;
            rev.minor = (rev.minor > 99 ? 99 : rev.minor); --> This will yield false 99 for a lot of digits 
            devId.fw[1] = rev.minor % 10 + (rev.minor / 10) * 16;
            std::memcpy(&devId.aux, rev.d, 4);
        }

Setting the version string to "v11.93-4-gd8cd888-dirty"
yields

ipmitool -I lanplus -H <bmc> -U <user>-P <pass> raw 6 1
00 00 11 99 02 00 cf c2 00 00 00 00 00 00 00

and

ipmitool -I lanplus -H <bmc> -U <user>-P <pass> mc info
Device ID                 : 0
Device Revision           : 0
Firmware Revision         : 17.99
IPMI Version              : 2.0
....```

[vmauery]

The encoding is clearly correct, it looks like parsing the revision string is not going how you expect.
The function you want to look at there is convertVersion. This takes the string and outputs the Revision object. The parts of the string are all converted as hex values. It sounds like this is not what you expect.

[marthauk]

Yes, thank you, looking closer at convertVersion I see that it expects the firmware version string to be in hex. That is not what I expected. Didn't think that openbmc expected decimal version strings either?
If that is expected this ticket can probably be closed.