Attention is currently required from: Jakub Czapiga. Sudheer Amrabadi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/60797 )
Change subject: Revert "libpayload/libc/fmap: Implement new FlashMap API" ......................................................................
Revert "libpayload/libc/fmap: Implement new FlashMap API"
This reverts commit 8fac662f308cdfbeec3f71d4728f71ad79c06925.
Change-Id: Ieccf2d709206e1cb53fec00d0fafb3748b9dedb6 --- D payloads/libpayload/include/fmap.h A payloads/libpayload/include/fmap_serialized.h M payloads/libpayload/include/libpayload.h M payloads/libpayload/libc/fmap.c D payloads/libpayload/tests/libc/Makefile.inc D payloads/libpayload/tests/libc/fmap_locate_area-test.c 6 files changed, 76 insertions(+), 180 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/97/60797/1
diff --git a/payloads/libpayload/include/fmap.h b/payloads/libpayload/include/fmap.h deleted file mode 100644 index 53ebe23..0000000 --- a/payloads/libpayload/include/fmap.h +++ /dev/null @@ -1,12 +0,0 @@ -/* SPDX_License-Identifier: BSD-3-Clause */ - -#ifndef _FMAP_H -#define _FMAP_H - -#include <commonlib/bsd/cb_err.h> -#include <stddef.h> - -/* Looks for area with |name| in FlashMap. Requires lib_sysinfo.fmap_cache. */ -cb_err_t fmap_locate_area(const char *name, size_t *offset, size_t *size); - -#endif /* _FMAP_H */ diff --git a/payloads/libpayload/include/fmap_serialized.h b/payloads/libpayload/include/fmap_serialized.h new file mode 100644 index 0000000..53a09af --- /dev/null +++ b/payloads/libpayload/include/fmap_serialized.h @@ -0,0 +1,74 @@ +/* + * Copyright 2010, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + * Alternatively, this software may be distributed under the terms of the + * GNU General Public License ("GPL") version 2 as published by the Free + * Software Foundation. + */ + +#ifndef FLASHMAP_SERIALIZED_H__ +#define FLASHMAP_SERIALIZED_H__ + +#include <stdint.h> + +#define FMAP_SIGNATURE "__FMAP__" +#define FMAP_VER_MAJOR 1 /* this header's FMAP minor version */ +#define FMAP_VER_MINOR 1 /* this header's FMAP minor version */ +#define FMAP_STRLEN 32 /* maximum length for strings, */ + /* including null-terminator */ + +enum fmap_flags { + FMAP_AREA_STATIC = 1 << 0, + FMAP_AREA_COMPRESSED = 1 << 1, + FMAP_AREA_RO = 1 << 2, + FMAP_AREA_PRESERVE = 1 << 3, +}; + +/* Mapping of volatile and static regions in firmware binary */ +struct fmap_area { + uint32_t offset; /* offset relative to base */ + uint32_t size; /* size in bytes */ + uint8_t name[FMAP_STRLEN]; /* descriptive name */ + uint16_t flags; /* flags for this area */ +} __packed; + +struct fmap { + uint8_t signature[8]; /* "__FMAP__" (0x5F5F464D41505F5F) */ + uint8_t ver_major; /* major version */ + uint8_t ver_minor; /* minor version */ + uint64_t base; /* address of the firmware binary */ + uint32_t size; /* size of firmware binary in bytes */ + uint8_t name[FMAP_STRLEN]; /* name of this firmware binary */ + uint16_t nareas; /* number of areas described by + fmap_areas[] below */ + struct fmap_area areas[]; +} __packed; + +#endif /* FLASHMAP_SERIALIZED_H__ */ diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 250ba05..a318d16 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -45,11 +45,10 @@ #include <stdbool.h> #include <libpayload-config.h> #include <cbgfx.h> -#include <commonlib/bsd/fmap_serialized.h> #include <ctype.h> #include <die.h> #include <endian.h> -#include <fmap.h> +#include <fmap_serialized.h> #include <ipchksum.h> #include <kconfig.h> #include <stddef.h> diff --git a/payloads/libpayload/libc/fmap.c b/payloads/libpayload/libc/fmap.c index 2d185a7..b7d6491 100644 --- a/payloads/libpayload/libc/fmap.c +++ b/payloads/libpayload/libc/fmap.c @@ -28,60 +28,10 @@
#include <libpayload-config.h> #include <libpayload.h> -#include <commonlib/bsd/fmap_serialized.h> #include <coreboot_tables.h> #include <cbfs.h> -#include <boot_device.h> +#include <fmap_serialized.h> #include <stdint.h> -#include <arch/virtual.h> - -/* Private fmap cache. */ -static struct fmap *_fmap_cache; - -static cb_err_t fmap_find_area(struct fmap *fmap, const char *name, size_t *offset, - size_t *size) -{ - for (size_t i = 0; i < le32toh(fmap->nareas); ++i) { - if (strncmp((const char *)fmap->areas[i].name, name, FMAP_STRLEN) != 0) - continue; - if (offset) - *offset = le32toh(fmap->areas[i].offset); - if (size) - *size = le32toh(fmap->areas[i].size); - return CB_SUCCESS; - } - - return CB_ERR; -} - -static bool fmap_is_signature_valid(struct fmap *fmap) -{ - return memcmp(fmap->signature, FMAP_SIGNATURE, sizeof(fmap->signature)) == 0; -} - -static bool fmap_setup_cache(void) -{ - /* Use FMAP cache if available */ - if (lib_sysinfo.fmap_cache - && fmap_is_signature_valid((struct fmap *)phys_to_virt(lib_sysinfo.fmap_cache))) { - _fmap_cache = (struct fmap *)phys_to_virt(lib_sysinfo.fmap_cache); - return true; - } - - return false; -} - -cb_err_t fmap_locate_area(const char *name, size_t *offset, size_t *size) -{ - if (!_fmap_cache && !fmap_setup_cache()) - return CB_ERR; - - return fmap_find_area(_fmap_cache, name, offset, size); -} - -/*********************************************************************************************** - * LEGACY CODE * - **********************************************************************************************/
int fmap_region_by_name(const uint32_t fmap_offset, const char * const name, uint32_t * const offset, uint32_t * const size) diff --git a/payloads/libpayload/tests/libc/Makefile.inc b/payloads/libpayload/tests/libc/Makefile.inc deleted file mode 100644 index 5f92bdf..0000000 --- a/payloads/libpayload/tests/libc/Makefile.inc +++ /dev/null @@ -1,3 +0,0 @@ -tests-y += fmap_locate_area-test - -fmap_locate_area-test-srcs += tests/libc/fmap_locate_area-test.c diff --git a/payloads/libpayload/tests/libc/fmap_locate_area-test.c b/payloads/libpayload/tests/libc/fmap_locate_area-test.c deleted file mode 100644 index ce7c36b..0000000 --- a/payloads/libpayload/tests/libc/fmap_locate_area-test.c +++ /dev/null @@ -1,112 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include "../libc/fmap.c" - -#include <libpayload.h> -#include <tests/test.h> - - -/* Mocks */ -struct sysinfo_t lib_sysinfo; -unsigned long virtual_offset = 0; - -static void reset_fmap_cache(void) -{ - _fmap_cache = NULL; -} - -static int setup_fmap_test(void **state) -{ - reset_fmap_cache(); - lib_sysinfo.fmap_cache = 0; - return 0; -} - -static void test_fmap_locate_area_no_fmap_available(void **state) -{ - size_t offset = 0; - size_t size = 0; - - assert_int_equal(-1, fmap_locate_area("COREBOOT", &offset, &size)); -} - -static void test_fmap_locate_area_incorrect_signature(void **state) -{ - size_t offset = 0; - size_t size = 0; - struct fmap mock_fmap = { - .signature = "NOT_MAP", - }; - lib_sysinfo.fmap_cache = (uintptr_t)&mock_fmap; - - assert_int_equal(-1, fmap_locate_area("COREBOOT", &offset, &size)); -} - -static void test_fmap_locate_area_success(void **state) -{ - size_t offset = 0; - size_t size = 0; - struct fmap mock_fmap = { - .signature = FMAP_SIGNATURE, - .ver_major = 1, - .ver_minor = 1, - .base = 0xAABB, - .size = 0x10000, - .nareas = 3, - }; - struct fmap_area area_1 = { - .size = 0x1100, - .offset = 0x11, - .name = {'F', 'I', 'R', 'S', 'T', '_', 'A', 'R', 'E', 'A', 0}, - .flags = 0, - }; - struct fmap_area area_2 = { - .size = 0x2200, - .offset = 0x1111, - .name = {'S', 'E', 'C', 'O', 'N', 'D', '_', 'A', 'R', 'E', 'A', 0}, - .flags = 0, - }; - struct fmap_area area_3 = { - .size = 0x100, - .offset = 0x3311, - .name = {'T', 'H', 'I', 'R', 'D', '_', 'A', 'R', 'E', 'A', 0}, - .flags = 0, - }; - u8 fmap_buffer[sizeof(struct fmap) + 3 * sizeof(struct fmap_area)]; - memcpy(fmap_buffer, &mock_fmap, sizeof(mock_fmap)); - memcpy(&fmap_buffer[sizeof(mock_fmap)], &area_1, sizeof(area_1)); - memcpy(&fmap_buffer[sizeof(mock_fmap) + sizeof(area_1)], &area_2, sizeof(area_2)); - memcpy(&fmap_buffer[sizeof(mock_fmap) + sizeof(area_1) + sizeof(area_2)], &area_3, - sizeof(area_3)); - - /* Cache only */ - reset_fmap_cache(); - lib_sysinfo.fmap_cache = (uintptr_t)fmap_buffer; - - assert_int_equal(0, fmap_locate_area("FIRST_AREA", &offset, &size)); - assert_int_equal(area_1.offset, offset); - assert_int_equal(area_1.size, size); - - assert_int_equal(0, fmap_locate_area("THIRD_AREA", &offset, &size)); - assert_int_equal(area_3.offset, offset); - assert_int_equal(area_3.size, size); - - assert_int_equal(0, fmap_locate_area("SECOND_AREA", &offset, &size)); - assert_int_equal(area_2.offset, offset); - assert_int_equal(area_2.size, size); - - reset_fmap_cache(); -} - -#define FMAP_LOCATE_AREA_TEST(fn) cmocka_unit_test_setup(fn, setup_fmap_test) - -int main(void) -{ - const struct CMUnitTest tests[] = { - FMAP_LOCATE_AREA_TEST(test_fmap_locate_area_no_fmap_available), - FMAP_LOCATE_AREA_TEST(test_fmap_locate_area_incorrect_signature), - FMAP_LOCATE_AREA_TEST(test_fmap_locate_area_success), - }; - - return lp_run_group_tests(tests, NULL, NULL); -}
-
Sudheer Amrabadi (Code Review)