Simplified registerRead and registerWrite interfaces
Uses an unsigned integer instead of arbitrary buffer. This avoids
the potential of the user application writing past the buffer size and
creating a buffer overrun.
Signed-off-by: Zane Shelley <zshelle@us.ibm.com>
Change-Id: If8f6144a88cc63d0bbdd3d63ee69135018377066
diff --git a/src/hei_user_interface.hpp b/src/hei_user_interface.hpp
index 09cbcfb..58f5e34 100644
--- a/src/hei_user_interface.hpp
+++ b/src/hei_user_interface.hpp
@@ -11,6 +11,7 @@
#include <stdlib.h>
#include <hei_chip.hpp>
+#include <hei_types.hpp>
namespace libhei
{
@@ -22,30 +23,27 @@
* the isolator by the user application via the isolator main
* APIs.
*
- * @param o_buffer Allocated memory space for the returned contents of the
- * register.
- *
- * @param io_bufSize The input parameter is the maximum size of the allocated
- * o_buffer. The return value is the number of bytes actually
- * written to the buffer.
- *
* @param i_regType The user application may support different types of
* registers. This value is provided to the isolator by the
* user application via the Chip Data Files. The user
* application is responsible for knowing what to do with this
* parameter.
*
- * @param i_address The register address. The values is a 1, 2, 4, or 8 byte
- * address (right justified), which is provided to the
- * isolator by the user application via the Chip Data Files.
+ * @param i_address The register address, which is provided to the isolator by
+ * the user application via the Chip Data Files. This is a 1,
+ * 2, 4, or 8 byte value (right justified) as defined by the
+ * register type.
+ *
+ * @param o_value The returned register value. This is a 1, 2, 4, or 8 byte
+ * value (right justified) as defined by the register type.
*
* @return false => register access was successful
* true => hardware access failure
* Note that in the case of a failure, the user application is
* responsible for reporting why the register access failed.
*/
-bool registerRead(const Chip& i_chip, void* o_buffer, size_t& io_bufSize,
- uint64_t i_regType, uint64_t i_address);
+bool registerRead(const Chip& i_chip, RegisterType_t i_regType,
+ uint64_t i_address, uint64_t& o_value);
#ifdef __HEI_ENABLE_HW_WRITE
@@ -56,30 +54,27 @@
* the isolator by the user application via the isolator main
* APIs.
*
- * @param i_buffer Allocated memory space containing the register contents to
- * write to hardware.
- *
- * @param io_bufSize The input parameter is the number of byte from i_buffer to
- * write to the hardware register. The return value is the
- * actual number of bytes written to the hardware register.
- *
* @param i_regType The user application may support different types of
* registers. This value is provided to the isolator by the
* user application via the Chip Data Files. The user
* application is responsible for knowing what to do with this
* parameter.
*
- * @param i_address The register address. The values is a 1, 2, 4, or 8 byte
- * address (right justified), which is provided to the
- * isolator by the user application via the Chip Data Files.
+ * @param i_address The register address, which is provided to the isolator by
+ * the user application via the Chip Data Files. This is a 1,
+ * 2, 4, or 8 byte value (right justified) as defined by the
+ * register type.
+ *
+ * @param i_value The register value to write. This is a 1, 2, 4, or 8 byte
+ * value (right justified) as defined by the register type.
*
* @return false => register access was successful
* true => hardware access failure
* Note that in the case of a failure, the user application is
* responsible for reporting why the register access failed.
*/
-bool registerWrite(const Chip& i_chip, void* i_buffer, size_t& io_bufSize,
- uint64_t i_regType, uint64_t i_address);
+bool registerWrite(const Chip& i_chip, RegisterType_t i_regType,
+ uint64_t i_address, uint64_t i_value);
#endif // __HEI_ENABLE_HW_WRITE
diff --git a/src/register/hei_hardware_register.cpp b/src/register/hei_hardware_register.cpp
index 2d2167e..d24d251 100644
--- a/src/register/hei_hardware_register.cpp
+++ b/src/register/hei_hardware_register.cpp
@@ -52,33 +52,23 @@
{
bool accessFailure = false;
+ // This register must be readable.
+ HEI_ASSERT(queryAttrFlag(REG_ATTR_ACCESS_READ));
+
// Read from hardware only if the read is forced or the entry for this
// instance does not exist in the cache.
if (i_force || !queryCache(i_chip))
{
- // This register must be readable.
- HEI_ASSERT(queryAttrFlag(REG_ATTR_ACCESS_READ));
-
- // Get the buffer from the register cache.
- BitString& bs = accessCache(i_chip);
-
- // Get the byte size of the buffer.
- size_t sz_buffer = BitString::getMinBytes(bs.getBitLen());
-
// Read this register from hardware.
- accessFailure = registerRead(i_chip, bs.getBufAddr(), sz_buffer,
- getType(), getAddress());
- if (accessFailure)
+ uint64_t val = 0;
+ accessFailure = registerRead(i_chip, getType(), getAddress(), val);
+ if (!accessFailure)
{
- // The read failed and we can't trust what was put in the register
- // cache. So remove this instance's entry from the cache.
- flush(i_chip);
- }
- else
- {
- // Sanity check. The returned size of the data written to the buffer
- // should match the register size.
- HEI_ASSERT(getSize() == sz_buffer);
+ // Get the buffer from the register cache.
+ BitString& bs = accessCache(i_chip);
+
+ // Set this value in the bit string buffer.
+ bs.setFieldRight(0, bs.getBitLen(), val);
}
}
@@ -91,8 +81,6 @@
bool HardwareRegister::write(const Chip& i_chip) const
{
- bool accessFailure = false;
-
// This register must be writable.
HEI_ASSERT(queryAttrFlag(REG_ATTR_ACCESS_WRITE));
@@ -102,21 +90,11 @@
// Get the buffer from the register cache.
BitString& bs = accessCache(i_chip);
- // Get the byte size of the buffer.
- size_t sz_buffer = BitString::getMinBytes(bs.getBitLen());
+ // Set this value from the bit string buffer.
+ uint64_t val = bs.getFieldRight(0, bs.getBitLen());
// Write to this register to hardware.
- accessFailure = registerWrite(i_chip, bs.getBufAddr(), sz_buffer, getType(),
- getAddress());
-
- if (accessFailure)
- {
- // Sanity check. The returned size of the data written to the buffer
- // should match the register size.
- HEI_ASSERT(getSize() == sz_buffer);
- }
-
- return accessFailure;
+ return registerWrite(i_chip, getType(), getAddress(), val);
}
#endif // __HEI_ENABLE_HW_WRITE
diff --git a/test/simulator/sim_hardware_access.cpp b/test/simulator/sim_hardware_access.cpp
index e8c45ad..d0625cc 100644
--- a/test/simulator/sim_hardware_access.cpp
+++ b/test/simulator/sim_hardware_access.cpp
@@ -15,44 +15,26 @@
//------------------------------------------------------------------------------
-bool registerRead(const Chip& i_chip, void* o_buffer, size_t& io_bufSize,
- uint64_t i_regType, uint64_t i_address)
+bool registerRead(const Chip& i_chip, RegisterType_t i_regType,
+ uint64_t i_address, uint64_t& o_value)
{
bool accessFailure = false;
- HEI_ASSERT(nullptr != o_buffer);
- HEI_ASSERT(0 != io_bufSize);
-
- // Get access to data through the singleton
+ // Get access to data through the singleton.
SimulatorData& theSimData = SimulatorData::getSingleton();
switch (i_regType)
{
case REG_TYPE_SCOM:
- {
- // Get the register value and change its endianness
- uint64_t regValue =
- htobe64(theSimData.getScomReg(i_chip, (uint32_t)i_address));
- // Get size of register value for calling code and memcopy
- io_bufSize = sizeof(regValue);
- memcpy(o_buffer, ®Value, io_bufSize);
+ o_value = theSimData.getScomReg(i_chip, (uint32_t)i_address);
break;
- }
case REG_TYPE_ID_SCOM:
- {
- // Get the register value and change its endianness
- uint64_t regValue =
- htobe64(theSimData.getIdScomReg(i_chip, i_address));
- // Get size of register value for calling code and memcopy
- io_bufSize = sizeof(regValue);
- memcpy(o_buffer, ®Value, io_bufSize);
+ o_value = theSimData.getIdScomReg(i_chip, i_address);
break;
- }
default:
accessFailure = true;
- HEI_ERR("registerRead(%p,%p,%" PRIu64 ",%" PRIx64 ",%" PRIx64 ")",
- i_chip.getChip(), o_buffer, (uint64_t)io_bufSize, i_regType,
- i_address);
+ HEI_ERR("registerRead(%p,%" PRIu8 ",%" PRIx64 ")", i_chip.getChip(),
+ i_regType, i_address);
}
return accessFailure;
@@ -62,22 +44,18 @@
#ifdef __HEI_ENABLE_HW_WRITE
-bool registerWrite(const Chip& i_chip, void* i_buffer, size_t& io_bufSize,
- uint64_t i_regType, uint64_t i_address)
+bool registerWrite(const Chip& i_chip, RegisterType_t i_regType,
+ uint64_t i_address, uint64_t i_value)
{
bool accessFailure = false;
- HEI_ASSERT(nullptr != i_buffer);
- HEI_ASSERT(0 != io_bufSize);
-
switch (i_regType)
{
// TODO: add cases for REG_TYPE_SCOM and REG_TYPE_ID_SCOM
default:
accessFailure = true;
- HEI_ERR("registerWrite(%p,%p,%" PRIu64 ",%" PRIx64 ",%" PRIx64 ")",
- i_chip.getChip(), i_buffer, (uint64_t)io_bufSize, i_regType,
- i_address);
+ HEI_ERR("registerWrite(%p,%" PRIu8 ",%" PRIx64 ",%" PRIx64 ")",
+ i_chip.getChip(), i_regType, i_address, i_value);
}
return accessFailure;