tools/pci: Replace memcpy on pci write with aligned copy
memcpy() does unaligned access. Using it for device memory causes
exception (SIGBUS).
Replace the memcpy in PciAccessBridge::write with aligned copy.
Signed-off-by: Vivekanand Veeracholan <vveerach@google.com>
Change-Id: I8e573715dc8971be547d882ce3cb6de31b2620e4
diff --git a/tools/helper.cpp b/tools/helper.cpp
index fbb0275..d3790de 100644
--- a/tools/helper.cpp
+++ b/tools/helper.cpp
@@ -110,4 +110,35 @@
return (result == ipmi_flash::ActionStatus::success);
}
+void* memcpyAligned(void* destination, const void* source, std::size_t size)
+{
+ std::size_t i = 0;
+ std::size_t bytesCopied = 0;
+
+ if ((alignof(destination) == alignof(std::uint64_t)) &&
+ (alignof(source) == alignof(std::uint64_t)))
+ {
+ auto src64 = reinterpret_cast<const volatile std::uint64_t*>(source);
+ auto dest64 = reinterpret_cast<volatile std::uint64_t*>(destination);
+
+ for (i = 0; i < size / sizeof(std::uint64_t); i++)
+ {
+ *dest64++ = *src64++;
+ bytesCopied += sizeof(std::uint64_t);
+ }
+ }
+
+ auto srcMem8 =
+ reinterpret_cast<const volatile std::uint8_t*>(source) + bytesCopied;
+ auto destMem8 =
+ reinterpret_cast<volatile std::uint8_t*>(destination) + bytesCopied;
+
+ for (i = bytesCopied; i < size; i++)
+ {
+ *destMem8++ = *srcMem8++;
+ }
+
+ return destination;
+}
+
} // namespace host_tool
diff --git a/tools/helper.hpp b/tools/helper.hpp
index 5580972..9acf58d 100644
--- a/tools/helper.hpp
+++ b/tools/helper.hpp
@@ -16,4 +16,13 @@
*/
bool pollStatus(std::uint16_t session, ipmiblob::BlobInterface* blob);
+/**
+ * Aligned memcpy
+ * @param[out] destination - destination memory pointer
+ * @param[in] source - source memory pointer
+ * @param[in] size - bytes to copy
+ * @return destination pointer
+ */
+void* memcpyAligned(void* destination, const void* source, std::size_t size);
+
} // namespace host_tool
diff --git a/tools/pci.cpp b/tools/pci.cpp
index 224a619..150095f 100644
--- a/tools/pci.cpp
+++ b/tools/pci.cpp
@@ -23,6 +23,8 @@
#include <pciaccess.h>
} // extern "C"
+#include "helper.hpp"
+
#include <fmt/format.h>
#include <stdplus/handle/managed.hpp>
@@ -111,7 +113,7 @@
dataLength));
}
- std::memcpy(addr + dataOffset, data.data(), data.size());
+ memcpyAligned(addr + dataOffset, data.data(), data.size());
}
void NuvotonPciBridge::enableBridge()
diff --git a/tools/test/tools_helper_unittest.cpp b/tools/test/tools_helper_unittest.cpp
index 1c5b30d..9aac01a 100644
--- a/tools/test/tools_helper_unittest.cpp
+++ b/tools/test/tools_helper_unittest.cpp
@@ -12,14 +12,14 @@
using ::testing::Return;
using ::testing::TypedEq;
-class UpdaterTest : public ::testing::Test
+class HelperTest : public ::testing::Test
{
protected:
ipmiblob::BlobInterfaceMock blobMock;
std::uint16_t session = 0xbeef;
};
-TEST_F(UpdaterTest, PollStatusReturnsAfterSuccess)
+TEST_F(HelperTest, PollStatusReturnsAfterSuccess)
{
ipmiblob::StatResponse verificationResponse = {};
/* the other details of the response are ignored, and should be. */
@@ -32,7 +32,7 @@
EXPECT_TRUE(pollStatus(session, &blobMock));
}
-TEST_F(UpdaterTest, PollStatusReturnsAfterFailure)
+TEST_F(HelperTest, PollStatusReturnsAfterFailure)
{
ipmiblob::StatResponse verificationResponse = {};
/* the other details of the response are ignored, and should be. */
@@ -45,4 +45,24 @@
EXPECT_FALSE(pollStatus(session, &blobMock));
}
+TEST_F(HelperTest, MemcpyAlignedOneByte)
+{
+ const char source = 'a';
+ char destination;
+
+ EXPECT_EQ(&destination,
+ memcpyAligned(&destination, &source, sizeof(source)));
+ EXPECT_EQ(destination, source);
+}
+
+TEST_F(HelperTest, MemcpyAlignedMultiByte)
+{
+ const char source[14] = "abcdefghijklm";
+ char destination[14] = "xxxxxxxxxxxxx";
+
+ EXPECT_EQ(&destination,
+ memcpyAligned(&destination, &source, sizeof(source)));
+ EXPECT_EQ(0, memcmp(&destination, &source, sizeof(source)));
+}
+
} // namespace host_tool