Refactor: bios: Construct the attribute value table

when constructing the attribute value table, we need to traverse
the attribute table. But there is a bug in the previous version
when traversing the attribute table.

Now we use the iterator of the last commit implementation to rebuild
attribute value table.

Signed-off-by: John Wang <wangzqbj@inspur.com>
Change-Id: I9fe2b6eabf2b01e124b780fc0fc8615d492c1fed
diff --git a/libpldm/bios_table.c b/libpldm/bios_table.c
index 26d30bf..89f7241 100644
--- a/libpldm/bios_table.c
+++ b/libpldm/bios_table.c
@@ -20,6 +20,12 @@
 			return PLDM_ERROR_INVALID_DATA;                        \
 	} while (0)
 
+#define BUFFER_SIZE_EXPECT(current_size, expected_size)                        \
+	do {                                                                   \
+		if (current_size < expected_size)                              \
+			return PLDM_ERROR_INVALID_LENGTH;                      \
+	} while (0)
+
 uint8_t pldm_bios_table_attr_entry_enum_decode_pv_num(
     const struct pldm_bios_attr_table_entry *entry)
 {
@@ -54,6 +60,35 @@
 	return PLDM_SUCCESS;
 }
 
+uint8_t pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+    const struct pldm_bios_attr_table_entry *entry, uint16_t *pv_hdls,
+    uint8_t pv_num)
+{
+	uint8_t num = pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
+	num = num < pv_num ? num : pv_num;
+	size_t i;
+	for (i = 0; i < num; i++) {
+		uint16_t *hdl = (uint16_t *)(entry->metadata + sizeof(uint8_t) +
+					     i * sizeof(uint16_t));
+		pv_hdls[i] = le16toh(*hdl);
+	}
+	return num;
+}
+
+int pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
+    const struct pldm_bios_attr_table_entry *entry, uint16_t *pv_hdls,
+    uint8_t pv_num)
+{
+	POINTER_CHECK(entry);
+	POINTER_CHECK(pv_hdls);
+	ATTR_TYPE_EXPECT(entry->attr_type, PLDM_BIOS_ENUMERATION);
+	uint8_t num = pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
+	if (num != pv_num)
+		return PLDM_ERROR_INVALID_DATA;
+	pldm_bios_table_attr_entry_enum_decode_pv_hdls(entry, pv_hdls, pv_num);
+	return PLDM_SUCCESS;
+}
+
 /** @brief Get length of an enum attribute entry
  */
 static size_t
@@ -142,6 +177,85 @@
 	return attr_table_entry->entry_length_handler(entry);
 }
 
+size_t pldm_bios_table_attr_value_entry_encode_enum_length(uint8_t count)
+{
+	return sizeof(struct pldm_bios_attr_val_table_entry) - 1 +
+	       sizeof(count) + count;
+}
+
+void pldm_bios_table_attr_value_entry_encode_enum(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint8_t count, uint8_t *handles)
+{
+	size_t length =
+	    pldm_bios_table_attr_value_entry_encode_enum_length(count);
+	assert(length <= entry_length);
+
+	struct pldm_bios_attr_val_table_entry *table_entry = entry;
+	table_entry->attr_handle = htole16(attr_handle);
+	table_entry->attr_type = attr_type;
+	table_entry->value[0] = count;
+	if (count != 0)
+		memcpy(&table_entry->value[1], handles, count);
+}
+
+int pldm_bios_table_attr_value_entry_encode_enum_check(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint8_t count, uint8_t *handles)
+{
+	POINTER_CHECK(entry);
+	if (count != 0 && handles == NULL)
+		return PLDM_ERROR_INVALID_DATA;
+	ATTR_TYPE_EXPECT(attr_type, PLDM_BIOS_ENUMERATION);
+	size_t length =
+	    pldm_bios_table_attr_value_entry_encode_enum_length(count);
+	BUFFER_SIZE_EXPECT(entry_length, length);
+	pldm_bios_table_attr_value_entry_encode_enum(
+	    entry, entry_length, attr_handle, attr_type, count, handles);
+	return PLDM_SUCCESS;
+}
+
+size_t
+pldm_bios_table_attr_value_entry_encode_string_length(uint16_t string_length)
+{
+	return sizeof(struct pldm_bios_attr_val_table_entry) - 1 +
+	       sizeof(string_length) + string_length;
+}
+
+void pldm_bios_table_attr_value_entry_encode_string(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint16_t str_length, const char *str)
+{
+	size_t length =
+	    pldm_bios_table_attr_value_entry_encode_string_length(str_length);
+	assert(length <= entry_length);
+
+	struct pldm_bios_attr_val_table_entry *table_entry = entry;
+	table_entry->attr_handle = htole16(attr_handle);
+	table_entry->attr_type = attr_type;
+	if (str_length != 0)
+		memcpy(table_entry->value + sizeof(str_length), str,
+		       str_length);
+	str_length = htole16(str_length);
+	memcpy(table_entry->value, &str_length, sizeof(str_length));
+}
+
+int pldm_bios_table_attr_value_entry_encode_string_check(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint16_t str_length, const char *str)
+{
+	POINTER_CHECK(entry);
+	if (str_length != 0 && str == NULL)
+		return PLDM_ERROR_INVALID_DATA;
+	ATTR_TYPE_EXPECT(attr_type, PLDM_BIOS_STRING);
+	size_t length =
+	    pldm_bios_table_attr_value_entry_encode_string_length(str_length);
+	BUFFER_SIZE_EXPECT(entry_length, length);
+	pldm_bios_table_attr_value_entry_encode_string(
+	    entry, entry_length, attr_handle, attr_type, str_length, str);
+	return PLDM_SUCCESS;
+}
+
 struct pldm_bios_table_iter {
 	const uint8_t *table_data;
 	size_t table_len;
@@ -196,4 +310,4 @@
 const void *pldm_bios_table_iter_value(struct pldm_bios_table_iter *iter)
 {
 	return iter->table_data + iter->current_pos;
-}
\ No newline at end of file
+}
diff --git a/libpldm/bios_table.h b/libpldm/bios_table.h
index 3d17f3c..0b20109 100644
--- a/libpldm/bios_table.h
+++ b/libpldm/bios_table.h
@@ -91,6 +91,30 @@
 int pldm_bios_table_attr_entry_enum_decode_def_num_check(
     const struct pldm_bios_attr_table_entry *entry, uint8_t *def_num);
 
+/** @brief Get possible values string handles
+ *  @param[in] entry - Pointer to bios attribute table entry
+ *  @param[out] pv_hdls - Pointer to a buffer to stroe
+ * PossibleValuesStringHandles
+ *  @param[in] pv_num - Number of PossibleValuesStringHandles expected
+ *  @return pldm_completion_codes
+ */
+uint8_t pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+    const struct pldm_bios_attr_table_entry *entry, uint16_t *pv_hdls,
+    uint8_t pv_num);
+
+/** @brief Get possible values string handles and check the validity of the
+ * parameters
+ *  @param[in] entry - Pointer to bios attribute table entry
+ *  @param[out] pv_hdls - Pointer to a buffer to stroe
+ * PossibleValuesStringHandles
+ *  @param[in] pv_num - Number of PossibleValuesStringHandles the buffer can
+ * stroe
+ *  @return Number of PossibleValuesStringHandles decoded
+ */
+int pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
+    const struct pldm_bios_attr_table_entry *entry, uint16_t *pv_hdls,
+    uint8_t pv_num);
+
 /** @brief Get the length of default string in bytes for the entry
  *  @param[in] entry - Pointer to bios attribute table entry
  *  @return length of default string in bytes
@@ -108,6 +132,83 @@
     const struct pldm_bios_attr_table_entry *entry,
     uint16_t *def_string_length);
 
+/** @brief Get length that an attribute value entry(type: enum) will take
+ *  @param[in] count - Total number of current values for this enumeration
+ *  @return The length that an entry(type: enum) will take
+ */
+size_t pldm_bios_table_attr_value_entry_encode_enum_length(uint8_t count);
+
+/** @brief Create an attribute value entry(type: enum)
+ *  @param[out] entry - Pointer to bios attribute value entry
+ *  @param[in] entry_length - Length of attribute value entry
+ *  @param[in] attr_handle - This handle points to an attribute in the
+ *  BIOS Attribute Vlaue Table.
+ *  @param[in] attr_type - Type of this attribute in the BIOS Attribute Value
+ * Table
+ *  @param[in] count - Total number of current values for this enum attribute
+ *  @param[in] handle_indexes - Index into the array(provided in the BIOS
+ * Attribute Table) of the possible values of string handles for this attribute.
+ */
+void pldm_bios_table_attr_value_entry_encode_enum(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint8_t count, uint8_t *handle_indexes);
+
+/** @brief Create an attribute value entry(type: enum) and check the validity of
+ * the parameters
+ *  @param[out] entry - Pointer to bios attribute value entry
+ *  @param[in] entry_length - Length of attribute value entry
+ *  @param[in] attr_handle - This handle points to an attribute in the
+ *  BIOS Attribute Vlaue Table.
+ *  @param[in] attr_type - Type of this attribute in the BIOS Attribute Value
+ * Table
+ *  @param[in] count - Total number of current values for this enum attribute
+ *  @param[in] handle_indexes - Index into the array(provided in the BIOS
+ * Attribute Table) of the possible values of string handles for this attribute.
+ *  @return pldm_completion_codes
+ */
+int pldm_bios_table_attr_value_entry_encode_enum_check(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint8_t count, uint8_t *handle_indexes);
+
+/** @brief Get length that an attribute value entry(type: string) will take
+ *  @param[in] string_length - Length of the current string in byte, 0 indicates
+ *  that the current string value is not set.
+ *  @return The length that an entry(type: string) will take
+ */
+size_t
+pldm_bios_table_attr_value_entry_encode_string_length(uint16_t string_length);
+
+/** @brief Create an attribute value entry(type: string)
+ *  @param[out] entry - Pointer to bios attribute value entry
+ *  @param[in] entry_length - Length of attribute value entry
+ *  @param[in] attr_handle - This handle points to an attribute in the
+ *  BIOS Attribute Vlaue Table.
+ *  @param[in] attr_type - Type of this attribute in the BIOS Attribute Value
+ * Table
+ *  @param[in] string_length - Length of current string in bytes. 0 indicates
+ * that the current string value is not set.
+ *  @param[in] string - The current string itsel
+ */
+void pldm_bios_table_attr_value_entry_encode_string(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint16_t string_length, const char *string);
+/** @brief Create an attribute value entry(type: string) and check the validity
+ * of the parameters
+ *  @param[out] entry - Pointer to bios attribute value entry
+ *  @param[in] entry_length - Length of attribute value entry
+ *  @param[in] attr_handle - This handle points to an attribute in the
+ *  BIOS Attribute Vlaue Table.
+ *  @param[in] attr_type - Type of this attribute in the BIOS Attribute Value
+ * Table
+ *  @param[in] string_length - Length of current string in bytes. 0 indicates
+ * that the current string value is not set.
+ *  @param[in] string - The current string itsel
+ *  @return pldm_completion_codes
+ */
+int pldm_bios_table_attr_value_entry_encode_string_check(
+    void *entry, size_t entry_length, uint16_t attr_handle, uint8_t attr_type,
+    uint16_t string_length, const char *string);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/libpldmresponder/bios.cpp b/libpldmresponder/bios.cpp
index a133b2d..31b4ec0 100644
--- a/libpldmresponder/bios.cpp
+++ b/libpldmresponder/bios.cpp
@@ -485,115 +485,40 @@
     }
 }
 
-/** @brief Construct the attibute value table for BIOS type Enumeration and
- *  Enumeration ReadOnly
- *
- *  @param[in] BIOSAttributeTable - the attribute table
- *  @param[in] BIOSStringTable - the string table
- *  @param[in, out] attributeValueTable - the attribute value table
- *
- */
-void constructAttrValueTable(const BIOSTable& BIOSAttributeTable,
-                             const BIOSTable& BIOSStringTable,
-                             Table& attributeValueTable)
+void constructAttrValueEntry(
+    const struct pldm_bios_attr_table_entry* attrTableEntry,
+    const std::string& attrName, const BIOSTable& BIOSStringTable,
+    Table& attrValueTable)
 {
-    Response response;
-    BIOSAttributeTable.load(response);
-
-    auto tableData = response.data();
-    size_t tableLen = response.size();
-    auto attrPtr =
-        reinterpret_cast<struct pldm_bios_attr_table_entry*>(response.data());
-
-    while (1)
+    CurrentValues currVals;
+    try
     {
-        uint16_t attrHdl = attrPtr->attr_handle;
-        uint8_t attrType = attrPtr->attr_type;
-        uint16_t stringHdl = attrPtr->string_handle;
-        tableData += (sizeof(struct pldm_bios_attr_table_entry) - 1);
-        uint8_t numPossiVals = *tableData;
-        tableData++; // pass number of possible values
-        PossibleValuesByHandle possiValsByHdl(numPossiVals, 0);
-        memcpy(possiValsByHdl.data(), tableData,
-               sizeof(uint16_t) * numPossiVals);
-        tableData += sizeof(uint16_t) * numPossiVals;
-        uint8_t numDefVals = *tableData;
-        tableData++;             // pass number of def vals
-        tableData += numDefVals; // pass all the def val indices
-
-        auto attrName = findStringName(stringHdl, BIOSStringTable);
-        if (attrName.empty())
-        {
-            if (std::distance(tableData, response.data() + tableLen) <=
-                padChksumMax)
-            {
-                log<level::ERR>("Did not find string name for handle",
-                                entry("STRING_HANDLE=%d", stringHdl));
-                return;
-            }
-            attrPtr =
-                reinterpret_cast<struct pldm_bios_attr_table_entry*>(tableData);
-            continue;
-        }
-        CurrentValues currVals;
-        try
-        {
-            currVals = getAttrValue(attrName);
-        }
-        catch (const std::exception& e)
-        {
-            log<level::ERR>(
-                "constructAttrValueTable returned error for attribute",
-                entry("NAME=%s", attrName.c_str()),
-                entry("ERROR=%s", e.what()));
-            if (std::distance(tableData, response.data() + tableLen) <=
-                padChksumMax)
-            {
-                return;
-            }
-
-            attrPtr =
-                reinterpret_cast<struct pldm_bios_attr_table_entry*>(tableData);
-            continue;
-        }
-        // sorting since the possible values are stored in sorted way
-        std::sort(currVals.begin(), currVals.end());
-        auto currValStrIndices =
-            findStrIndices(possiValsByHdl, currVals, BIOSStringTable);
-        // number of current values equals to the number of string handles
-        // received not the number of strings received from getAttrValue
-        uint8_t numCurrVals = currValStrIndices.size();
-
-        BIOSTableRow enumAttrValTable(
-            (sizeof(struct pldm_bios_attr_val_table_entry) - 1) +
-                sizeof(uint8_t) + numCurrVals * sizeof(uint8_t),
-            0);
-        BIOSTableRow::iterator it = enumAttrValTable.begin();
-        auto attrValPtr =
-            reinterpret_cast<struct pldm_bios_attr_val_table_entry*>(
-                enumAttrValTable.data());
-        attrValPtr->attr_handle = attrHdl;
-        attrValPtr->attr_type = attrType;
-        std::advance(it, (sizeof(pldm_bios_attr_val_table_entry) - 1));
-        std::copy_n(&numCurrVals, sizeof(numCurrVals), it);
-        std::advance(it, sizeof(numCurrVals));
-        if (numCurrVals)
-        {
-            std::copy(currValStrIndices.begin(), currValStrIndices.end(), it);
-            std::advance(it, currValStrIndices.size());
-        }
-        std::move(enumAttrValTable.begin(), enumAttrValTable.end(),
-                  std::back_inserter(attributeValueTable));
-
-        if (std::distance(tableData, response.data() + tableLen) <=
-            padChksumMax)
-        {
-            break;
-        }
-
-        attrPtr =
-            reinterpret_cast<struct pldm_bios_attr_table_entry*>(tableData);
+        currVals = getAttrValue(attrName);
     }
+    catch (const std::exception& e)
+    {
+        log<level::ERR>("getAttrValue returned error for attribute",
+                        entry("NAME=%s", attrName.c_str()),
+                        entry("ERROR=%s", e.what()));
+        return;
+    }
+    uint8_t pv_num =
+        pldm_bios_table_attr_entry_enum_decode_pv_num(attrTableEntry);
+    PossibleValuesByHandle pvHdls(pv_num, 0);
+    pldm_bios_table_attr_entry_enum_decode_pv_hdls(attrTableEntry,
+                                                   pvHdls.data(), pv_num);
+    std::sort(currVals.begin(), currVals.end());
+
+    auto currValStrIndices = findStrIndices(pvHdls, currVals, BIOSStringTable);
+
+    auto entryLength = pldm_bios_table_attr_value_entry_encode_enum_length(
+        currValStrIndices.size());
+    auto tableSize = attrValueTable.size();
+    attrValueTable.resize(tableSize + entryLength);
+    pldm_bios_table_attr_value_entry_encode_enum(
+        attrValueTable.data() + tableSize, entryLength,
+        attrTableEntry->attr_handle, attrTableEntry->attr_type,
+        currValStrIndices.size(), currValStrIndices.data());
 }
 
 } // end namespace bios_type_enum
@@ -669,98 +594,34 @@
     }
 }
 
-/** @brief Construct the attibute value table for BIOS type String and
- *  String ReadOnly
- *
- *  @param[in] BIOSAttributeTable - the attribute table
- *  @param[in] BIOSStringTable - the string table
- *  @param[in, out] attributeValueTable - the attribute value table
- *
- */
-void constructAttrValueTable(const BIOSTable& BIOSAttributeTable,
+void constructAttrValueEntry(const pldm_bios_attr_table_entry* attrTableEntry,
+                             const std::string& attrName,
                              const BIOSTable& BIOSStringTable,
-                             Table& attributeValueTable)
+                             Table& attrValueTable)
 {
-    Response response;
-    BIOSAttributeTable.load(response);
-
-    auto dataPtr = response.data();
-    size_t tableLen = response.size();
-
-    while (true)
+    std::ignore = BIOSStringTable;
+    std::string currStr;
+    uint16_t currStrLen = 0;
+    try
     {
-        auto attrPtr =
-            reinterpret_cast<struct pldm_bios_attr_table_entry*>(dataPtr);
-        uint16_t attrHdl = attrPtr->attr_handle;
-        uint8_t attrType = attrPtr->attr_type;
-        uint16_t stringHdl = attrPtr->string_handle;
-        dataPtr += (sizeof(struct pldm_bios_attr_table_entry) - 1);
-        // pass number of StringType, MinimumStringLength, MaximumStringLength
-        dataPtr += sizeof(uint8_t) + sizeof(uint16_t) + sizeof(uint16_t);
-        auto sizeDefaultStr = *(reinterpret_cast<uint16_t*>(dataPtr));
-        // pass number of DefaultStringLength, DefaultString
-        dataPtr += sizeof(uint16_t) + sizeDefaultStr;
-
-        auto attrName = findStringName(stringHdl, BIOSStringTable);
-        if (attrName.empty())
-        {
-            if (std::distance(dataPtr, response.data() + tableLen) <=
-                padChksumMax)
-            {
-                log<level::ERR>("Did not find string name for handle",
-                                entry("STRING_HANDLE=%d", stringHdl));
-                return;
-            }
-            continue;
-        }
-
-        uint16_t currStrLen = 0;
-        std::string currStr;
-        try
-        {
-            currStr = getAttrValue(attrName);
-            currStrLen = currStr.size();
-        }
-        catch (const std::exception& e)
-        {
-            log<level::ERR>("getAttrValue returned error for attribute",
-                            entry("NAME=%s", attrName.c_str()),
-                            entry("ERROR=%s", e.what()));
-            if (std::distance(dataPtr, response.data() + tableLen) <=
-                padChksumMax)
-            {
-                return;
-            }
-            continue;
-        }
-
-        BIOSTableRow strAttrValTable(
-            bios_parser::bios_string::attrValueTableSize + currStrLen, 0);
-        BIOSTableRow::iterator it = strAttrValTable.begin();
-        auto attrValPtr =
-            reinterpret_cast<struct pldm_bios_attr_val_table_entry*>(
-                strAttrValTable.data());
-        attrValPtr->attr_handle = attrHdl;
-        attrValPtr->attr_type = attrType;
-        std::advance(it, (sizeof(pldm_bios_attr_val_table_entry) - 1));
-        std::copy_n(reinterpret_cast<uint8_t*>(&currStrLen), sizeof(uint16_t),
-                    it);
-        std::advance(it, sizeof(uint16_t));
-        if (currStrLen)
-        {
-            std::copy_n(currStr.cbegin(), currStrLen, it);
-            std::advance(it, currStrLen);
-        }
-
-        attributeValueTable.insert(attributeValueTable.end(),
-                                   strAttrValTable.begin(),
-                                   strAttrValTable.end());
-
-        if (std::distance(dataPtr, response.data() + tableLen) <= padChksumMax)
-        {
-            break;
-        }
+        currStr = getAttrValue(attrName);
+        currStrLen = currStr.size();
     }
+    catch (const std::exception& e)
+    {
+        log<level::ERR>("getAttrValue returned error for attribute",
+                        entry("NAME=%s", attrName.c_str()),
+                        entry("ERROR=%s", e.what()));
+        return;
+    }
+    auto entryLength =
+        pldm_bios_table_attr_value_entry_encode_string_length(currStrLen);
+    auto tableSize = attrValueTable.size();
+    attrValueTable.resize(tableSize + entryLength);
+    pldm_bios_table_attr_value_entry_encode_string(
+        attrValueTable.data() + tableSize, entryLength,
+        attrTableEntry->attr_handle, attrTableEntry->attr_type, currStrLen,
+        currStr.c_str());
 }
 
 } // end namespace bios_type_string
@@ -796,16 +657,6 @@
     {bios_parser::bIOSEnumJson, bios_type_enum::constructAttrTable},
     {bios_parser::bIOSStrJson, bios_type_string::constructAttrTable}};
 
-using valueHandler = std::function<void(const BIOSTable& BIOSAttributeTable,
-
-                                        const BIOSTable& BIOSStringTable,
-
-                                        Table& attributeTable)>;
-
-std::map<BIOSJsonName, valueHandler> attrValueHandlers{
-    {bios_parser::bIOSEnumJson, bios_type_enum::constructAttrValueTable},
-    {bios_parser::bIOSStrJson, bios_type_string::constructAttrValueTable}};
-
 /** @brief Construct the BIOS attribute table
  *
  *  @param[in] BIOSAttributeTable - the attribute table
@@ -871,6 +722,37 @@
     return response;
 }
 
+using AttrValTableEntryConstructHandler =
+    std::function<void(const struct pldm_bios_attr_table_entry* tableEntry,
+                       const std::string& attrName,
+                       const BIOSTable& BIOSStringTable, Table& table)>;
+
+using AttrType = uint8_t;
+const std::map<AttrType, AttrValTableEntryConstructHandler>
+    AttrValTableConstructMap{
+        {PLDM_BIOS_STRING, bios_type_string::constructAttrValueEntry},
+        {PLDM_BIOS_STRING_READ_ONLY, bios_type_string::constructAttrValueEntry},
+        {PLDM_BIOS_ENUMERATION, bios_type_enum::constructAttrValueEntry},
+        {PLDM_BIOS_ENUMERATION_READ_ONLY,
+         bios_type_enum::constructAttrValueEntry},
+    };
+
+void constructAttrValueTableEntry(
+    const struct pldm_bios_attr_table_entry* attrEntry,
+    const BIOSTable& BIOSStringTable, Table& attributeValueTable)
+{
+    auto attrName = findStringName(attrEntry->string_handle, BIOSStringTable);
+    if (attrName.empty())
+    {
+        log<level::ERR>("invalid string handle",
+                        entry("STRING_HANDLE=%d", attrEntry->string_handle));
+        return;
+    }
+
+    AttrValTableConstructMap.at(attrEntry->attr_type)(
+        attrEntry, attrName, BIOSStringTable, attributeValueTable);
+}
+
 /** @brief Construct the BIOS attribute value table
  *
  *  @param[in] BIOSAttributeValueTable - the attribute value table
@@ -886,78 +768,50 @@
                                     const BIOSTable& BIOSStringTable,
                                     uint32_t& /*transferHandle*/,
                                     uint8_t& /*transferOpFlag*/,
-                                    uint8_t instanceID, const char* biosJsonDir)
+                                    uint8_t instanceID)
 {
     Response response(sizeof(pldm_msg_hdr) + PLDM_GET_BIOS_TABLE_MIN_RESP_BYTES,
                       0);
     auto responsePtr = reinterpret_cast<pldm_msg*>(response.data());
     uint32_t nxtTransferHandle = 0;
     uint8_t transferFlag = PLDM_START_AND_END;
-    size_t respPayloadLength{};
 
-    if (BIOSAttributeValueTable.isEmpty())
-    { // no persisted table, constructing fresh table and data
-        Table attributeValueTable;
-        fs::path dir(biosJsonDir);
-
-        for (auto it = attrValueHandlers.begin(); it != attrValueHandlers.end();
-             it++)
-        {
-            fs::path file = dir / it->first;
-            if (fs::exists(file))
-            {
-                it->second(BIOSAttributeTable, BIOSStringTable,
-                           attributeValueTable);
-            }
-        }
-
-        if (attributeValueTable.empty())
-        { // no available json file is found
-            encode_get_bios_table_resp(instanceID, PLDM_BIOS_TABLE_UNAVAILABLE,
-                                       nxtTransferHandle, transferFlag, nullptr,
-                                       response.size(), responsePtr);
-            return response;
-        }
-        // calculate pad
-        uint8_t padSize = utils::getNumPadBytes(attributeValueTable.size());
-        std::vector<uint8_t> pad(padSize, 0);
-        if (padSize)
-        {
-            std::move(pad.begin(), pad.end(),
-                      std::back_inserter(attributeValueTable));
-        }
-        if (!attributeValueTable.empty())
-        {
-            // compute checksum
-            boost::crc_32_type result;
-            result.process_bytes(attributeValueTable.data(),
-                                 attributeValueTable.size());
-            uint32_t checkSum = result.checksum();
-            size_t size = attributeValueTable.size();
-            attributeValueTable.resize(size + sizeof(checkSum));
-            std::copy_n(reinterpret_cast<uint8_t*>(&checkSum), sizeof(checkSum),
-                        attributeValueTable.data() + size);
-            BIOSAttributeValueTable.store(attributeValueTable);
-        }
-
-        response.resize(sizeof(pldm_msg_hdr) +
-                        PLDM_GET_BIOS_TABLE_MIN_RESP_BYTES +
-                        attributeValueTable.size());
-        responsePtr = reinterpret_cast<pldm_msg*>(response.data());
-        respPayloadLength = response.size();
+    if (!BIOSAttributeValueTable.isEmpty())
+    {
         encode_get_bios_table_resp(instanceID, PLDM_SUCCESS, nxtTransferHandle,
-                                   transferFlag, attributeValueTable.data(),
-                                   respPayloadLength, responsePtr);
-    }
-    else
-    { // persisted table present, constructing response
-        respPayloadLength = response.size();
-        encode_get_bios_table_resp(instanceID, PLDM_SUCCESS, nxtTransferHandle,
-                                   transferFlag, nullptr, respPayloadLength,
+                                   transferFlag, nullptr, response.size(),
                                    responsePtr); // filling up the header here
         BIOSAttributeValueTable.load(response);
+        return response;
     }
 
+    Table attributeValueTable;
+    Table attributeTable;
+    BIOSAttributeTable.load(attributeTable);
+    traverseBIOSAttrTable(
+        attributeTable,
+        [&BIOSStringTable, &attributeValueTable](
+            const struct pldm_bios_attr_table_entry* tableEntry) {
+            constructAttrValueTableEntry(tableEntry, BIOSStringTable,
+                                         attributeValueTable);
+        });
+    if (attributeValueTable.empty())
+    {
+        encode_get_bios_table_resp(instanceID, PLDM_BIOS_TABLE_UNAVAILABLE,
+                                   nxtTransferHandle, transferFlag, nullptr,
+                                   response.size(), responsePtr);
+        return response;
+    }
+    utils::padAndChecksum(attributeValueTable);
+    BIOSAttributeValueTable.store(attributeValueTable);
+
+    response.resize(sizeof(pldm_msg_hdr) + PLDM_GET_BIOS_TABLE_MIN_RESP_BYTES +
+                    attributeValueTable.size());
+    responsePtr = reinterpret_cast<pldm_msg*>(response.data());
+    encode_get_bios_table_resp(instanceID, PLDM_SUCCESS, nxtTransferHandle,
+                               transferFlag, attributeValueTable.data(),
+                               response.size(), responsePtr);
+
     return response;
 }
 
@@ -1034,7 +888,7 @@
                     response = getBIOSAttributeValueTable(
                         BIOSAttributeValueTable, BIOSAttributeTable,
                         BIOSStringTable, transferHandle, transferOpFlag,
-                        request->hdr.instance_id, biosJsonDir);
+                        request->hdr.instance_id);
                 }
                 break;
             default:
diff --git a/libpldmresponder/bios_parser.hpp b/libpldmresponder/bios_parser.hpp
index 8b0c46e..e922b09 100644
--- a/libpldmresponder/bios_parser.hpp
+++ b/libpldmresponder/bios_parser.hpp
@@ -124,12 +124,6 @@
                                    sizeof(uint16_t) + sizeof(uint8_t) +
                                    sizeof(uint16_t) + sizeof(uint16_t) +
                                    sizeof(uint16_t));
-/* attrValueTableSize is the sum of fixed length of members which construct a
- * string attribute value table, including attr_handle(uint16_t),
- * attr_type(uint8_t), CurrentStringLength(uint16_t)*/
-constexpr auto attrValueTableSize = 5;
-static_assert(attrValueTableSize ==
-              sizeof(uint16_t) + sizeof(uint8_t) + sizeof(uint16_t));
 
 /** @brief Get the string related values and the default values for the
  *         BIOSString and BIOSStringReadOnly types
diff --git a/test/libpldm_bios_table_test.cpp b/test/libpldm_bios_table_test.cpp
index ea94bfe..bc4ec09 100644
--- a/test/libpldm_bios_table_test.cpp
+++ b/test/libpldm_bios_table_test.cpp
@@ -42,14 +42,36 @@
         reinterpret_cast<struct pldm_bios_attr_table_entry*>(enumEntry.data());
     uint8_t pvNumber = pldm_bios_table_attr_entry_enum_decode_pv_num(entry);
     EXPECT_EQ(pvNumber, 2);
-    uint8_t defNumber = pldm_bios_table_attr_entry_enum_decode_def_num(entry);
-    EXPECT_EQ(defNumber, 1);
-
     pvNumber = 0;
     auto rc =
         pldm_bios_table_attr_entry_enum_decode_pv_num_check(entry, &pvNumber);
     EXPECT_EQ(rc, PLDM_SUCCESS);
     EXPECT_EQ(pvNumber, 2);
+
+    std::vector<uint16_t> pvHandles(pvNumber, 0);
+    pvNumber = pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+        entry, pvHandles.data(), pvHandles.size());
+    EXPECT_EQ(pvNumber, 2);
+    EXPECT_EQ(pvHandles[0], 2);
+    EXPECT_EQ(pvHandles[1], 3);
+    pvHandles.resize(1);
+    pvNumber = pldm_bios_table_attr_entry_enum_decode_pv_hdls(
+        entry, pvHandles.data(), pvHandles.size());
+    EXPECT_EQ(pvNumber, 1);
+    EXPECT_EQ(pvHandles[0], 2);
+
+    pvHandles.resize(2);
+    rc = pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
+        entry, pvHandles.data(), pvHandles.size());
+    EXPECT_EQ(rc, PLDM_SUCCESS);
+    EXPECT_EQ(pvHandles[0], 2);
+    EXPECT_EQ(pvHandles[1], 3);
+    rc = pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(
+        entry, pvHandles.data(), 1);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+
+    uint8_t defNumber = pldm_bios_table_attr_entry_enum_decode_def_num(entry);
+    EXPECT_EQ(defNumber, 1);
     defNumber = 0;
     rc =
         pldm_bios_table_attr_entry_enum_decode_def_num_check(entry, &defNumber);
@@ -69,6 +91,9 @@
     rc =
         pldm_bios_table_attr_entry_enum_decode_def_num_check(entry, &defNumber);
     EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+    rc =
+        pldm_bios_table_attr_entry_enum_decode_pv_hdls_check(entry, nullptr, 0);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
 }
 
 TEST(AttrTable, StringEntryDecodeTest)
@@ -157,6 +182,95 @@
     pldm_bios_table_iter_free(iter);
 }
 
+TEST(AttrValTable, EnumEntryEncodeTest)
+{
+    std::vector<uint8_t> enumEntry{
+        0, 0, /* attr handle */
+        0,    /* attr type */
+        2,    /* number of current value */
+        0,    /* current value string handle index */
+        1,    /* current value string handle index */
+    };
+
+    auto length = pldm_bios_table_attr_value_entry_encode_enum_length(2);
+    EXPECT_EQ(length, enumEntry.size());
+    std::vector<uint8_t> encodeEntry(length, 0);
+    uint8_t handles[] = {0, 1};
+    pldm_bios_table_attr_value_entry_encode_enum(
+        encodeEntry.data(), encodeEntry.size(), 0, 0, 2, handles);
+    EXPECT_EQ(encodeEntry, enumEntry);
+
+    EXPECT_DEATH(
+        pldm_bios_table_attr_value_entry_encode_enum(
+            encodeEntry.data(), encodeEntry.size() - 1, 0, 0, 2, handles),
+        "length <= entry_length");
+
+    auto rc = pldm_bios_table_attr_value_entry_encode_enum_check(
+        encodeEntry.data(), encodeEntry.size(), 0, PLDM_BIOS_ENUMERATION, 2,
+        handles);
+    EXPECT_EQ(rc, PLDM_SUCCESS);
+    EXPECT_EQ(encodeEntry, enumEntry);
+    auto entry = reinterpret_cast<struct pldm_bios_attr_val_table_entry*>(
+        enumEntry.data());
+    entry->attr_type = PLDM_BIOS_ENUMERATION_READ_ONLY;
+    rc = pldm_bios_table_attr_value_entry_encode_enum_check(
+        encodeEntry.data(), encodeEntry.size(), 0,
+        PLDM_BIOS_ENUMERATION_READ_ONLY, 2, handles);
+    EXPECT_EQ(rc, PLDM_SUCCESS);
+    EXPECT_EQ(encodeEntry, enumEntry);
+    rc = pldm_bios_table_attr_value_entry_encode_enum_check(
+        encodeEntry.data(), encodeEntry.size(), 0, PLDM_BIOS_PASSWORD, 2,
+        handles);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+    rc = pldm_bios_table_attr_value_entry_encode_enum_check(
+        encodeEntry.data(), encodeEntry.size() - 1, 0, PLDM_BIOS_ENUMERATION, 2,
+        handles);
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+}
+
+TEST(AttrValTable, stringEntryEncodeTest)
+{
+    std::vector<uint8_t> stringEntry{
+        0,   0,        /* attr handle */
+        1,             /* attr type */
+        3,   0,        /* current string length */
+        'a', 'b', 'c', /* defaut value string handle index */
+    };
+
+    auto length = pldm_bios_table_attr_value_entry_encode_string_length(3);
+    EXPECT_EQ(length, stringEntry.size());
+    std::vector<uint8_t> encodeEntry(length, 0);
+    pldm_bios_table_attr_value_entry_encode_string(
+        encodeEntry.data(), encodeEntry.size(), 0, 1, 3, "abc");
+    EXPECT_EQ(encodeEntry, stringEntry);
+
+    EXPECT_DEATH(
+        pldm_bios_table_attr_value_entry_encode_string(
+            encodeEntry.data(), encodeEntry.size() - 1, 0, 1, 3, "abc"),
+        "length <= entry_length");
+
+    auto rc = pldm_bios_table_attr_value_entry_encode_string_check(
+        encodeEntry.data(), encodeEntry.size(), 0, PLDM_BIOS_STRING, 3, "abc");
+    EXPECT_EQ(rc, PLDM_SUCCESS);
+    EXPECT_EQ(encodeEntry, stringEntry);
+    auto entry = reinterpret_cast<struct pldm_bios_attr_val_table_entry*>(
+        stringEntry.data());
+    entry->attr_type = PLDM_BIOS_STRING_READ_ONLY;
+    rc = pldm_bios_table_attr_value_entry_encode_string_check(
+        encodeEntry.data(), encodeEntry.size(), 0, PLDM_BIOS_STRING_READ_ONLY,
+        3, "abc");
+    EXPECT_EQ(rc, PLDM_SUCCESS);
+    EXPECT_EQ(encodeEntry, stringEntry);
+    rc = pldm_bios_table_attr_value_entry_encode_string_check(
+        encodeEntry.data(), encodeEntry.size(), 0, PLDM_BIOS_PASSWORD, 3,
+        "abc");
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_DATA);
+    rc = pldm_bios_table_attr_value_entry_encode_string_check(
+        encodeEntry.data(), encodeEntry.size() - 1, 0, PLDM_BIOS_STRING, 3,
+        "abc");
+    EXPECT_EQ(rc, PLDM_ERROR_INVALID_LENGTH);
+}
+
 TEST(Itearator, DeathTest)
 {