From 79a9e0e63fd1001a3f9615f96c09acba5f20250d Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Sat, 15 Nov 2014 03:02:29 +0000 Subject: Revert "vboot: Plumb the two disk sizes and 'gpt on device' param through" This reverts commit 5040a945dfd0dd305d3ca8e923b8bf0bd5c6528e. This patch breaks booting any image (both fixed and removable) on Veyron_Pinky (and presumably every other non-NAND board?). By the power vested in me through the office of ChromeOS tree sheriff (well, five hours early but whatever) it is hereby reverted! BUG=chromium:425677 BRANCH=none TEST=Can successfully boot on Veyron_Pinky again. Change-Id: I9323a3d5e34491337fc7eb09dd00d845ac42997d Reviewed-on: https://chromium-review.googlesource.com/229963 Reviewed-by: Julius Werner Commit-Queue: Julius Werner Tested-by: Julius Werner --- firmware/include/vboot_api.h | 42 +++++++++-------------------------- firmware/lib/include/load_kernel_fw.h | 4 ---- firmware/lib/vboot_api_kernel.c | 6 ++--- firmware/lib/vboot_kernel.c | 6 ++--- futility/cmd_verify_kernel.c | 2 -- tests/vboot_api_kernel_tests.c | 1 - tests/vboot_kernel_tests.c | 6 ----- utility/load_kernel_test.c | 1 - 8 files changed, 15 insertions(+), 53 deletions(-) diff --git a/firmware/include/vboot_api.h b/firmware/include/vboot_api.h index 853a34ad..526c1e4a 100644 --- a/firmware/include/vboot_api.h +++ b/firmware/include/vboot_api.h @@ -553,35 +553,14 @@ VbError_t VbExHashFirmwareBody(VbCommonParams *cparams, * when processing read-only recovery image. */ -/* - * Disks are used in two ways: - * - As a random-access device to read and write the GPT - * - As a streaming device to read the kernel - * These are implemented differently on raw NAND vs eMMC/SATA/USB - * - On eMMC/SATA/USB, both of these refer to the same underlying - * storage, so they have the same size and LBA size. In this case, - * the GPT should not point to the same address as itself. - * - On raw NAND, the GPT is held on a portion of the SPI flash. - * Random access GPT operations refer to the SPI and streaming - * operations refer to NAND. The GPT may therefore point into - * the same offsets as itself. - * These types are distinguished by the following flag and VbDiskInfo - * has separate fields to describe the random-access ("GPT") and - * streaming aspects of the disk. If a disk is random-access (i.e. - * not raw NAND) then these fields are equal. - */ -#define VB_DISK_FLAG_EXTERNAL_GPT 0x00000004 - /* Information on a single disk */ typedef struct VbDiskInfo { /* Disk handle */ VbExDiskHandle_t handle; - /* Size of a random-access LBA sector in bytes */ + /* Size of a LBA sector in bytes */ uint64_t bytes_per_lba; - /* Number of random-access LBA sectors on the device */ + /* Number of LBA sectors on the device */ uint64_t lba_count; - /* Number of streaming sectors on the device */ - uint64_t streaming_lba_count; /* Flags (see VB_DISK_FLAG_* constants) */ uint32_t flags; /* @@ -623,9 +602,6 @@ VbError_t VbExDiskFreeInfo(VbDiskInfo *infos, * Read lba_count LBA sectors, starting at sector lba_start, from the disk, * into the buffer. * - * This is used for random access to the GPT. It does not (necessarily) access - * the streaming portion of the device. - * * If the disk handle is invalid (for example, the handle refers to a disk * which as been removed), the function must return error but must not * crash. @@ -637,9 +613,6 @@ VbError_t VbExDiskRead(VbExDiskHandle_t handle, uint64_t lba_start, * Write lba_count LBA sectors, starting at sector lba_start, to the disk, from * the buffer. * - * This is used for random access to the GPT. It does not (necessarily) access - * the streaming portion of the device. - * * If the disk handle is invalid (for example, the handle refers to a disk * which as been removed), the function must return error but must not * crash. @@ -660,9 +633,10 @@ typedef void *VbExStream_t; * * @return Error code, or VBERROR_SUCCESS. * - * This is used for access to the streaming portion of the device, and does - * not (necessarily) access the GPT. The size of the content addressed is within - * streaming_lba_count. + * lba_start and lba_count are subject to disk type-dependent alignment + * restrictions. An invalid value will lead to an error code. In particular, + * on raw NAND devices, lba_start and lba_count must be page-aligned after + * subtracting the offset of the GPT. */ VbError_t VbExStreamOpen(VbExDiskHandle_t handle, uint64_t lba_start, uint64_t lba_count, VbExStream_t *stream_ptr); @@ -676,6 +650,10 @@ VbError_t VbExStreamOpen(VbExDiskHandle_t handle, uint64_t lba_start, * * @return Error code, or VBERROR_SUCCESS. Failure to read as much data as * requested is an error. + * + * bytes is subject to disk type-dependent alignment restrictions. An invalid + * value will lead to an error code. In particular, on raw NAND devices, bytes + * must be a page multiple. */ VbError_t VbExStreamRead(VbExStream_t stream, uint32_t bytes, void *buffer); diff --git a/firmware/lib/include/load_kernel_fw.h b/firmware/lib/include/load_kernel_fw.h index bd816d65..a710ee5d 100644 --- a/firmware/lib/include/load_kernel_fw.h +++ b/firmware/lib/include/load_kernel_fw.h @@ -42,10 +42,6 @@ typedef struct LoadKernelParams { uint64_t bytes_per_lba; /* Last addressable lba sector on current device */ uint64_t ending_lba; - /* Random-access GPT size */ - uint64_t gpt_lba_count; - /* External GPT */ - uint8_t external_gpt; /* 1 = external, 0 = internal */ /* Destination buffer for kernel (normally at 0x100000) */ void *kernel_buffer; /* Size of kernel buffer in bytes */ diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c index 942dbcc2..0609b55b 100644 --- a/firmware/lib/vboot_api_kernel.c +++ b/firmware/lib/vboot_api_kernel.c @@ -88,7 +88,7 @@ uint32_t VbTryLoadKernel(VbCommonParams *cparams, LoadKernelParams *p, */ if (512 != disk_info[i].bytes_per_lba || 32 > disk_info[i].lba_count || - get_info_flags != (disk_info[i].flags & ~VB_DISK_FLAG_EXTERNAL_GPT)) { + get_info_flags != disk_info[i].flags) { VBDEBUG((" skipping: bytes_per_lba=%" PRIu64 " lba_count=%" PRIu64 " flags=0x%x\n", disk_info[i].bytes_per_lba, @@ -98,9 +98,7 @@ uint32_t VbTryLoadKernel(VbCommonParams *cparams, LoadKernelParams *p, } p->disk_handle = disk_info[i].handle; p->bytes_per_lba = disk_info[i].bytes_per_lba; - p->ending_lba = disk_info[i].streaming_lba_count - 1; - p->gpt_lba_count = disk_info[i].lba_count; - p->external_gpt = disk_info[i].flags & VB_DISK_FLAG_EXTERNAL_GPT; + p->ending_lba = disk_info[i].lba_count - 1; retval = LoadKernel(p, cparams); VBDEBUG(("VbTryLoadKernel() LoadKernel() = %d\n", retval)); diff --git a/firmware/lib/vboot_kernel.c b/firmware/lib/vboot_kernel.c index 740e1f23..62e62967 100644 --- a/firmware/lib/vboot_kernel.c +++ b/firmware/lib/vboot_kernel.c @@ -116,9 +116,9 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams) /* Read GPT data */ gpt.sector_bytes = (uint32_t)blba; gpt.drive_sectors = params->ending_lba + 1; - gpt.gpt_drive_sectors = params->gpt_lba_count; - gpt.stored_on_device = params->external_gpt ? GPT_STORED_OFF_DEVICE - : GPT_STORED_ON_DEVICE; + /* TODO: Set stored_on_device and gpt_drive_sectors appropriately */ + gpt.stored_on_device = GPT_STORED_ON_DEVICE; + gpt.gpt_drive_sectors = gpt.drive_sectors; if (0 != AllocAndReadGptData(params->disk_handle, &gpt)) { VBDEBUG(("Unable to read GPT data\n")); shcall->check_result = VBSD_LKC_CHECK_GPT_READ_ERROR; diff --git a/futility/cmd_verify_kernel.c b/futility/cmd_verify_kernel.c index 0240ec33..f5ee2e00 100644 --- a/futility/cmd_verify_kernel.c +++ b/futility/cmd_verify_kernel.c @@ -96,8 +96,6 @@ static int do_verify_kernel(int argc, char *argv[]) params.disk_handle = (VbExDiskHandle_t)1; params.bytes_per_lba = 512; params.ending_lba = disk_bytes / 512 - 1; - params.gpt_lba_count = params.ending_lba + 1; - params.external_gpt = 0; params.kernel_buffer_size = 16 * 1024 * 1024; params.kernel_buffer = malloc(params.kernel_buffer_size); diff --git a/tests/vboot_api_kernel_tests.c b/tests/vboot_api_kernel_tests.c index e31cf8bd..bccad0de 100644 --- a/tests/vboot_api_kernel_tests.c +++ b/tests/vboot_api_kernel_tests.c @@ -229,7 +229,6 @@ VbError_t VbExDiskGetInfo(VbDiskInfo **infos_ptr, uint32_t *count, mock_disks[num_disks].bytes_per_lba = t->disks_to_provide[i].bytes_per_lba; mock_disks[num_disks].lba_count = - mock_disks[num_disks].streaming_lba_count = t->disks_to_provide[i].lba_count; mock_disks[num_disks].flags = t->disks_to_provide[i].flags; diff --git a/tests/vboot_kernel_tests.c b/tests/vboot_kernel_tests.c index 4866a6d3..c546ed12 100644 --- a/tests/vboot_kernel_tests.c +++ b/tests/vboot_kernel_tests.c @@ -151,7 +151,6 @@ static void ResetMocks(void) lkp.gbb_size = sizeof(gbb_data); lkp.bytes_per_lba = 512; lkp.ending_lba = 1023; - lkp.gpt_lba_count = 1024; lkp.kernel_buffer = kernel_buffer; lkp.kernel_buffer_size = sizeof(kernel_buffer); lkp.disk_handle = (VbExDiskHandle_t)1; @@ -542,11 +541,6 @@ static void InvalidParamsTest(void) TEST_EQ(LoadKernel(&lkp, &cparams), VBERROR_NO_KERNEL_FOUND, "Bad GPT"); - ResetMocks(); - lkp.gpt_lba_count = 0; - TEST_EQ(LoadKernel(&lkp, &cparams), VBERROR_NO_KERNEL_FOUND, - "GPT size = 0"); - /* This causes the stream open call to fail */ ResetMocks(); lkp.disk_handle = NULL; diff --git a/utility/load_kernel_test.c b/utility/load_kernel_test.c index 63dfc711..8e6c5191 100644 --- a/utility/load_kernel_test.c +++ b/utility/load_kernel_test.c @@ -205,7 +205,6 @@ int main(int argc, char* argv[]) { } fseek(image_file, 0, SEEK_END); lkp.ending_lba = (ftell(image_file) / LBA_BYTES) - 1; - lkp.gpt_lba_count = lkp.ending_lba + 1; rewind(image_file); printf("Ending LBA: %" PRIu64 "\n", lkp.ending_lba); -- cgit v1.2.1