From 4756206b07f2250155c9c4c1d75577281ea111f5 Mon Sep 17 00:00:00 2001 From: Hsin-Te Yuan Date: Tue, 28 Jun 2022 16:38:26 +0800 Subject: vboot_api_kernel.c: Remove kparams_ptr Pass VbSelectAndLoadKernelParams kparams as a function argument instead of using global variable kparams_ptr. Remove VbSelectAndLoadKernel and replace its tests with the unit tests for vb2_set_boot_mode, vb2api_kernel_phase2, vb2api_kernel_finalize, and vb2api_normal_boot. BUG=b:172339016 BRANCH=none TEST=make runtests Cq-Depend: chromium:3731710 Signed-off-by: Hsin-Te Yuan Change-Id: I26895ced5e310b2894b9d42d0ad5514d3b0b930a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3731412 Reviewed-by: Yu-Ping Wu --- firmware/2lib/2auxfw_sync.c | 1 + firmware/2lib/2ec_sync.c | 1 + firmware/2lib/2kernel.c | 5 +- firmware/2lib/2rsa.c | 1 - firmware/2lib/2secdata_kernel.c | 3 +- firmware/2lib/2stub.c | 27 ----------- firmware/2lib/include/2api.h | 57 +++-------------------- firmware/include/vboot_api.h | 21 ++++----- firmware/lib/include/vboot_struct.h | 2 +- firmware/lib/include/vboot_test.h | 17 ------- firmware/lib/vboot_api_kernel.c | 91 ++++++------------------------------- 11 files changed, 36 insertions(+), 190 deletions(-) delete mode 100644 firmware/lib/include/vboot_test.h (limited to 'firmware') diff --git a/firmware/2lib/2auxfw_sync.c b/firmware/2lib/2auxfw_sync.c index eaea1d46..a60165c1 100644 --- a/firmware/2lib/2auxfw_sync.c +++ b/firmware/2lib/2auxfw_sync.c @@ -48,6 +48,7 @@ static vb2_error_t auxfw_sync_check_update(struct vb2_context *ctx, return vb2ex_auxfw_check(severity); } +test_mockable vb2_error_t vb2api_auxfw_sync(struct vb2_context *ctx) { enum vb2_auxfw_update_severity fw_update = VB2_AUXFW_NO_UPDATE; diff --git a/firmware/2lib/2ec_sync.c b/firmware/2lib/2ec_sync.c index e75313b4..b1bc7ebd 100644 --- a/firmware/2lib/2ec_sync.c +++ b/firmware/2lib/2ec_sync.c @@ -338,6 +338,7 @@ static vb2_error_t ec_sync_phase2(struct vb2_context *ctx) return sync_ec(ctx); } +test_mockable vb2_error_t vb2api_ec_sync(struct vb2_context *ctx) { struct vb2_shared_data *sd = vb2_get_sd(ctx); diff --git a/firmware/2lib/2kernel.c b/firmware/2lib/2kernel.c index bec9c507..30d6bbe6 100644 --- a/firmware/2lib/2kernel.c +++ b/firmware/2lib/2kernel.c @@ -37,7 +37,8 @@ static int vb2_reset_nv_requests(struct vb2_context *ctx) return need_reboot; } -vb2_error_t vb2api_normal_boot(struct vb2_context *ctx) +vb2_error_t vb2api_normal_boot(struct vb2_context *ctx, + VbSelectAndLoadKernelParams *kparams) { struct vb2_shared_data *sd = vb2_get_sd(ctx); uint32_t max_rollforward = vb2_nv_get(ctx, @@ -51,7 +52,7 @@ vb2_error_t vb2api_normal_boot(struct vb2_context *ctx) return VB2_REQUEST_REBOOT; } - vb2_error_t rv = VbTryLoadKernel(ctx, VB_DISK_FLAG_FIXED); + vb2_error_t rv = VbTryLoadKernel(ctx, VB_DISK_FLAG_FIXED, kparams); VB2_DEBUG("Checking if TPM kernel version needs advancing\n"); diff --git a/firmware/2lib/2rsa.c b/firmware/2lib/2rsa.c index dcd8bad0..eb07b04c 100644 --- a/firmware/2lib/2rsa.c +++ b/firmware/2lib/2rsa.c @@ -14,7 +14,6 @@ #include "2rsa_private.h" #include "2sha.h" #include "2sysincludes.h" -#include "vboot_test.h" /** * a[] -= mod diff --git a/firmware/2lib/2secdata_kernel.c b/firmware/2lib/2secdata_kernel.c index 0d4208fa..9973cd46 100644 --- a/firmware/2lib/2secdata_kernel.c +++ b/firmware/2lib/2secdata_kernel.c @@ -11,7 +11,6 @@ #include "2secdata.h" #include "2secdata_struct.h" #include "2sysincludes.h" -#include "vboot_test.h" #define MAJOR_VER(x) (((x) & 0xf0) >> 4) #define MINOR_VER(x) ((x) & 0x0f) @@ -213,6 +212,7 @@ uint32_t vb2_secdata_kernel_get(struct vb2_context *ctx, return 0; } +test_mockable void vb2_secdata_kernel_set(struct vb2_context *ctx, enum vb2_secdata_kernel_param param, uint32_t value) @@ -273,6 +273,7 @@ void vb2_secdata_kernel_set(struct vb2_context *ctx, VB2_REC_OR_DIE(ctx, "%s\n", msg); } +test_mockable const uint8_t *vb2_secdata_kernel_get_ec_hash(struct vb2_context *ctx) { struct vb2_shared_data *sd = vb2_get_sd(ctx); diff --git a/firmware/2lib/2stub.c b/firmware/2lib/2stub.c index ef0bbd8a..51d2eb20 100644 --- a/firmware/2lib/2stub.c +++ b/firmware/2lib/2stub.c @@ -159,33 +159,6 @@ vb2_error_t vb2ex_auxfw_finalize(struct vb2_context *ctx) return VB2_SUCCESS; } -/*****************************************************************************/ -/* UI-related stubs */ - -__attribute__((weak)) -vb2_error_t vb2ex_broken_screen_ui(struct vb2_context *ctx) -{ - return VB2_SUCCESS; -} - -__attribute__((weak)) -vb2_error_t vb2ex_manual_recovery_ui(struct vb2_context *ctx) -{ - return VB2_SUCCESS; -} - -__attribute__((weak)) -vb2_error_t vb2ex_developer_ui(struct vb2_context *ctx) -{ - return VB2_SUCCESS; -} - -__attribute__((weak)) -vb2_error_t vb2ex_diagnostic_ui(struct vb2_context *ctx) -{ - return VB2_SUCCESS; -} - /*****************************************************************************/ /* Timer-related stubs */ diff --git a/firmware/2lib/include/2api.h b/firmware/2lib/include/2api.h index ff89a29d..a833da7c 100644 --- a/firmware/2lib/include/2api.h +++ b/firmware/2lib/include/2api.h @@ -28,6 +28,7 @@ #include "2return_codes.h" #include "2rsa.h" #include "2secdata_struct.h" +#include "vboot_api.h" #define _VB2_TRY_IMPL(expr, ctx, recovery_reason, ...) do { \ vb2_error_t _vb2_try_rv = (expr); \ @@ -165,14 +166,14 @@ enum vb2_context_flags { /* * System supports EC software sync. Caller may set this flag at any - * time before calling VbSelectAndLoadKernel(). + * time before calling vb2api_kernel_phase2(). */ VB2_CONTEXT_EC_SYNC_SUPPORTED = (1 << 15), /* * EC software sync is slow to update; warning screen should be * displayed. Caller may set this flag at any time before calling - * VbSelectAndLoadKernel(). Deprecated as part of chromium:1038259. + * vb2api_kernel_phase2(). Deprecated as part of chromium:1038259. */ VB2_CONTEXT_DEPRECATED_EC_SYNC_SLOW = (1 << 16), @@ -847,9 +848,11 @@ vb2_error_t vb2api_kernel_phase2(struct vb2_context *ctx); * Handle a normal boot. * * @param ctx Vboot context. + * @param kparams Params specific to loading the kernel * @return VB2_SUCCESS, or error code on error. */ -vb2_error_t vb2api_normal_boot(struct vb2_context *ctx); +vb2_error_t vb2api_normal_boot(struct vb2_context *ctx, + VbSelectAndLoadKernelParams *kparams); /** * Finalize for kernel verification stage. @@ -1484,54 +1487,6 @@ vb2_error_t vb2ex_ec_battery_cutoff(void); /*****************************************************************************/ /* Functions for firmware UI. */ -/* TODO(b/172339016): Remove vb2ex_*_ui(). */ - -/** - * UI for a non-manual recovery ("BROKEN"). - * - * Enter the broken screen UI, which shows that an unrecoverable error was - * encountered last boot. Wait for the user to physically reset or shut down. - * - * @param ctx Vboot context - * @return VB2_SUCCESS, or non-zero error code. - */ -vb2_error_t vb2ex_broken_screen_ui(struct vb2_context *ctx); - -/** - * UI for a manual recovery-mode boot. - * - * Enter the recovery menu, which prompts the user to insert recovery media, - * navigate the step-by-step recovery, or enter developer mode if allowed. - * - * @param ctx Vboot context - * @return VB2_SUCCESS, or non-zero error code. - */ -vb2_error_t vb2ex_manual_recovery_ui(struct vb2_context *ctx); - -/** - * UI for a developer-mode boot. - * - * Enter the developer menu, which provides options to switch out of developer - * mode, boot from external media, use legacy bootloader, or boot Chrome OS from - * disk. - * - * If a timeout occurs, take the default boot action. - * - * @param ctx Vboot context - * @return VB2_SUCCESS, or non-zero error code. - */ -vb2_error_t vb2ex_developer_ui(struct vb2_context *ctx); - -/** - * UI for a diagnostic tools boot. - * - * Enter the diagnostic tools menu, which provides debug information and - * diagnostic tests of various hardware components. - * - * @param ctx Vboot context - * @return VB2_SUCCESS, or non-zero error code. - */ -vb2_error_t vb2ex_diagnostic_ui(struct vb2_context *ctx); /** * Get the vboot debug info. diff --git a/firmware/include/vboot_api.h b/firmware/include/vboot_api.h index ce3ac2d6..4dba0a06 100644 --- a/firmware/include/vboot_api.h +++ b/firmware/include/vboot_api.h @@ -45,15 +45,14 @@ typedef struct VbSharedDataHeader VbSharedDataHeader; typedef void *VbExDiskHandle_t; typedef struct VbSelectAndLoadKernelParams { - /* Inputs to VbSelectAndLoadKernel() */ + /* Inputs to VbTryLoadKernel() */ /* Destination buffer for kernel (normally at 0x100000 on x86) */ void *kernel_buffer; /* Size of kernel buffer in bytes */ uint32_t kernel_buffer_size; /* - * Outputs from VbSelectAndLoadKernel(); valid only if it returns - * success. + * Outputs from VbTryLoadKernel(); valid only if it returns success. */ /* Handle of disk containing loaded kernel */ VbExDiskHandle_t disk_handle; @@ -69,14 +68,6 @@ typedef struct VbSelectAndLoadKernelParams { uint32_t flags; } VbSelectAndLoadKernelParams; -/** - * Select and loads the kernel. - * - * Returns VB2_SUCCESS if success, non-zero if error; on error, caller - * should reboot. */ -vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, - VbSelectAndLoadKernelParams *kparams); - /** * Attempt loading a kernel from the specified type(s) of disks. * @@ -85,9 +76,11 @@ vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, * * @param ctx Vboot context * @param disk_flags Flags to pass to VbExDiskGetInfo() + * @param kparams Params specific to loading the kernel * @return VB2_SUCCESS or the most specific VB2_ERROR_LK error. */ -vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags); +vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags, + VbSelectAndLoadKernelParams *kparams); /* miniOS flags */ @@ -106,10 +99,12 @@ vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags); * * @param ctx Vboot context * @param minios_flags Flags for miniOS + * @param kparams Params specific to loading the kernel * @return VB2_SUCCESS or the most specific VB2_ERROR_LK error. */ vb2_error_t VbTryLoadMiniOsKernel(struct vb2_context *ctx, - uint32_t minios_flags); + uint32_t minios_flags, + VbSelectAndLoadKernelParams *kparams); /*****************************************************************************/ /* Disk access (previously in boot_device.h) */ diff --git a/firmware/lib/include/vboot_struct.h b/firmware/lib/include/vboot_struct.h index 374dfd40..edcb2b45 100644 --- a/firmware/lib/include/vboot_struct.h +++ b/firmware/lib/include/vboot_struct.h @@ -137,7 +137,7 @@ typedef struct VbSharedDataHeader { uint8_t reserved4[7]; /* Flags from firmware keyblock */ uint64_t fw_keyblock_flags; - /* Kernel TPM version at start of VbSelectAndLoadKernel() */ + /* Kernel TPM version at start of vb2api_kernel_phase1 */ uint32_t kernel_version_tpm_start; /* Kernel lowest version found */ uint32_t kernel_version_lowest; diff --git a/firmware/lib/include/vboot_test.h b/firmware/lib/include/vboot_test.h deleted file mode 100644 index fb1f52ad..00000000 --- a/firmware/lib/include/vboot_test.h +++ /dev/null @@ -1,17 +0,0 @@ -/* Copyright 2019 The Chromium OS Authors. All rights reserved. - * Use of this source code is governed by a BSD-style license that can be - * found in the LICENSE file. - * - * This header is for APIs that are only used by test code. - */ - -#ifndef VBOOT_REFERENCE_TEST_API_H_ -#define VBOOT_REFERENCE_TEST_API_H_ - -/**************************************************************************** - * vboot_api_kernel.c */ - -struct VbSelectAndLoadKernelParams; -struct VbSelectAndLoadKernelParams **VbApiKernelGetParamsPtr(void); - -#endif /* VBOOT_REFERENCE_TEST_API_H_ */ diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index f4e5146a..0fdc2d33 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -15,18 +15,6 @@ #include "load_kernel_fw.h" #include "vboot_api.h" #include "vboot_struct.h" -#include "vboot_test.h" - -/* Global variables */ -static VbSelectAndLoadKernelParams *kparams_ptr; - -#ifdef CHROMEOS_ENVIRONMENT -/* Global variable accessor for unit tests */ -struct VbSelectAndLoadKernelParams **VbApiKernelGetParamsPtr(void) -{ - return &kparams_ptr; -} -#endif static int is_valid_disk(VbDiskInfo *info, uint32_t disk_flags) { @@ -40,7 +28,8 @@ static int is_valid_disk(VbDiskInfo *info, uint32_t disk_flags) static vb2_error_t VbTryLoadKernelImpl(struct vb2_context *ctx, uint32_t disk_flags, int minios, - uint32_t minios_flags) + uint32_t minios_flags, + VbSelectAndLoadKernelParams *kparams) { vb2_error_t rv = VB2_ERROR_LK_NO_DISK_FOUND; VbDiskInfo* disk_info = NULL; @@ -48,11 +37,8 @@ static vb2_error_t VbTryLoadKernelImpl(struct vb2_context *ctx, uint32_t i; vb2_error_t new_rv; - /* TODO: Should have been set by VbSelectAndLoadKernel. Remove when - this global is no longer needed. */ - VB2_ASSERT(kparams_ptr); - - kparams_ptr->disk_handle = NULL; + VB2_ASSERT(kparams); + kparams->disk_handle = NULL; /* Find disks */ if (VB2_SUCCESS != VbExDiskGetInfo(&disk_info, &disk_count, disk_flags)) @@ -70,14 +56,14 @@ static vb2_error_t VbTryLoadKernelImpl(struct vb2_context *ctx, disk_info[i].flags); continue; } - kparams_ptr->disk_handle = disk_info[i].handle; + kparams->disk_handle = disk_info[i].handle; if (minios) { - new_rv = LoadMiniOsKernel(ctx, kparams_ptr, + new_rv = LoadMiniOsKernel(ctx, kparams, &disk_info[i], minios_flags); VB2_DEBUG("LoadMiniOsKernel() = %#x\n", new_rv); } else { - new_rv = LoadKernel(ctx, kparams_ptr, &disk_info[i]); + new_rv = LoadKernel(ctx, kparams, &disk_info[i]); VB2_DEBUG("LoadKernel() = %#x\n", new_rv); } @@ -118,69 +104,20 @@ static vb2_error_t VbTryLoadKernelImpl(struct vb2_context *ctx, } test_mockable -vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags) +vb2_error_t VbTryLoadKernel(struct vb2_context *ctx, uint32_t disk_flags, + VbSelectAndLoadKernelParams *kparams) { ctx->flags &= ~VB2_CONTEXT_DISABLE_TPM; - return VbTryLoadKernelImpl(ctx, disk_flags, 0, 0); + return VbTryLoadKernelImpl(ctx, disk_flags, 0, 0, kparams); } test_mockable vb2_error_t VbTryLoadMiniOsKernel(struct vb2_context *ctx, - uint32_t minios_flags) + uint32_t minios_flags, + VbSelectAndLoadKernelParams *kparams) { - VB2_TRY(VbTryLoadKernelImpl(ctx, VB_DISK_FLAG_FIXED, 1, minios_flags)); + VB2_TRY(VbTryLoadKernelImpl(ctx, VB_DISK_FLAG_FIXED, 1, minios_flags, + kparams)); ctx->flags |= VB2_CONTEXT_DISABLE_TPM; return VB2_SUCCESS; } - -vb2_error_t VbSelectAndLoadKernel(struct vb2_context *ctx, - VbSelectAndLoadKernelParams *kparams) -{ - /* TODO: Send this argument through subsequent function calls, rather - than relying on a global to pass it to VbTryLoadKernel. */ - kparams_ptr = kparams; - - VB2_TRY(vb2api_kernel_phase1(ctx)); - VB2_TRY(vb2api_kernel_phase2(ctx)); - - switch (ctx->boot_mode) { - case VB2_BOOT_MODE_MANUAL_RECOVERY: - /* Manual recovery boot. This has UI. */ - VB2_TRY(vb2ex_manual_recovery_ui(ctx)); - break; - case VB2_BOOT_MODE_BROKEN_SCREEN: - /* - * In EFS2, recovery mode can be entered even when battery is - * drained or damaged. EC-RO sets NO_BOOT flag in such case and - * uses PD power to boot AP. - * - * TODO: Inform user why recovery failed to start. - */ - if (ctx->flags & VB2_CONTEXT_NO_BOOT) - VB2_DEBUG("NO_BOOT in RECOVERY mode\n"); - - /* Broken screen. This has UI. */ - VB2_TRY(vb2ex_broken_screen_ui(ctx)); - break; - case VB2_BOOT_MODE_DIAGNOSTICS: - /* Diagnostic boot. This has UI. */ - VB2_TRY(vb2ex_diagnostic_ui(ctx)); - /* - * The diagnostic menu should either boot a rom, or - * return either of reboot or shutdown. - */ - return VB2_REQUEST_REBOOT; - case VB2_BOOT_MODE_DEVELOPER: - /* Developer boot. This has UI. */ - VB2_TRY(vb2ex_developer_ui(ctx)); - break; - case VB2_BOOT_MODE_NORMAL: - /* Normal boot */ - VB2_TRY(vb2api_normal_boot(ctx)); - break; - default: - return VB2_ERROR_ESCAPE_NO_BOOT; - } - - return vb2api_kernel_finalize(ctx); -} -- cgit v1.2.1