From 742a1f6ac66eac80cd1f814430972256b62f9ac5 Mon Sep 17 00:00:00 2001 From: Andrew Jeffery Date: Fri, 2 Mar 2018 09:26:03 +1030 Subject: pnor_partition_table: Refactor to allow tests to specify patch location The Table class was unhelpful for testing in a couple of ways: 1. It attempted to access files on the filesystem whilst parsing ToC entries 2. It incorrectly assumed the location of the files it was accessing Both of these issues come down to handling of patch files and the configuration of the 'actual' member of the partition struct. Hoist the handling of the partition entry's data size out of the ToC parser, and rework the Table constructor to only require a struct mbox_context pointer. We can then use the paths member of mbox_context to find the patch location rather than hard-code the value generated by the configure script. This prompts a rework and rename of the wrapper functions in mboxd_pnor_partition_table.{cpp,h} to better align with the new behaviour of the Table constructor. Reworking the wrappers has knock-on effects in the tests, but the changes are straight-forward. Change-Id: I87e63daf0d28b93566f7e5cb565cbf0790428479 Signed-off-by: Andrew Jeffery --- mboxd_pnor_partition_table.cpp | 26 ++++++-------- mboxd_pnor_partition_table.h | 8 ++--- pnor_partition_table.cpp | 41 ++++++++++------------ pnor_partition_table.hpp | 13 ++++--- test/vpnor/Makefile.am.include | 8 +++++ test/vpnor/create_pnor_partition_table.cpp | 32 +++++++++++------ test/vpnor/create_read_window_oob.cpp | 2 +- test/vpnor/create_read_window_partition_exists.cpp | 2 +- .../vpnor/create_read_window_partition_invalid.cpp | 2 +- .../create_read_window_straddle_partitions.cpp | 2 +- test/vpnor/create_read_window_toc.cpp | 4 +-- test/vpnor/read_patch.cpp | 2 +- test/vpnor/toc_lookup_failed.cpp | 2 +- test/vpnor/toc_lookup_found.cpp | 2 +- test/vpnor/toc_missing_file.cpp | 2 +- test/vpnor/toc_overlap.cpp | 2 +- test/vpnor/write_patch.cpp | 2 +- test/vpnor/write_patch_resize.cpp | 2 +- test/vpnor/write_prsv.cpp | 2 +- test/vpnor/write_ro.cpp | 2 +- test/vpnor/write_rw.cpp | 2 +- 21 files changed, 83 insertions(+), 77 deletions(-) diff --git a/mboxd_pnor_partition_table.cpp b/mboxd_pnor_partition_table.cpp index e58b019..db53dc8 100644 --- a/mboxd_pnor_partition_table.cpp +++ b/mboxd_pnor_partition_table.cpp @@ -26,17 +26,17 @@ int init_vpnor(struct mbox_context *context) strncpy(context->paths.patch_loc, PARTITION_FILES_PATCH_LOC, PATH_MAX); context->paths.prsv_loc[PATH_MAX - 1] = '\0'; - rc = vpnor_create_partition_table_from_path(context, - PARTITION_FILES_RO_LOC); + rc = init_vpnor_from_paths(context); if (rc < 0) + { return rc; + } } return 0; } -int vpnor_create_partition_table_from_path(struct mbox_context *context, - const char *path) +int init_vpnor_from_paths(struct mbox_context *context) { namespace err = sdbusplus::xyz::openbmc_project::Common::Error; namespace fs = std::experimental::filesystem; @@ -44,14 +44,11 @@ int vpnor_create_partition_table_from_path(struct mbox_context *context, if (context && !context->vpnor) { - fs::path dir(path); try { context->vpnor = new vpnor_partition_table; context->vpnor->table = - new openpower::virtual_pnor::partition::Table( - std::move(dir), 1 << context->erase_size_shift, - context->flash_size); + new openpower::virtual_pnor::partition::Table(context); } catch (vpnor::TocEntryError &e) { @@ -87,17 +84,14 @@ int vpnor_copy_bootloader_partition(const struct mbox_context *context) try { - openpower::virtual_pnor::partition::Table blTable( - fs::path{PARTITION_FILES_RO_LOC}, eraseSize, pnorSize); - vpnor_partition_table vtbl{}; - vtbl.table = &blTable; - struct mbox_context local - { - }; + struct mbox_context local = *context; local.vpnor = &vtbl; local.block_size_shift = log_2(eraseSize); - memcpy(&local.paths, &context->paths, sizeof(local.paths)); + + openpower::virtual_pnor::partition::Table blTable(&local); + + vtbl.table = &blTable; size_t tocOffset = 0; diff --git a/mboxd_pnor_partition_table.h b/mboxd_pnor_partition_table.h index 06934b9..d13a2d2 100644 --- a/mboxd_pnor_partition_table.h +++ b/mboxd_pnor_partition_table.h @@ -37,16 +37,14 @@ int init_vpnor(struct mbox_context *context); /** @brief Create a virtual PNOR partition table. * * @param[in] context - mbox context pointer - * @param[in] path - location of the partition file * - * This API is same as above one but it reads the partition file from - * from the given location(path). + * This API is same as above one but requires context->path is initialised + * with all the necessary paths. * * Returns 0 if the call succeeds, else a negative error code. */ -int vpnor_create_partition_table_from_path(struct mbox_context *context, - const char* path); +int init_vpnor_from_paths(struct mbox_context *context); /** @brief Copy bootloader partition (alongwith TOC) to LPC memory * diff --git a/pnor_partition_table.cpp b/pnor_partition_table.cpp index 50d7d59..376b191 100644 --- a/pnor_partition_table.cpp +++ b/pnor_partition_table.cpp @@ -22,11 +22,11 @@ using namespace sdbusplus::xyz::openbmc_project::Common::Error; namespace partition { -Table::Table(fs::path&& directory, size_t blockSize, size_t pnorSize) : - szBytes(sizeof(pnor_partition_table)), directory(std::move(directory)), - numParts(0), blockSize(blockSize), pnorSize(pnorSize) +Table::Table(const struct mbox_context* ctx) : + szBytes(sizeof(pnor_partition_table)), numParts(0), + blockSize(1 << ctx->erase_size_shift), pnorSize(ctx->flash_size) { - preparePartitions(); + preparePartitions(ctx); prepareHeader(); hostTbl = endianFixup(tbl); } @@ -68,10 +68,11 @@ inline void Table::allocateMemory(const fs::path& tocFile) tbl.resize(capacity()); } -void Table::preparePartitions() +void Table::preparePartitions(const struct mbox_context* ctx) { - fs::path tocFile = directory; - tocFile /= PARTITION_TOC_FILE; + const fs::path roDir = ctx->paths.ro_loc; + const fs::path patchDir = ctx->paths.patch_loc; + fs::path tocFile = roDir / PARTITION_TOC_FILE; allocateMemory(tocFile); std::ifstream file(tocFile.c_str()); @@ -81,6 +82,7 @@ void Table::preparePartitions() while (std::getline(file, line)) { pnor_partition& part = table.partitions[numParts]; + fs::path patch; fs::path file; // The ToC file presented in the vpnor squashfs looks like: @@ -124,7 +126,7 @@ void Table::preparePartitions() } } - file = directory / part.data.name; + file = roDir / part.data.name; if (!fs::exists(file)) { std::stringstream err; @@ -132,6 +134,14 @@ void Table::preparePartitions() throw InvalidTocEntry(err.str()); } + patch = patchDir / part.data.name; + if (fs::is_regular_file(patch)) + { + const size_t size = part.data.size * blockSize; + part.data.actual = + std::min(size, static_cast(fs::file_size(patch))); + } + ++numParts; } } @@ -226,20 +236,7 @@ static inline void writeSizes(pnor_partition& part, size_t start, size_t end, part.data.base = align_up(start, blockSize) / blockSize; size_t sizeInBlocks = align_up(size, blockSize) / blockSize; part.data.size = sizeInBlocks; - - // If a a patch partition file exists, populate actual size with its file - // size if it is smaller than the total size. - fs::path patchFile(PARTITION_FILES_PATCH_LOC); - patchFile /= part.data.name; - if (fs::is_regular_file(patchFile)) - { - part.data.actual = - std::min(size, static_cast(fs::file_size(patchFile))); - } - else - { - part.data.actual = size; - } + part.data.actual = size; } static inline void writeUserdata(pnor_partition& part, uint32_t version, diff --git a/pnor_partition_table.hpp b/pnor_partition_table.hpp index 88c8fb9..10dccdd 100644 --- a/pnor_partition_table.hpp +++ b/pnor_partition_table.hpp @@ -7,6 +7,7 @@ #include #include #include "common.h" +#include "mbox.h" #include "pnor_partition_defs.h" namespace openpower @@ -89,15 +90,11 @@ class Table /** @brief Constructor accepting the path of the directory * that houses the PNOR partition files. * - * @param[in] directory - path of the directory housing PNOR partitions - * @param[in] blockSize - PNOR block size, in bytes. See - * open-power/hostboot/blob/master/src/usr/pnor/ffs.h for - * the PNOR FFS structure. - * @param[in] pnorSize - PNOR size, in bytes + * @param[in] ctx - Acquire sizes and paths relevant to the table * * Throws MalformedTocEntry, InvalidTocEntry */ - Table(fs::path&& directory, size_t blockSize, size_t pnorSize); + Table(const struct mbox_context* ctx); Table(const Table&) = delete; Table& operator=(const Table&) = delete; @@ -181,10 +178,12 @@ class Table private: /** @brief Prepares a vector of PNOR partition structures. + * + * @param[in] ctx - An mbox context providing partition locations * * Throws: MalformedTocEntry, InvalidTocEntry */ - void preparePartitions(); + void preparePartitions(const struct mbox_context* ctx); /** @brief Prepares the PNOR header. */ diff --git a/test/vpnor/Makefile.am.include b/test/vpnor/Makefile.am.include index 22bcd41..8d0dd65 100644 --- a/test/vpnor/Makefile.am.include +++ b/test/vpnor/Makefile.am.include @@ -8,6 +8,14 @@ VPNOR_LDADD = -lstdc++fs \ test_vpnor_create_pnor_partition_table_SOURCES = \ $(TEST_MBOX_VPNOR_SRCS) \ + $(TEST_MOCK_SRCS) \ + mboxd_msg.c \ + mboxd_windows.c \ + mboxd_lpc.c \ + mboxd_lpc_virtual.cpp \ + mboxd_pnor_partition_table.cpp \ + mboxd_flash_virtual.cpp \ + pnor_partition.cpp \ %reldir%/create_pnor_partition_table.cpp test_vpnor_create_pnor_partition_table_LDFLAGS = $(OESDK_TESTCASE_FLAGS) test_vpnor_create_pnor_partition_table_LDADD = $(VPNOR_LDADD) diff --git a/test/vpnor/create_pnor_partition_table.cpp b/test/vpnor/create_pnor_partition_table.cpp index 09f481d..761b5cd 100644 --- a/test/vpnor/create_pnor_partition_table.cpp +++ b/test/vpnor/create_pnor_partition_table.cpp @@ -6,13 +6,22 @@ #include "config.h" #include "pnor_partition_table.hpp" +extern "C" { +#include "test/mbox.h" +#include "test/system.h" +} + #include "test/vpnor/tmpd.hpp" static const auto BLOCK_SIZE = 4 * 1024; +static const auto ERASE_SIZE = BLOCK_SIZE; static const auto PNOR_SIZE = 64 * 1024 * 1024; +static const auto MEM_SIZE = PNOR_SIZE; +static const auto N_WINDOWS = 1; +static const auto WINDOW_SIZE = BLOCK_SIZE; const std::string toc[] = { - "partition01=HBB,00000000,00000400,80,ECC,PRESERVED", + "partition01=HBB,00000000,00001000,80,ECC,PRESERVED", }; constexpr auto partitionName = "HBB"; @@ -20,19 +29,20 @@ namespace test = openpower::virtual_pnor::test; int main() { - struct mbox_context _ctx, *ctx = &_ctx; + struct mbox_context* ctx; + system_set_reserved_size(MEM_SIZE); + system_set_mtd_sizes(PNOR_SIZE, ERASE_SIZE); + ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - - const openpower::virtual_pnor::partition::Table table(root.ro(), BLOCK_SIZE, - PNOR_SIZE); + const openpower::virtual_pnor::partition::Table table(ctx); pnor_partition_table expectedTable{}; expectedTable.data.magic = PARTITION_HEADER_MAGIC; expectedTable.data.version = PARTITION_VERSION_1; - expectedTable.data.size = 1; // 1 block + expectedTable.data.size = 1; expectedTable.data.entry_size = sizeof(pnor_partition); - expectedTable.data.entry_count = 1; // 1 partition + expectedTable.data.entry_count = 1; expectedTable.data.block_size = BLOCK_SIZE; expectedTable.data.block_count = (PNOR_SIZE) / expectedTable.data.block_size; @@ -41,9 +51,9 @@ int main() pnor_partition expectedPartition{}; strcpy(expectedPartition.data.name, partitionName); - expectedPartition.data.base = 0; // starts at offset 0 - expectedPartition.data.size = 1; // 1 block - expectedPartition.data.actual = 0x400; // 1024 bytes + expectedPartition.data.base = 0; + expectedPartition.data.size = 1; + expectedPartition.data.actual = 0x1000; expectedPartition.data.id = 1; expectedPartition.data.pid = PARENT_PATITION_ID; expectedPartition.data.type = PARTITION_TYPE_DATA; @@ -63,7 +73,7 @@ int main() sizeof(pnor_partition)); assert(rc == 0); - const pnor_partition& first = table.partition(0); // Partition at offset 0 + const pnor_partition& first = table.partition(0); rc = memcmp(&first, &result.partitions[0], sizeof(pnor_partition)); assert(rc == 0); diff --git a/test/vpnor/create_read_window_oob.cpp b/test/vpnor/create_read_window_oob.cpp index ce85617..8b2b0f4 100644 --- a/test/vpnor/create_read_window_oob.cpp +++ b/test/vpnor/create_read_window_oob.cpp @@ -49,7 +49,7 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/create_read_window_partition_exists.cpp b/test/vpnor/create_read_window_partition_exists.cpp index d72f6d1..f0466c9 100644 --- a/test/vpnor/create_read_window_partition_exists.cpp +++ b/test/vpnor/create_read_window_partition_exists.cpp @@ -57,7 +57,7 @@ int main() test::VpnorRoot root(ctx, toc, BLOCK_SIZE); root.write("HBB", data, sizeof(data)); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); int rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/create_read_window_partition_invalid.cpp b/test/vpnor/create_read_window_partition_invalid.cpp index 52dd159..cdd895b 100644 --- a/test/vpnor/create_read_window_partition_invalid.cpp +++ b/test/vpnor/create_read_window_partition_invalid.cpp @@ -45,7 +45,7 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/create_read_window_straddle_partitions.cpp b/test/vpnor/create_read_window_straddle_partitions.cpp index ca0ac0b..67149fa 100644 --- a/test/vpnor/create_read_window_straddle_partitions.cpp +++ b/test/vpnor/create_read_window_straddle_partitions.cpp @@ -54,7 +54,7 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); int rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/create_read_window_toc.cpp b/test/vpnor/create_read_window_toc.cpp index 00b9dfe..1f68371 100644 --- a/test/vpnor/create_read_window_toc.cpp +++ b/test/vpnor/create_read_window_toc.cpp @@ -55,12 +55,12 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor::partition::Table table(root.ro(), BLOCK_SIZE, PNOR_SIZE); + vpnor::partition::Table table(ctx); /* Make sure the ToC exactly fits in the space allocated for it */ assert(table.capacity() == TOC_PART_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/read_patch.cpp b/test/vpnor/read_patch.cpp index b06907c..2a91b66 100644 --- a/test/vpnor/read_patch.cpp +++ b/test/vpnor/read_patch.cpp @@ -56,7 +56,7 @@ int main() std::vector patch(PATCH_SIZE, 0xff); root.patch("ONE", patch.data(), patch.size()); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); int rc = mbox_command_dispatch(ctx, get_info, sizeof(get_info)); assert(rc == 1); diff --git a/test/vpnor/toc_lookup_failed.cpp b/test/vpnor/toc_lookup_failed.cpp index a35975a..5230ab1 100644 --- a/test/vpnor/toc_lookup_failed.cpp +++ b/test/vpnor/toc_lookup_failed.cpp @@ -39,7 +39,7 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor::partition::Table table(root.ro(), BLOCK_SIZE, PNOR_SIZE); + vpnor::partition::Table table(ctx); try { diff --git a/test/vpnor/toc_lookup_found.cpp b/test/vpnor/toc_lookup_found.cpp index c98b718..fdc5a10 100644 --- a/test/vpnor/toc_lookup_found.cpp +++ b/test/vpnor/toc_lookup_found.cpp @@ -39,7 +39,7 @@ int main() ctx = mbox_create_test_context(N_WINDOWS, WINDOW_SIZE); test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor::partition::Table table(root.ro(), BLOCK_SIZE, PNOR_SIZE); + vpnor::partition::Table table(ctx); const struct pnor_partition& part = table.partition("TWO"); assert(part.data.id == 2); diff --git a/test/vpnor/toc_missing_file.cpp b/test/vpnor/toc_missing_file.cpp index aad80b4..dd602bb 100644 --- a/test/vpnor/toc_missing_file.cpp +++ b/test/vpnor/toc_missing_file.cpp @@ -44,7 +44,7 @@ int main() try { - vpnor::partition::Table table(root.ro(), BLOCK_SIZE, PNOR_SIZE); + vpnor::partition::Table table(ctx); } catch (vpnor::InvalidTocEntry& e) { diff --git a/test/vpnor/toc_overlap.cpp b/test/vpnor/toc_overlap.cpp index 3f1ca3f..97748e2 100644 --- a/test/vpnor/toc_overlap.cpp +++ b/test/vpnor/toc_overlap.cpp @@ -41,7 +41,7 @@ int main() try { - vpnor::partition::Table table(root.ro(), BLOCK_SIZE, PNOR_SIZE); + vpnor::partition::Table table(ctx); } catch (vpnor::InvalidTocEntry& e) { diff --git a/test/vpnor/write_patch.cpp b/test/vpnor/write_patch.cpp index b0d5fa7..f45ab26 100644 --- a/test/vpnor/write_patch.cpp +++ b/test/vpnor/write_patch.cpp @@ -50,7 +50,7 @@ int main(void) fs::path patch = root.patch() / "TEST1"; assert(fs::copy_file(root.ro() / "TEST1", patch)); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); /* Test */ memset(src, 0x33, sizeof(src)); diff --git a/test/vpnor/write_patch_resize.cpp b/test/vpnor/write_patch_resize.cpp index 24c71b0..350b210 100644 --- a/test/vpnor/write_patch_resize.cpp +++ b/test/vpnor/write_patch_resize.cpp @@ -49,7 +49,7 @@ int main(void) std::vector patchContent(PATCH_SIZE, 0xaa); root.patch("TEST1", patchContent.data(), patchContent.size()); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); /* Test */ std::vector update(UPDATE_SIZE, 0x55); diff --git a/test/vpnor/write_prsv.cpp b/test/vpnor/write_prsv.cpp index 03624e4..faa05db 100644 --- a/test/vpnor/write_prsv.cpp +++ b/test/vpnor/write_prsv.cpp @@ -39,7 +39,7 @@ int main(void) verbosity = (verbose)2; test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); /* Test */ memset(src, 0xaa, sizeof(src)); diff --git a/test/vpnor/write_ro.cpp b/test/vpnor/write_ro.cpp index fe934dd..504822a 100644 --- a/test/vpnor/write_ro.cpp +++ b/test/vpnor/write_ro.cpp @@ -36,7 +36,7 @@ int main(void) verbosity = (verbose)2; test::VpnorRoot root(ctx, toc, BLOCK_SIZE); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); /* Test */ rc = write_flash(ctx, 0x1000, src, sizeof(src)); diff --git a/test/vpnor/write_rw.cpp b/test/vpnor/write_rw.cpp index eff959b..aba12bb 100644 --- a/test/vpnor/write_rw.cpp +++ b/test/vpnor/write_rw.cpp @@ -40,7 +40,7 @@ int main(void) test::VpnorRoot root(ctx, toc, BLOCK_SIZE); /* write_flash() doesn't copy the file for us */ assert(fs::copy_file(root.ro() / "TEST1", root.rw() / "TEST1")); - vpnor_create_partition_table_from_path(ctx, root.ro().c_str()); + init_vpnor_from_paths(ctx); /* Test */ memset(src, 0xbb, sizeof(src)); -- cgit v1.2.1