Add backend_adjust_offset to avoid the windows overlap
In mihawk, the windows overlap will cause the cache coherence.
Hostboot writes the SPD data to BMC through a window and read it
back with another window. It causes the data missed and DIMM lost.
Hostboot log
23.80714|<<DBG-956|SPD::getMemType() - MemType: 0xff, Error: NoHUID: 0x30008.
BMC log: The overlaped windows
Window @ 0x756e0000 for size 0x00046000 maps flash offset 0x000e7000
Window @ 0x757e0000 for size 0x00048000 maps flash offset 0x000e5000
Tested: 1. In mihawk, it can fix the SPD cache coherence issue
2. Add unit test to verify VPNOR offset alignment
Change-Id: I92670ade4e2a91b5c49a0acabfc0456f90d49b93
Signed-off-by: Alvin Wang <alvinwang@msn.com>
[AJ: Remove some MSG_INFO() spam, fix whitespace issues]
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/backend.h b/backend.h
index f7b56b4..fadc94b 100644
--- a/backend.h
+++ b/backend.h
@@ -123,6 +123,17 @@
* Return: 0 on success otherwise negative error code
*/
int (*reset)(struct backend *backend, void *buf, uint32_t count);
+
+ /*
+ * align_offset() - Align the offset to avoid overlap
+ * @context: The backend context pointer
+ * @offset: The flash offset
+ * @window_size:The window size
+ *
+ * Return: 0 on success otherwise negative error code
+ */
+ int (*align_offset)(struct backend *backend, uint32_t *offset,
+ uint32_t window_size);
};
/* Make this better */
@@ -222,6 +233,26 @@
return backend->ops->reset(backend, buf, count);
}
+
+static inline int backend_align_offset(struct backend *backend, uint32_t *offset, uint32_t window_size)
+{
+ assert(backend);
+ if (backend->ops->align_offset){
+ return backend->ops->align_offset(backend, offset, window_size);
+ }else{
+ /*
+ * It would be nice to align the offsets which we map to window
+ * size, this will help prevent overlap which would be an
+ * inefficient use of our reserved memory area (we would like
+ * to "cache" as much of the acutal flash as possible in
+ * memory). If we're protocol V1 however we must ensure the
+ * offset requested is exactly mapped.
+ */
+ *offset &= ~(window_size - 1);
+ return 0;
+ }
+}
+
struct backend backend_get_mtd(void);
int backend_probe_mtd(struct backend *master, const char *path);
diff --git a/file/backend.c b/file/backend.c
index 224719d..865a434 100644
--- a/file/backend.c
+++ b/file/backend.c
@@ -260,6 +260,7 @@
.write = file_write,
.reset = file_reset,
.validate = NULL,
+ .align_offset = NULL,
};
struct backend backend_get_file(void)
diff --git a/mtd/backend.c b/mtd/backend.c
index be8f2b2..921c041 100644
--- a/mtd/backend.c
+++ b/mtd/backend.c
@@ -349,6 +349,7 @@
.write = mtd_write,
.validate = NULL,
.reset = mtd_reset,
+ .align_offset = NULL,
};
struct backend backend_get_mtd(void)
diff --git a/vpnor/backend.cpp b/vpnor/backend.cpp
index f1b2c63..43591db 100644
--- a/vpnor/backend.cpp
+++ b/vpnor/backend.cpp
@@ -419,6 +419,8 @@
const pnor_partition& part = priv->vpnor->table->partition(offset);
if (vpnor_partition_is_readonly(part))
{
+ MSG_DBG("Try to write read only partition (part=%s, offset=0x%x)\n",
+ part.data.name, offset);
return -EPERM;
}
}
@@ -462,6 +464,47 @@
return reset_lpc_memory;
}
+/*
+ * vpnor_align_offset() - Align the offset
+ * @context: The backend context pointer
+ * @offset: The flash offset
+ * @window_size:The window size
+ *
+ * Return: 0 on success otherwise negative error code
+ */
+static int vpnor_align_offset(struct backend* backend, uint32_t* offset,
+ uint32_t window_size)
+{
+ const struct vpnor_data* priv = (const struct vpnor_data*)backend->priv;
+
+ /* Adjust the offset to align with the offset of partition base */
+ try
+ {
+ // Get the base of the partition
+ const pnor_partition& part = priv->vpnor->table->partition(*offset);
+ uint32_t base = part.data.base * VPNOR_ERASE_SIZE;
+
+ // Get the base offset relative to the window_size
+ uint32_t base_offset = base & (window_size - 1);
+
+ // Adjust the offset to align with the base
+ *offset = ((*offset - base_offset) & ~(window_size - 1)) + base_offset;
+ MSG_DBG(
+ "vpnor_align_offset: to @ 0x%.8x(base=0x%.8x base_offset=0x%.8x)\n",
+ *offset, base, base_offset);
+ return 0;
+ }
+ catch (const openpower::virtual_pnor::UnmappedOffset& e)
+ {
+ /*
+ * Writes to unmapped areas are not meaningful, so deny the request.
+ * This removes the ability for a compromised host to abuse unused
+ * space if any data was to be persisted (which it isn't).
+ */
+ return -EACCES;
+ }
+}
+
static const struct backend_ops vpnor_ops = {
.init = vpnor_dev_init,
.free = vpnor_free,
@@ -471,6 +514,7 @@
.write = vpnor_write,
.validate = vpnor_validate,
.reset = vpnor_reset,
+ .align_offset = vpnor_align_offset,
};
struct backend backend_get_vpnor(void)
diff --git a/vpnor/test/Makefile.am.include b/vpnor/test/Makefile.am.include
index 3e2edaa..71a1517 100644
--- a/vpnor/test/Makefile.am.include
+++ b/vpnor/test/Makefile.am.include
@@ -16,6 +16,14 @@
$(PHOSPHOR_LOGGING_LIBS) \
$(PHOSPHOR_DBUS_INTERFACES_LIBS)
+
+vpnor_test_create_aligned_window_SOURCES = \
+ $(TEST_MOCK_VPNOR_SRCS) \
+ $(TEST_MBOX_VPNOR_INTEG_SRCS) \
+ %reldir%/create_aligned_window.cpp
+vpnor_test_create_aligned_window_LDFLAGS = $(OESDK_TESTCASE_FLAGS)
+vpnor_test_create_aligned_window_LDADD = $(VPNOR_LDADD)
+
vpnor_test_create_pnor_partition_table_SOURCES = \
$(TEST_MOCK_VPNOR_SRCS) \
$(TEST_MBOX_VPNOR_INTEG_SRCS) \
@@ -227,6 +235,7 @@
vpnor_test_force_readonly_toc_LDADD = $(VPNOR_LDADD)
check_PROGRAMS += \
+ %reldir%/create_aligned_window \
%reldir%/create_pnor_partition_table \
%reldir%/create_read_window_partition_exists \
%reldir%/write_prsv \
@@ -260,3 +269,5 @@
XFAIL_TESTS += \
%reldir%/write_toc
+
+
diff --git a/vpnor/test/create_aligned_window.cpp b/vpnor/test/create_aligned_window.cpp
new file mode 100644
index 0000000..910e7c5
--- /dev/null
+++ b/vpnor/test/create_aligned_window.cpp
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: Apache-2.0
+// Copyright (C) 2018 IBM Corp.
+
+#include "config.h"
+
+extern "C" {
+#include "test/mbox.h"
+#include "test/system.h"
+}
+
+#include "vpnor/test/tmpd.hpp"
+
+#include <cassert>
+#include <cstring>
+#include <experimental/filesystem>
+#include <fstream>
+#include <vector>
+
+#include "vpnor/backend.h"
+
+// A read window assumes that the toc is located at offset 0,
+// so create dummy partition at arbitrary offset 0x1000.
+const std::string toc[] = {
+ "partition01=HBB,00001000,00002000,80,ECC,READONLY",
+ "partition05=DJVPD,0x000e5000,0x0022d000,00,ECC,PRESERVED",
+};
+
+static const auto BLOCK_SIZE = 4096;
+static const auto ERASE_SIZE = BLOCK_SIZE;
+static const auto N_WINDOWS = 1;
+static const auto WINDOW_SIZE = 1024 * 1024;
+static const auto MEM_SIZE = WINDOW_SIZE * 3;
+
+static const uint8_t get_info[] = {0x02, 0x00, 0x02, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00};
+
+// Used to check the alignment
+static const uint8_t create_read_window1[] = {
+ 0x04, 0x01, 0xe7, 0x00, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+static const uint8_t response1[] = {0x04, 0x01, 0x00, 0xfd, 0x00, 0x01, 0xe5,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01};
+
+static const uint8_t create_read_window2[] = {
+ 0x04, 0x02, 0xe5, 0x01, 0x01, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+
+static const uint8_t response2[] = {0x04, 0x02, 0x00, 0xfd, 0x48, 0x00, 0xe5,
+ 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01};
+
+namespace test = openpower::virtual_pnor::test;
+
+int main()
+{
+ struct mbox_context* ctx;
+
+ system_set_reserved_size(MEM_SIZE);
+ system_set_mtd_sizes(MEM_SIZE, ERASE_SIZE);
+
+ ctx = mbox_create_frontend_context(N_WINDOWS, WINDOW_SIZE);
+
+ test::VpnorRoot root(&ctx->backend, toc, BLOCK_SIZE);
+
+ int rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info));
+ assert(rc == 1);
+
+ // send the request for partition5
+ rc = mbox_command_dispatch(ctx, create_read_window1,
+ sizeof(create_read_window1));
+ assert(rc == 1);
+ rc = mbox_cmp(ctx, response1, sizeof(response1));
+ assert(rc == 0);
+
+ // send the request for partition5
+ rc = mbox_command_dispatch(ctx, create_read_window2,
+ sizeof(create_read_window2));
+ assert(rc == 1);
+ rc = mbox_cmp(ctx, response2, sizeof(response2));
+ assert(rc == 0);
+
+ return rc;
+}
diff --git a/windows.c b/windows.c
index ebe16c1..a62b50c 100644
--- a/windows.c
+++ b/windows.c
@@ -574,26 +574,14 @@
window_reset(context, cur);
}
-/*
- * In case of the virtual pnor, as of now it's possible that a window may
- * have content less than it's max size. We basically copy one flash partition
- * per window, and some partitions are smaller than the max size. An offset
- * right after such a small partition ends should lead to new mapping. The code
- * below prevents that.
- */
-#ifndef VIRTUAL_PNOR_ENABLED
+ /* Adjust the offset for alignment by the backend. It will help prevent the
+ * overlap.
+ */
if (!exact) {
- /*
- * It would be nice to align the offsets which we map to window
- * size, this will help prevent overlap which would be an
- * inefficient use of our reserved memory area (we would like
- * to "cache" as much of the acutal flash as possible in
- * memory). If we're protocol V1 however we must ensure the
- * offset requested is exactly mapped.
- */
- offset &= ~(cur->size - 1);
+ if (backend_align_offset(&(context->backend), &offset, cur->size)) {
+ MSG_ERR("Can't adjust the offset by backend\n");
+ }
}
-#endif
if (offset > context->backend.flash_size) {
MSG_ERR("Tried to open read window past flash limit\n");