flash: Introduce flash_validate()
Clean up the protocol_negotiate_version() mess. The existing approach
came about due to viewing the vpnor implementation as an edge case in
its own right. The code becomes much neater if we consider all backends
as equal and afford them the callbacks necessary for correct behaviour.
Change-Id: Ifaeee9da459818cf22b2f137ddc5b8d0356b9be9
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
diff --git a/vpnor/Makefile.am.include b/vpnor/Makefile.am.include
index 75bd92e..36e3c50 100644
--- a/vpnor/Makefile.am.include
+++ b/vpnor/Makefile.am.include
@@ -2,9 +2,7 @@
%reldir%/mboxd_pnor_partition_table.cpp \
%reldir%/flash.cpp \
%reldir%/pnor_partition.cpp \
- %reldir%/lpc_reset.cpp \
- %reldir%/protocol.cpp \
- %reldir%/protocol_negotiate_version.cpp
+ %reldir%/lpc_reset.cpp
mboxd_LDFLAGS += -lstdc++fs \
$(SDBUSPLUS_LIBS) \
diff --git a/vpnor/flash.cpp b/vpnor/flash.cpp
index cd2a37f..847e6a5 100644
--- a/vpnor/flash.cpp
+++ b/vpnor/flash.cpp
@@ -237,3 +237,39 @@
}
return 0;
}
+
+static bool vpnor_partition_is_readonly(const pnor_partition& part)
+{
+ return part.data.user.data[1] & PARTITION_READONLY;
+}
+
+int flash_validate(struct mbox_context* context, uint32_t offset,
+ uint32_t size __attribute__((unused)), bool ro)
+{
+ /* All reads are allowed */
+ if (ro)
+ {
+ return 0;
+ }
+
+ /* Only allow write windows on regions mapped by the ToC as writeable */
+ try
+ {
+ const pnor_partition& part = context->vpnor->table->partition(offset);
+ if (vpnor_partition_is_readonly(part))
+ {
+ return -EPERM;
+ }
+ }
+ 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;
+ }
+
+ return 0;
+}
diff --git a/vpnor/protocol.cpp b/vpnor/protocol.cpp
deleted file mode 100644
index c3f4fca..0000000
--- a/vpnor/protocol.cpp
+++ /dev/null
@@ -1,65 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-// Copyright (C) 2018 IBM Corp.
-#include "config.h"
-
-extern "C" {
-#include "mboxd.h"
-#include "protocol.h"
-#include "vpnor/protocol.h"
-}
-
-#include "vpnor/pnor_partition_table.hpp"
-
-/* XXX: Maybe this should be a method on a class? */
-static bool vpnor_partition_is_readonly(const pnor_partition& part)
-{
- return part.data.user.data[1] & PARTITION_READONLY;
-}
-
-typedef int (*create_window_fn)(struct mbox_context* context,
- struct protocol_create_window* io);
-
-static int generic_vpnor_create_window(struct mbox_context* context,
- struct protocol_create_window* io,
- create_window_fn create_window)
-{
- if (io->req.ro)
- {
- return create_window(context, io);
- }
-
- /* Only allow write windows on regions mapped by the ToC as writeable */
- size_t offset = io->req.offset;
- offset <<= context->block_size_shift;
- try
- {
- const pnor_partition& part = context->vpnor->table->partition(offset);
- if (vpnor_partition_is_readonly(part))
- {
- return -EPERM;
- }
- }
- 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;
- }
-
- return create_window(context, io);
-}
-
-int protocol_v1_vpnor_create_window(struct mbox_context* context,
- struct protocol_create_window* io)
-{
- return generic_vpnor_create_window(context, io, protocol_v1_create_window);
-}
-
-int protocol_v2_vpnor_create_window(struct mbox_context* context,
- struct protocol_create_window* io)
-{
- return generic_vpnor_create_window(context, io, protocol_v2_create_window);
-}
diff --git a/vpnor/protocol_negotiate_version.cpp b/vpnor/protocol_negotiate_version.cpp
deleted file mode 100644
index 12593ea..0000000
--- a/vpnor/protocol_negotiate_version.cpp
+++ /dev/null
@@ -1,58 +0,0 @@
-// SPDX-License-Identifier: Apache-2.0
-// Copyright (C) 2018 IBM Corp.
-#include "config.h"
-
-#include <errno.h>
-
-extern "C" {
-#include "mboxd.h"
-#include "protocol.h"
-#include "vpnor/protocol.h"
-}
-
-/* We need to hijack create_window for vpnor */
-
-static const struct protocol_ops protocol_ops_v1 = {
- .reset = protocol_v1_reset,
- .get_info = protocol_v1_get_info,
- .get_flash_info = protocol_v1_get_flash_info,
- .create_window = protocol_v1_vpnor_create_window,
- .mark_dirty = protocol_v1_mark_dirty,
- .erase = NULL,
- .flush = protocol_v1_flush,
- .close = protocol_v1_close,
- .ack = protocol_v1_ack,
-};
-
-static const struct protocol_ops protocol_ops_v2 = {
- .reset = protocol_v1_reset,
- .get_info = protocol_v2_get_info,
- .get_flash_info = protocol_v2_get_flash_info,
- .create_window = protocol_v2_vpnor_create_window,
- .mark_dirty = protocol_v2_mark_dirty,
- .erase = protocol_v2_erase,
- .flush = protocol_v2_flush,
- .close = protocol_v2_close,
- .ack = protocol_v1_ack,
-};
-
-static const struct protocol_ops* protocol_ops_map[] = {
- [0] = NULL,
- [1] = &protocol_ops_v1,
- [2] = &protocol_ops_v2,
-};
-
-int protocol_negotiate_version(struct mbox_context* context, uint8_t requested)
-{
- /* Check we support the version requested */
- if (requested < API_MIN_VERSION)
- return -EINVAL;
-
- uint8_t version =
- (requested > API_MAX_VERSION) ? API_MAX_VERSION : requested;
- context->version = static_cast<enum api_version>(version);
-
- context->protocol = protocol_ops_map[context->version];
-
- return context->version;
-}
diff --git a/vpnor/test/Makefile.am.include b/vpnor/test/Makefile.am.include
index f3f4fc2..5ff4dcc 100644
--- a/vpnor/test/Makefile.am.include
+++ b/vpnor/test/Makefile.am.include
@@ -14,8 +14,6 @@
vpnor/flash.cpp \
vpnor/pnor_partition.cpp \
vpnor/pnor_partition_table.cpp \
- vpnor/protocol.cpp \
- vpnor/protocol_negotiate_version.cpp \
%reldir%/tmpd.cpp
VPNOR_LDADD = -lstdc++fs \