From ad343101c59beb0c2b0c1033f031ff76ff2237d7 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Wed, 28 Feb 2018 00:35:50 +1030 Subject: pnor_partition: Refactor to enforce stronger boundaries for abstractions The RORequest and RWRequest classes did not provide a clear abstraction over the operation of populating a window or partition associated with a CREATE_{READ,WRITE}_WINDOW request. The role of the classes was to find the partition for the provided offset, locate and then open its backing file. However, the file-descriptor for the backing file was exposed outside of the class, as was the FFS partition struct, both of which were managed _internal_ to the class. Thus the classes provided no encapsulation of state and awkwardly split the tasks of managing and utilising the resources between the callee and caller. This commit inverts the behaviour in a fulfil() method handles the mechanics of locating, opening, manipulating and closing the backing file, requiring nothing of the caller. The pnor_partition reference is managed entirely inside the Request class, derived from the offset passed to the constructor. Unifying the mechanics into fulfil() results in a decent reduction in lines of code at the expense of some cyclomatic complexity. fulfil() is publicly exposed via read() and write() wrappers on the class, and the RORequest and RWRequest classes are removed as a result. Change-Id: Ie587ed31f1a6db97bcb490dbcc2e27ece0b1f82c Signed-off-by: Andrew Jeffery --- pnor_partition.cpp | 209 +++++++++++++++++++---------------------------------- 1 file changed, 76 insertions(+), 133 deletions(-) (limited to 'pnor_partition.cpp') diff --git a/pnor_partition.cpp b/pnor_partition.cpp index cdb9633..390c4d5 100644 --- a/pnor_partition.cpp +++ b/pnor_partition.cpp @@ -1,6 +1,7 @@ // SPDX-License-Identifier: Apache-2.0 // Copyright (C) 2018 IBM Corp. #include "pnor_partition.hpp" +#include "pnor_partition_table.hpp" #include "config.h" #include "mboxd_flash.h" #include "mboxd_pnor_partition_table.h" @@ -8,6 +9,7 @@ #include #include +#include #include #include #include @@ -26,185 +28,126 @@ namespace openpower namespace virtual_pnor { -using namespace phosphor::logging; -using namespace sdbusplus::xyz::openbmc_project::Common::Error; -using namespace std::string_literals; +namespace fs = std::experimental::filesystem; -ReturnCode Request::open(const std::string& path, int mode) +fs::path Request::getPartitionFilePath(int flags) { - if (mode == O_RDWR && partition->data.user.data[1] & PARTITION_READONLY) - { - MSG_ERR("Can't open RO partition '%s' for write\n", path.c_str()); - return ReturnCode::PARTITION_READ_ONLY; - } - - fs::path partitionFilePath = path; - - if (!fs::exists(partitionFilePath)) - { - return ReturnCode::FILE_NOT_FOUND; - } - - auto descriptor = ::open(partitionFilePath.c_str(), mode); - if (descriptor < 0) - { - return ReturnCode::FILE_OPEN_FAILURE; - } - - fd.set(descriptor); - descriptor = -1; - - return ReturnCode::SUCCESS; -} - -std::string Request::getPartitionFilePath(struct mbox_context* context, - uint32_t offset) -{ - partition = vpnor_get_partition(context, offset); - if (!partition) - { - MSG_ERR("Couldn't get the partition info for offset 0x%" PRIx32 "\n", - offset); - elog(); - } - - fs::path partitionFilePath; - // Check if partition exists in patch location - partitionFilePath = context->paths.patch_loc; - partitionFilePath /= partition->data.name; - if (fs::is_regular_file(partitionFilePath)) + auto dst = fs::path(ctx->paths.patch_loc) / partition.data.name; + if (fs::is_regular_file(dst)) { - return partitionFilePath.string(); + return dst; } - switch (partition->data.user.data[1] & + switch (partition.data.user.data[1] & (PARTITION_PRESERVED | PARTITION_READONLY)) { case PARTITION_PRESERVED: - partitionFilePath = context->paths.prsv_loc; + dst = ctx->paths.prsv_loc; break; case PARTITION_READONLY: - partitionFilePath = context->paths.ro_loc; + dst = ctx->paths.ro_loc; break; default: - partitionFilePath = context->paths.rw_loc; + dst = ctx->paths.rw_loc; } - partitionFilePath /= partition->data.name; - return partitionFilePath.string(); -} + dst /= partition.data.name; -const pnor_partition* RORequest::getPartitionInfo(struct mbox_context* context, - uint32_t offset) -{ - std::string path = getPartitionFilePath(context, offset); - ReturnCode rc = Request::open(path, O_RDONLY); - if (rc == ReturnCode::SUCCESS) + if (fs::exists(dst)) { - return partition; - } - // not interested in any other error except FILE_NOT_FOUND - if (rc != ReturnCode::FILE_NOT_FOUND) - { - MSG_ERR( - "RORequest: Error opening partition file '%s' (offset 0x%" PRIx32 - "): %u\n", - path.c_str(), offset, static_cast(rc)); - elog(); + return dst; } - // if the offset lies in read only partition then throw error. - if (partition->data.user.data[1] & PARTITION_READONLY) + if (flags == O_RDONLY) { - MSG_ERR("RORequest: Requested offset 0x%" PRIx32 - " is in a read-only partition (%s)\n", - offset, path.c_str()); - elog(); + dst = fs::path(ctx->paths.ro_loc) / partition.data.name; + assert(fs::exists(dst)); + return dst; } - // we don't get the file in the respective partition(RW/PSRV) - // try to open it from RO location. + assert(flags == O_RDWR); + auto src = fs::path(ctx->paths.ro_loc) / partition.data.name; + assert(fs::exists(src)); - fs::path partitionFilePath = context->paths.ro_loc; - partitionFilePath /= partition->data.name; + MSG_DBG("RWRequest: Didn't find '%s' under '%s', copying from '%s'\n", + partition.data.name, dst.c_str(), src.c_str()); - rc = Request::open(partitionFilePath, O_RDONLY); - if (rc != ReturnCode::SUCCESS) + dst = ctx->paths.rw_loc; + if (partition.data.user.data[1] & PARTITION_PRESERVED) { - MSG_ERR("RORequest: Failed to open partition file '%s' at RO fallback " - "(offset 0x%" PRIx32 "): %u\n", - partitionFilePath.c_str(), offset, static_cast(rc)); - elog(); + dst = ctx->paths.prsv_loc; } - return partition; + dst /= partition.data.name; + fs::copy_file(src, dst); + + return dst; } -const pnor_partition* RWRequest::getPartitionInfo(struct mbox_context* context, - uint32_t offset) +ssize_t Request::fulfil(void *buf, size_t len, int flags) { - std::string path = getPartitionFilePath(context, offset); - std::error_code ec; - - ReturnCode rc = Request::open(path, O_RDWR); - if (rc == ReturnCode::SUCCESS) - { - return partition; - } - // not interested in any other error except FILE_NOT_FOUND - if (rc != ReturnCode::FILE_NOT_FOUND) + if (!(flags == O_RDONLY || flags == O_RDWR)) { - MSG_ERR( - "RWRequest: Error opening partition file '%s' (offset 0x%" PRIx32 - "): %d\n", - path.c_str(), offset, static_cast(rc)); - elog(); + std::stringstream err; + err << "Require O_RDONLY (0x" << std::hex << O_RDONLY << " or O_RDWR " + << std::hex << O_RDWR << " for flags, got: 0x" << std::hex << flags; + throw std::invalid_argument(err.str()); } - // if the file is not available in the respective(RW/PSRV) partition - // then copy the file from RO to the respective(RW/PSRV) partition - // and open it for writing. + fs::path path = getPartitionFilePath(flags); - fs::path fromPath = context->paths.ro_loc; - fromPath /= partition->data.name; - if (!fs::exists(fromPath)) + int fd = ::open(path.c_str(), flags); + if (fd == -1) { - MSG_ERR("RWRequest: Partition '%s' for offset 0x%" PRIx32 - " not found in RO directory '%s'\n", - partition->data.name, offset, context->paths.ro_loc); - elog(); + MSG_ERR("Failed to open backing file at '%s': %d\n", + path.c_str(), errno); + throw std::system_error(errno, std::system_category()); } - // copy the file from ro to respective partition - fs::path toPath = context->paths.rw_loc; - if (partition->data.user.data[1] & PARTITION_PRESERVED) + int mprot = PROT_READ | ((flags == O_RDWR) ? PROT_WRITE : 0); + auto map = mmap(NULL, partition.data.actual, mprot, MAP_SHARED, fd, 0); + if (map == MAP_FAILED) { - toPath = context->paths.prsv_loc; + close(fd); + MSG_ERR("Failed to map backing file '%s' for %d bytes: %d\n", + path.c_str(), partition.data.actual, errno); + throw std::system_error(errno, std::system_category()); } - MSG_DBG("RWRequest: Didn't find '%s' under '%s', copying from '%s'\n", - partition->data.name, toPath.c_str(), fromPath.c_str()); - - toPath /= partition->data.name; - if (!fs::copy_file(fromPath, toPath, ec)) + // copy to the reserved memory area + if (flags == O_RDONLY) { - MSG_ERR("RWRequest: Failed to copy file from '%s' to '%s': %d\n", - fromPath.c_str(), toPath.c_str(), ec.value()); - elog(); + len = std::min(partition.data.actual - offset, len); + memcpy(buf, (char *)map + offset, len); } - - rc = Request::open(toPath.c_str(), O_RDWR); - - if (rc != ReturnCode::SUCCESS) + else { - MSG_ERR("RWRequest: Failed to open file at '%s': %d\n", toPath.c_str(), - static_cast(rc)); - elog(); + // if the asked offset + no of bytes to read 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 ((base + offset + len) > (base + partition.data.actual)) + { + munmap(map, partition.data.actual); + close(fd); + + /* FIXME: offset calculation */ + std::stringstream err; + err << "Request size 0x" << std::hex << len << " from offset 0x" + << std::hex << offset << " exceeds the partition size 0x" + << std::hex << partition.data.actual; + throw OutOfBoundsOffset(err.str()); + } + memcpy((char *)map + offset, buf, len); + set_flash_bytemap(ctx, base + offset, len, FLASH_DIRTY); } + munmap(map, partition.data.actual); + close(fd); - return partition; + return len; } } // namespace virtual_pnor -- cgit v1.2.1