vpd-tool: Add module VPD sanity check #658

Alpana07 · 2025-09-02T14:20:00Z

vpd-manager: Add module VPD sanity check

Added new dbus API to perform the santity check for all the vpds.
It takes one eeprom path at a time and checks for it's header, TOC
and all it's records.

Test:
        busctl introspect com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager
        NAME                             TYPE      SIGNATURE RESULT/VALUE FLAGS
        .CollectFRUVPD                   method    o         -            -
        .GetExpandedLocationCode         method    sq        s            -
        .GetFRUsByExpandedLocationCode   method    s         ao           -
        .GetFRUsByUnexpandedLocationCode method    sq        ao           -
        .GetHardwarePath                 method    o         s            -
        .PerformVPDRecollection          method    -         -            -
        .ReadKeyword                     method    sv        v            -
        .UpdateKeyword                   method    sv        i            -
        .WriteKeyword                    method    ossay     i            -
        .WriteKeywordOnHardware          method    sv        i            -
        .deleteFRUVPD                    method    o         -            -
        .performVpdSanityCheck          method    s         -            -
        .CollectionStatus                property  s         "Completed"  emits-change writable

        busctl  call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager performVpdSanityCheck  s "/sys/bus/spi/drivers/at25/spi12.0/eeprom"

        Journal-
        vpd-manager[1078]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1011 Performing sanity check for file path: /sys/bus/spi/drivers/at25/spi12.0/eeprom
        vpd-manager[1078]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 206 DBG: test purpose:checkHeader done
        vpd-manager[1078]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 246 DBG: test purpose:readTOC done
        vpd-manager[1078]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 297 DBG: test purpose:readPT done
        vpd-manager[1078]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1023 SUCCESS

vpd-manager/include/ipz_parser.hpp

vpd-manager/include/manager.hpp

vpd-manager/include/parser_interface.hpp

vpd-manager/src/ipz_parser.cpp

vpd-manager/src/manager.cpp

vpd-tool/src/vpd_tool.cpp

vpd-tool/src/vpd_tool_main.cpp

vpd-manager/include/ipz_parser.hpp

vpd-manager/src/manager.cpp

vpd-manager/include/isdimm_parser.hpp

vpd-tool/src/vpd_tool_main.cpp

vpd-tool/include/tool_utils.hpp

vpd-manager/include/manager.hpp

vpd-manager/src/ipz_parser.cpp

vpd-manager/include/manager.hpp

vpd-manager/src/ipz_parser.cpp

vpd-manager/src/manager.cpp

vpd-manager/src/parser.cpp

vpd-tool/include/vpd_tool.hpp

vpd-tool/src/vpd_tool_main.cpp

vpd-manager/include/manager.hpp

vpd-manager/include/parser.hpp

vpd-manager/include/ipz_parser.hpp

vpd-manager/include/parser_interface.hpp

branupama · 2025-09-15T07:37:30Z

vpd-manager/include/parser.hpp

+     * @throw Data Exception
+     * @throw ECC Exception
+     */
+    void vpdSanityCheck();


we can keep API name as performVpdSanityCheck or checkVpdSanity.

we use this term very frequently "sanityCheck" , so i have given this name.
1st level of function is named as performVpdSanityCheck , so can't use it again.

vpd-tool/src/vpd_tool.cpp

vpd-tool/include/vpd_tool.hpp

vpd-manager/include/manager.hpp

vpd-manager/src/ipz_parser.cpp

branupama · 2025-09-17T07:35:56Z

why ~ file is being added ?

vpd-manager/src/ipz_parser.cpp

vpd-tool/src/vpd_tool_main.cpp

branupama · 2025-09-17T08:27:24Z

vpd-manager/src/parser.cpp

+                ParserFactory::getParser(m_vpdVector, l_eachPath,
+                                         m_vpdStartOffset);
+
+            auto rc1 = l_parser->sanityChecker();


we can avoid this variable by using rc variable.

i have used 2 rcs here .
I need both of them here.

internal rc1 will collect immidiate return code for each eerpom call.

Outer rc will be updated based on condition-
rc = success, rc1 = success, rc2 = success
rc = Failure, rc1 = Failure, rc2 = Failure
rc = Failure, rc1 = success, rc2 = Failure
rc = Failure, rc1 = Failure, rc2 = success

vpd-manager/src/parser.cpp

vpd-manager/include/manager.hpp

vpd-manager/src/manager.cpp

vpd-manager/src/parser.cpp

vpd-tool/include/vpd_tool.hpp

vpd-manager/src/parser.cpp

vpd-manager/src/manager.cpp

vpd-manager/src/parser.cpp

vpd-manager/src/ipz_parser.cpp

vpd-manager/include/parser.hpp

branupama · 2025-09-17T16:24:32Z

vpd-manager/src/ipz_parser.cpp

+{
+    try
+    {
+        logging::logMessage(


keep necessary logs only. This log is not required.

we are not sure, it ran for 1 or more eeproms( redundant), in that case this log will be a proof.

vpd-manager/src/ipz_parser.cpp

vpd-manager/src/parser.cpp

vpd-tool/src/vpd_tool_main.cpp

Added new dbus API to perform the santity check for all the vpds. It takes one eeprom path at a time and checks for it's redundant paths. For each of them it checks for header, TOC and all it's records and their respective ECC. Test: busctl introspect com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager NAME TYPE SIGNATURE RESULT/VALUE FLAGS .CollectFRUVPD method o - - .GetExpandedLocationCode method sq s - .GetFRUsByExpandedLocationCode method s ao - .GetFRUsByUnexpandedLocationCode method sq ao - .GetHardwarePath method o s - .PerformVPDRecollection method - - - .ReadKeyword method sv v - .UpdateKeyword method sv i - .WriteKeyword method ossay i - .WriteKeywordOnHardware method sv i - .deleteFRUVPD method o - - .performVpdSanityCheck method s - - .CollectionStatus property s "Completed" emits-change writable $ busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager performVpdSanityCheck s "/sys/bus/spi/drivers/at25/spi12.0/eeprom" $vpd-tool --sanityCheck -P "/sys/bus/spi/drivers/at25/spi22.0/eeprom" DBG: validating eeprom - /sys/bus/spi/drivers/at25/spi22.0/eeprom DBG: Calling DBUS API for - /sys/bus/spi/drivers/at25/spi22.0/eeprom DBG: returned from DBUS API, return code - 0 VPD validation Successful. journal- Sep 10 21:09:53 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1006 Performing sanity check for file path: /sys/bus/spi/drivers/at25/spi22.0/eeprom Sep 10 21:09:53 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1019 SUCCESS Sep 10 21:09:53 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/parser.cpp, Line: 386 Sanity checker Passed for /sys/bus/spi/drivers/at25/spi22.0/eeprom Sep 10 21:09:54 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1006 Performing sanity check for file path: /sys/bus/spi/drivers/at25/spi23.0/eeprom Sep 10 21:09:54 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1019 SUCCESS Sep 10 21:09:54 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/parser.cpp, Line: 386 Sanity checker Passed for /sys/bus/spi/drivers/at25/spi23.0/eeprom Sep 10 21:11:10 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1006 Performing sanity check for file path: /sys/bus/spi/drivers/at25/spi22.0/eeprom Sep 10 21:11:10 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1019 SUCCESS Sep 10 21:11:10 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/parser.cpp, Line: 386 Sanity checker Passed for /sys/bus/spi/drivers/at25/spi22.0/eeprom Sep 10 21:11:11 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1006 Performing sanity check for file path: /sys/bus/spi/drivers/at25/spi23.0/eeprom Sep 10 21:11:11 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/ipz_parser.cpp, Line: 1019 SUCCESS Sep 10 21:11:11 p10bmc vpd-manager[1094]: FileName: /usr/src/debug/openpower-fru-vpd/1.0+git/vpd-manager/src/parser.cpp, Line: 386 Sanity checker Passed for /sys/bus/spi/drivers/at25/spi23.0/eeprom Error case- $busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager performVpdSanityCheck s "" "0x50003D0E": { "SRC": "BD554000", "Message": "An EEPROM path access error occurred.", "PLID": "0x50003D0E", "CreatorID": "BMC", "Subsystem": "CEC Hardware - VPD Interface", "Commit Time": "09/05/2025 09:10:03", "Sev": "Informational Event", "CompID": "bmc vpd" Signed-off-by: Alpana Kumari <[email protected]>

SunnySrivastava1984 · 2025-09-18T13:26:06Z

vpd-tool/include/vpd_tool.hpp

    void clearVpdDumpDir() const noexcept;
+
+    /**
+     * @brief It validates the record and ECC of FRU path.


nit: API validates record against the ECC for that record.

SunnySrivastava1984 · 2025-09-18T13:27:36Z

vpd-tool/include/vpd_tool.hpp

+     * @brief It validates the record and ECC of FRU path.
+     *
+     * This API calls dbus API to verify the record and ECC of user provided
+     * EEPROM path. That dbus API internally checks for- is there a redundant


That dbus API

Which D-Bus API? Can be more specific?

SunnySrivastava1984 · 2025-09-18T13:28:25Z

vpd-tool/include/vpd_tool.hpp

+     *
+     * This API calls dbus API to verify the record and ECC of user provided
+     * EEPROM path. That dbus API internally checks for- is there a redundant
+     * EEPROM for this FRU. If found , it verifies the  same for redundant one


checks for- is

What is the "-" for?

Could you please rephrase the additional description.

SunnySrivastava1984 · 2025-09-18T13:28:54Z

vpd-tool/include/vpd_tool.hpp

+     * EEPROM path. That dbus API internally checks for- is there a redundant
+     * EEPROM for this FRU. If found , it verifies the  same for redundant one
+     * also.
+     * @param[in] eepromPath - EEPROM path to perform sanity check


I don't see any "eepromPath" as input to the API.

SunnySrivastava1984 · 2025-09-18T13:29:51Z

vpd-tool/src/vpd_tool.cpp

 #include <tuple>
 namespace vpd
 {
+


Not required?

SunnySrivastava1984 · 2025-09-18T13:44:57Z

vpd-manager/include/parser.hpp

        const types::WriteVpdParams& i_paramsToWriteData);

+    /*
+     * @brief API to perform sanity check on the EEPROM


When you say "the" EEPROM. Which EEPROM should I assume. I don't see any in the parameter list or reference to any EEPROM here.

SunnySrivastava1984 · 2025-09-18T13:47:12Z

vpd-manager/src/parser.cpp


+int Parser::vpdSanityCheck() noexcept
+{
+    std::vector<types::Path> l_fruPaths{m_vpdFilePath};


Do we need another data structure for this? Can we check it in place? I see Unnecessary string copies.

SunnySrivastava1984 · 2025-09-18T13:50:12Z

vpd-manager/src/parser.cpp

+        for (const auto& l_eachPath : l_fruPaths)
+        {
+            // Read the VPD data into a vector.
+            vpdSpecificUtility::getVpdDataInVector(l_eachPath, m_vpdVector,


Instead of two calls can we use "getVpdParserInstance"? Will that help?

SunnySrivastava1984 · 2025-09-18T13:55:07Z

vpd-manager/include/parser_interface.hpp

    }

+    /**
+     * @brief Virtual API to perform sanity check on EEPROM.


@branupama When we say "the" it needs to be definite. Here I don't see reference to the any specific EEPROM in the API , hence I don't agree with "the EEPROM"

Having said that, to explain which EEPROM this API will perform action upon, @Alpana07 you will have to give some reference, like
API to perform sanity check on EEPROM passed via constructor of parser class.

SunnySrivastava1984 · 2025-09-18T13:57:37Z

vpd-manager/src/ipz_parser.cpp

            // Verify the ECC for this Record
            if (!recordEccCheck(itrToPT))
            {
+                logging::logMessage(


We already maintain a list of invalid records, can that be used here instead of logging one at a time.

Alpana07 requested review from SunnySrivastava1984 and souvik1914581 September 2, 2025 14:20

Alpana07 force-pushed the vpd-tool_moduleVPD branch 5 times, most recently from 05e6490 to 18f651e Compare September 3, 2025 08:24

Alpana07 requested review from RekhaAparna01 and branupama September 3, 2025 08:25

Alpana07 force-pushed the vpd-tool_moduleVPD branch 2 times, most recently from ae32363 to cf8e46d Compare September 4, 2025 18:55

SunnySrivastava1984 requested changes Sep 5, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch from cf8e46d to 4ec2b49 Compare September 5, 2025 09:05

Alpana07 requested a review from SunnySrivastava1984 September 5, 2025 09:08

Alpana07 force-pushed the vpd-tool_moduleVPD branch from 4ec2b49 to 08cea7e Compare September 5, 2025 14:02

SunnySrivastava1984 requested changes Sep 8, 2025

View reviewed changes

vpd-manager/include/ipz_parser.hpp Show resolved Hide resolved

vpd-manager/src/manager.cpp Outdated Show resolved Hide resolved

vpd-manager/src/manager.cpp Outdated Show resolved Hide resolved

Alpana07 force-pushed the vpd-tool_moduleVPD branch from 08cea7e to ce6efd2 Compare September 10, 2025 21:15

Alpana07 requested a review from SunnySrivastava1984 September 10, 2025 21:16

Alpana07 force-pushed the vpd-tool_moduleVPD branch from ce6efd2 to 863203d Compare September 11, 2025 04:21

SunnySrivastava1984 requested changes Sep 11, 2025

View reviewed changes

vpd-manager/include/isdimm_parser.hpp Outdated Show resolved Hide resolved

Alpana07 force-pushed the vpd-tool_moduleVPD branch from 863203d to e55b3be Compare September 11, 2025 11:18

SunnySrivastava1984 requested changes Sep 12, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch from e55b3be to 1d9024f Compare September 12, 2025 14:00

Alpana07 requested a review from SunnySrivastava1984 September 12, 2025 14:00

souvik1914581 suggested changes Sep 15, 2025

View reviewed changes

branupama reviewed Sep 15, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch 2 times, most recently from bb9077b to f25f592 Compare September 17, 2025 07:08

branupama reviewed Sep 17, 2025

View reviewed changes

vpd-manager/src/ipz_parser.cpp Outdated Show resolved Hide resolved

vpd-tool/src/vpd_tool_main.cpp Outdated Show resolved Hide resolved

vpd-tool/src/vpd_tool_main.cpp Outdated Show resolved Hide resolved

branupama reviewed Sep 17, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch from f25f592 to 5c6880d Compare September 17, 2025 11:12

branupama reviewed Sep 17, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch from 5c6880d to 8341ae6 Compare September 17, 2025 14:13

branupama reviewed Sep 17, 2025

View reviewed changes

Alpana07 force-pushed the vpd-tool_moduleVPD branch from 8341ae6 to bc14ab7 Compare September 17, 2025 18:58

Alpana07 requested review from branupama and souvik1914581 September 17, 2025 19:04

SunnySrivastava1984 requested changes Sep 18, 2025

View reviewed changes

vpd-tool: Add module VPD sanity check #658

Are you sure you want to change the base?

vpd-tool: Add module VPD sanity check #658

Uh oh!

Conversation

Alpana07 commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

branupama commented Sep 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Alpana07 commented Sep 2, 2025 •

edited

Loading