summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAndrew Jeffery <andrew@aj.id.au>2018-02-28 23:16:48 +1030
committerAndrew Jeffery <andrew@aj.id.au>2018-03-24 13:59:32 +1030
commitae1edb940c65e13e610108203e6a1cba0ab424c7 (patch)
tree404e3aeffeca9a94635599cd3d1d1b09d56b10d3
parentcd928513e21f02ebca1f68a414f5347a7de8326d (diff)
downloadphosphor-mboxd-ae1edb940c65e13e610108203e6a1cba0ab424c7.zip
phosphor-mboxd-ae1edb940c65e13e610108203e6a1cba0ab424c7.tar.gz
pnor_partition_table: Raise exception for unmapped offsets
Allow reads and writes of offsets that don't map onto partitions defined in the ToC. Do so by ignoring the mapping failure and filling a window with 0xff in the hole from the requested offset to the following partition. This change also removes the reliance on InternalFailure as the exception of choice for communicating failures. We can do better without the teeth-pulling required by phosphor-logging by translating custom exceptions into phosphor-logging exceptions at the edges. Change-Id: Ibfa961a66b0b979354c6dc226ccbe7e9fbafc16d Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
-rw-r--r--mboxd_flash_virtual.cpp53
-rw-r--r--pnor_partition_table.cpp19
-rw-r--r--pnor_partition_table.hpp33
-rw-r--r--test/vpnor/Makefile.am.include2
-rw-r--r--test/vpnor/toc_lookup_failed.cpp2
5 files changed, 90 insertions, 19 deletions
diff --git a/mboxd_flash_virtual.cpp b/mboxd_flash_virtual.cpp
index 1b94ca9..42d4ee6 100644
--- a/mboxd_flash_virtual.cpp
+++ b/mboxd_flash_virtual.cpp
@@ -18,6 +18,7 @@ extern "C" {
#include "mboxd_flash.h"
#include "mboxd_pnor_partition_table.h"
#include "pnor_partition.hpp"
+#include "pnor_partition_table.hpp"
#include "xyz/openbmc_project/Common/error.hpp"
#include <phosphor-logging/log.hpp>
#include <phosphor-logging/elog-errors.hpp>
@@ -27,6 +28,9 @@ extern "C" {
#include <exception>
#include <stdexcept>
+namespace err = sdbusplus::xyz::openbmc_project::Common::Error;
+namespace vpnor = openpower::virtual_pnor;
+
/** @brief unique_ptr functor to release a char* reference. */
struct StringDeleter
{
@@ -167,9 +171,25 @@ int64_t copy_flash(struct mbox_context* context, uint32_t offset, void* mem,
munmap(mapped_mem, partitionInfo->data.actual);
}
}
- catch (InternalFailure& e)
+ catch (vpnor::UnmappedOffset& e)
+ {
+ /*
+ * Hooo boy. Pretend that this is valid flash so we don't have
+ * discontiguous regions presented to the host. Instead, fill a window
+ * with 0xff so the 'flash' looks erased. Writes to such regions are
+ * dropped on the floor, see the implementation of write_flash() below.
+ */
+ MSG_INFO("Host requested unmapped region of %" PRId32
+ " bytes at offset 0x%" PRIx32 "\n",
+ size, offset);
+ uint32_t span = e.next - e.base;
+ rc = std::min(size, span);
+ memset(mem, 0xff, rc);
+ }
+ catch (std::exception& e)
{
- commit<InternalFailure>();
+ MSG_ERR("%s\n", e.what());
+ phosphor::logging::commit<err::InternalFailure>();
rc = -MBOX_R_SYSTEM_ERROR;
}
return rc;
@@ -216,12 +236,16 @@ int write_flash(struct mbox_context* context, uint32_t offset, void* buf,
// if the asked offset + no of bytes to write is greater
// then size of the partition file then throw error.
+ //
+ // FIXME: Don't use .actual, use (.size << ctx->block_size_shift),
+ // otherwise we can't grow the size of the data to fill the partition
if ((offset + count) > (baseOffset + partitionInfo->data.actual))
{
- MSG_ERR("Offset is beyond the partition file length[0x%.8x]\n",
- partitionInfo->data.actual);
munmap(mapped_mem, partitionInfo->data.actual);
- elog<InternalFailure>();
+ std::stringstream err;
+ err << "Write extends beyond the partition length " << std::hex
+ << partitionInfo->data.actual;
+ throw vpnor::OutOfBoundsOffset(err.str());
}
auto diffOffset = offset - baseOffset;
@@ -230,9 +254,24 @@ int write_flash(struct mbox_context* context, uint32_t offset, void* buf,
set_flash_bytemap(context, offset, count, FLASH_DIRTY);
}
- catch (InternalFailure& e)
+ catch (vpnor::UnmappedOffset& e)
+ {
+ /* Paper over the fact that the write isn't persistent */
+ MSG_INFO("Dropping %d bytes host wrote to unmapped offset 0x%" PRIx32
+ "\n",
+ count, offset);
+ return 0;
+ }
+ catch (const vpnor::OutOfBoundsOffset& e)
+ {
+ MSG_ERR("%s\n", e.what());
+ return -MBOX_R_PARAM_ERROR;
+ }
+ catch (const std::exception& e)
{
- rc = -1;
+ MSG_ERR("%s\n", e.what());
+ phosphor::logging::commit<err::InternalFailure>();
+ return -MBOX_R_SYSTEM_ERROR;
}
return rc;
}
diff --git a/pnor_partition_table.cpp b/pnor_partition_table.cpp
index c9ef3a3..50d7d59 100644
--- a/pnor_partition_table.cpp
+++ b/pnor_partition_table.cpp
@@ -149,13 +149,15 @@ const pnor_partition& Table::partition(size_t offset) const
{
return part;
}
- }
- MSG_ERR("Partition corresponding to offset 0x%zx not found", offset);
- elog<InternalFailure>();
+ /* Are we in a hole between partitions? */
+ if (blockOffset < part.data.base)
+ {
+ throw UnmappedOffset(offset, part.data.base * blockSize);
+ }
+ }
- static pnor_partition p{};
- return p;
+ throw UnmappedOffset(offset, pnorSize);
}
const pnor_partition& Table::partition(const std::string& name) const
@@ -170,10 +172,9 @@ const pnor_partition& Table::partition(const std::string& name) const
}
}
- MSG_ERR("Partition '%s' not found", name.c_str());
- elog<InternalFailure>();
- static pnor_partition p{};
- return p;
+ std::stringstream err;
+ err << "Partition " << name << " is not listed in the table of contents";
+ throw UnknownPartition(err.str());
}
} // namespace partition
diff --git a/pnor_partition_table.hpp b/pnor_partition_table.hpp
index 730e75b..88c8fb9 100644
--- a/pnor_partition_table.hpp
+++ b/pnor_partition_table.hpp
@@ -163,6 +163,8 @@ class Table
*
* @returns const reference to pnor_partition, if found, else an
* exception will be thrown.
+ *
+ * Throws: UnmappedOffset
*/
const pnor_partition& partition(size_t offset) const;
@@ -172,6 +174,8 @@ class Table
*
* @returns const reference to pnor_partition, if found, else an
* exception will be thrown.
+ *
+ * Throws: UnknownPartition
*/
const pnor_partition& partition(const std::string& name) const;
@@ -304,6 +308,35 @@ class InvalidTocEntry : public TocEntryError
}
};
+class UnmappedOffset : public std::exception
+{
+ public:
+ UnmappedOffset(size_t base, size_t next) : base(base), next(next)
+ {
+ }
+
+ const size_t base;
+ const size_t next;
+};
+
+class OutOfBoundsOffset : public ReasonedError
+{
+ public:
+ OutOfBoundsOffset(const std::string&& reason) :
+ ReasonedError(std::move(reason))
+ {
+ }
+};
+
+class UnknownPartition : public ReasonedError
+{
+ public:
+ UnknownPartition(const std::string&& reason) :
+ ReasonedError(std::move(reason))
+ {
+ }
+};
+
} // namespace virtual_pnor
} // namespace openpower
diff --git a/test/vpnor/Makefile.am.include b/test/vpnor/Makefile.am.include
index e8d51ca..2db5b8e 100644
--- a/test/vpnor/Makefile.am.include
+++ b/test/vpnor/Makefile.am.include
@@ -227,6 +227,4 @@ check_PROGRAMS += \
%reldir%/create_read_window_oob \
%reldir%/create_read_window_toc \
%reldir%/create_read_window_straddle_partitions
-
-XFAIL_TESTS += %reldir%/create_read_window_oob
endif
diff --git a/test/vpnor/toc_lookup_failed.cpp b/test/vpnor/toc_lookup_failed.cpp
index c38bcbd..a35975a 100644
--- a/test/vpnor/toc_lookup_failed.cpp
+++ b/test/vpnor/toc_lookup_failed.cpp
@@ -45,7 +45,7 @@ int main()
{
table.partition("TWO");
}
- catch (err::InternalFailure& e)
+ catch (vpnor::UnknownPartition& e)
{
return 0;
}
OpenPOWER on IntegriCloud