vboot: Plumb the two disk sizes and external GPT param through

This patch reinstates the external GPT support which was previously
committed and reverted. Improvements since last time include:
- Cleaned-up internal interface based on code review
- Function correctly on legacy bootloaders (e.g., depthcharge before
  NAND-related patches are added)
- Better comments
- Treat new field values = 0 -> not use new feature
- Tests are added to ensure external GPT flag is passed down properly
The original commit had change-id
I5a77e417aea8ee9442d18c200d1b073aa5375ecf
Its commit message is reproduced below, and then an additional test.
----
To support an external GPT, disks have two new attributes:
- A binary flag indicating whether the GPT is in the same address
  space as the payloads or a separate one.
- The number of sectors of the streaming portion of storage, as
  opposed to the portion containing the GPT.
These have been added elsewhere to GptData (in cgptlib) and BlockDev
(in depthcharge). This patch adds the plumbing between those, including
in the DiskInfo interface between the firmware and vboot.
BUG=chromium:425677
BRANCH=none
TEST=Interactively wrote the GPT with cgpt and observed the following
boot with depthcharge to read the GPT from SPI and then read from
the proper locations in NAND flash.
TEST=make runalltests passes.
TEST=boots from USB with depthcharge from HEAD.

Change-Id: Ia7956517a7b9da0301f01fac5a10204f6d78cf4f
Signed-off-by: Dan Ehrenberg <dehrenberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/234640
Reviewed-by: Bill Richardson <wfrichar@chromium.org>

firmware/include/vboot_api.h

@@ -555,14 +555,38 @@ 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 LBA sector in bytes */
+	/* Size of a random-access LBA sector in bytes */
 	uint64_t bytes_per_lba;
-	/* Number of LBA sectors on the device */
+	/* Number of random-access LBA sectors on the device.
+	 * If streaming_lba_count is 0, this stands in for the size of the
+	 * randomly accessed portion as well as the streaming portion.
+	 * Otherwise, this is only the randomly-accessed portion. */
 	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;
 	/*
@@ -604,6 +628,9 @@ 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 is not for the partition
+ * contents. The upper limit is lba_count.
+ *
  * 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.
@@ -615,6 +642,9 @@ 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.
@@ -635,10 +665,9 @@ typedef void *VbExStream_t;
  *
  * @return Error code, or VBERROR_SUCCESS.
  *
- * lba_start and lba_count are subject to disk type-dependent alignment
+ * This is used for access to the contents of the actual partitions on the
- * restrictions. An invalid value will lead to an error code. In particular,
+ * device. It is not used to access the GPT. The size of the content addressed
- * on raw NAND devices, lba_start and lba_count must be page-aligned after
+ * is within streaming_lba_count.
- * subtracting the offset of the GPT.
  */
 VbError_t VbExStreamOpen(VbExDiskHandle_t handle, uint64_t lba_start,
 			 uint64_t lba_count, VbExStream_t *stream_ptr);
@@ -653,9 +682,8 @@ 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
+ * This is used for access to the contents of the actual partitions on the
- * value will lead to an error code. In particular, on raw NAND devices, bytes
+ * device. It is not used to access the GPT.
- * must be a page multiple.
  */
 VbError_t VbExStreamRead(VbExStream_t stream, uint32_t bytes, void *buffer);
 

firmware/lib/include/load_kernel_fw.h

@@ -16,9 +16,11 @@
 
 /* Boot flags for LoadKernel().boot_flags */
 /* Developer switch is on */
-#define BOOT_FLAG_DEVELOPER (0x01ULL)
+#define BOOT_FLAG_DEVELOPER    (0x01ULL)
 /* In recovery mode */
-#define BOOT_FLAG_RECOVERY  (0x02ULL)
+#define BOOT_FLAG_RECOVERY     (0x02ULL)
+/* GPT is external */
+#define BOOT_FLAG_EXTERNAL_GPT (0x04ULL)
 
 typedef struct LoadKernelParams {
 	/* Inputs to LoadKernel() */
@@ -40,8 +42,10 @@ typedef struct LoadKernelParams {
 	VbExDiskHandle_t disk_handle;
 	/* Bytes per lba sector on current device */
 	uint64_t bytes_per_lba;
-	/* Last addressable lba sector on current device */
+	/* Number of LBA-addressable sectors on the main device */
-	uint64_t ending_lba;
+	uint64_t streaming_lba_count;
+	/* Random-access GPT size */
+	uint64_t gpt_lba_count;
 	/* Destination buffer for kernel (normally at 0x100000) */
 	void *kernel_buffer;
 	/* Size of kernel buffer in bytes */

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) {
+		    get_info_flags != (disk_info[i].flags & ~VB_DISK_FLAG_EXTERNAL_GPT)) {
 			VBDEBUG(("  skipping: bytes_per_lba=%" PRIu64
 				 " lba_count=%" PRIu64 " flags=0x%x\n",
 				 disk_info[i].bytes_per_lba,
@@ -98,7 +98,11 @@ 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].lba_count - 1;
+		p->gpt_lba_count = disk_info[i].lba_count;
+		p->streaming_lba_count = disk_info[i].streaming_lba_count
+						?: p->gpt_lba_count;
+		p->boot_flags |= disk_info[i].flags & VB_DISK_FLAG_EXTERNAL_GPT
+				? BOOT_FLAG_EXTERNAL_GPT : 0;
 		retval = LoadKernel(p, cparams);
 		VBDEBUG(("VbTryLoadKernel() LoadKernel() = %d\n", retval));
 

firmware/lib/vboot_kernel.c

@@ -57,7 +57,7 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams)
 
 	/* Sanity Checks */
 	if (!params->bytes_per_lba ||
-	    !params->ending_lba) {
+	    !params->streaming_lba_count) {
 		VBDEBUG(("LoadKernel() called with invalid params\n"));
 		retval = VBERROR_INVALID_PARAMETER;
 		goto LoadKernelExit;
@@ -90,7 +90,7 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams)
 	shcall->boot_flags = (uint32_t)params->boot_flags;
 	shcall->boot_mode = boot_mode;
 	shcall->sector_size = (uint32_t)params->bytes_per_lba;
-	shcall->sector_count = params->ending_lba + 1;
+	shcall->sector_count = params->streaming_lba_count;
 	shared->lk_call_count++;
 
 	/* Initialization */
@@ -115,10 +115,10 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams)
 
 	/* Read GPT data */
 	gpt.sector_bytes = (uint32_t)blba;
-	gpt.streaming_drive_sectors = params->ending_lba + 1;
+	gpt.streaming_drive_sectors = params->streaming_lba_count;
-	/* TODO: Set stored_on_device and gpt_drive_sectors appropriately */
+	gpt.gpt_drive_sectors = params->gpt_lba_count;
-	gpt.gpt_drive_sectors = gpt.streaming_drive_sectors;
+	gpt.flags = params->boot_flags & BOOT_FLAG_EXTERNAL_GPT
-	gpt.flags = 0;
+			? GPT_FLAG_EXTERNAL : 0;
 	if (0 != AllocAndReadGptData(params->disk_handle, &gpt)) {
 		VBDEBUG(("Unable to read GPT data\n"));
 		shcall->check_result = VBSD_LKC_CHECK_GPT_READ_ERROR;

futility/cmd_verify_kernel.c

@@ -30,9 +30,9 @@ VbError_t VbExDiskRead(VbExDiskHandle_t handle, uint64_t lba_start,
 {
 	if (handle != (VbExDiskHandle_t)1)
 		return VBERROR_UNKNOWN;
-	if (lba_start > params.ending_lba)
+	if (lba_start >= params.streaming_lba_count)
 		return VBERROR_UNKNOWN;
-	if (lba_start + lba_count > params.ending_lba + 1)
+	if (lba_start + lba_count > params.streaming_lba_count)
 		return VBERROR_UNKNOWN;
 
 	memcpy(buffer, diskbuf + lba_start * 512, lba_count * 512);
@@ -44,9 +44,9 @@ VbError_t VbExDiskWrite(VbExDiskHandle_t handle, uint64_t lba_start,
 {
 	if (handle != (VbExDiskHandle_t)1)
 		return VBERROR_UNKNOWN;
-	if (lba_start > params.ending_lba)
+	if (lba_start >= params.streaming_lba_count)
 		return VBERROR_UNKNOWN;
-	if (lba_start + lba_count > params.ending_lba + 1)
+	if (lba_start + lba_count > params.streaming_lba_count)
 		return VBERROR_UNKNOWN;
 
 	memcpy(diskbuf + lba_start * 512, buffer, lba_count * 512);
@@ -95,7 +95,8 @@ static int do_verify_kernel(int argc, char *argv[])
 	params.shared_data_size = sizeof(shared_data);
 	params.disk_handle = (VbExDiskHandle_t)1;
 	params.bytes_per_lba = 512;
-	params.ending_lba = disk_bytes / 512 - 1;
+	params.streaming_lba_count = disk_bytes / 512;
+	params.gpt_lba_count = params.streaming_lba_count;
 
 	params.kernel_buffer_size = 16 * 1024 * 1024;
 	params.kernel_buffer = malloc(params.kernel_buffer_size);
@@ -108,7 +109,7 @@ static int do_verify_kernel(int argc, char *argv[])
 	params.gbb_data = NULL;
 	params.gbb_size = 0;
 
-	/* TODO: optional dev-mode flag */
+	/* TODO(chromium:441893): support dev-mode flag and external gpt flag */
 	params.boot_flags = 0;
 
 	/*

tests/vboot_api_kernel_tests.c

@@ -37,6 +37,7 @@ typedef struct {
 	disk_desc_t disks_to_provide[MAX_TEST_DISKS];
 	int disk_count_to_return;
 	VbError_t loadkernel_return_val[MAX_TEST_DISKS];
+	uint8_t external_expected[MAX_TEST_DISKS];
 
 	/* outputs from test */
 	uint32_t expected_recovery_request_val;
@@ -67,13 +68,16 @@ test_case_t test[] = {
 			{512,  100,  0, 0},
 			/* still wrong flags */
 			{512,  100,  -1, 0},
-			{512,  100,  VB_DISK_FLAG_REMOVABLE, pickme},
+			{512,  100,
+			 VB_DISK_FLAG_REMOVABLE | VB_DISK_FLAG_EXTERNAL_GPT,
+			 pickme},
 			/* already got one */
 			{512,  100,  VB_DISK_FLAG_REMOVABLE, "holygrail"},
 		},
 		.disk_count_to_return = DEFAULT_COUNT,
 		.diskgetinfo_return_val = VBERROR_SUCCESS,
 		.loadkernel_return_val = {0, 1, 1, 1, 1, 1, 1, 1, 1, 1,},
+		.external_expected = {1, 0, 0, 0, 0, 0, 0, 0, 0, 0,},
 
 		.expected_recovery_request_val = VBNV_RECOVERY_NOT_REQUESTED,
 		.expected_to_find_disk = pickme,
@@ -182,6 +186,7 @@ static uint32_t got_recovery_request_val;
 static const char *got_find_disk;
 static const char *got_load_disk;
 static uint32_t got_return_val;
+static uint32_t got_external_mismatch;
 
 /**
  * Reset mock data (for use before each test)
@@ -229,6 +234,7 @@ 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;
@@ -275,6 +281,9 @@ VbError_t LoadKernel(LoadKernelParams *params, VbCommonParams *cparams)
 	VBDEBUG(("%s(%d): got_find_disk = %s\n", __FUNCTION__,
 		 load_kernel_calls,
 		 got_find_disk ? got_find_disk : "0"));
+	if (t->external_expected[load_kernel_calls] !=
+			!!(params->boot_flags & BOOT_FLAG_EXTERNAL_GPT))
+		got_external_mismatch++;
 	return t->loadkernel_return_val[load_kernel_calls++];
 }
 
@@ -306,6 +315,7 @@ static void VbTryLoadKernelTest(void)
 			TEST_PTR_EQ(got_load_disk, t->expected_to_load_disk,
 				    "  load disk");
 		}
+		TEST_EQ(got_external_mismatch, 0, "  external GPT errors");
 	}
 }
 

tests/vboot_kernel_tests.c

@@ -50,6 +50,7 @@ static int preamble_verify_fail;
 static int verify_data_fail;
 static RSAPublicKey *mock_data_key;
 static int mock_data_key_allocated;
+static int gpt_flag_external;
 
 static uint8_t gbb_data[sizeof(GoogleBinaryBlockHeader) + 2048];
 static GoogleBinaryBlockHeader *gbb = (GoogleBinaryBlockHeader*)gbb_data;
@@ -126,6 +127,8 @@ static void ResetMocks(void)
 	mock_data_key = (RSAPublicKey *)"TestDataKey";
 	mock_data_key_allocated = 0;
 
+	gpt_flag_external = 0;
+
 	memset(gbb, 0, sizeof(*gbb));
 	gbb->major_version = GBB_MAJOR_VER;
 	gbb->minor_version = GBB_MINOR_VER;
@@ -150,7 +153,8 @@ static void ResetMocks(void)
 	lkp.gbb_data = gbb;
 	lkp.gbb_size = sizeof(gbb_data);
 	lkp.bytes_per_lba = 512;
-	lkp.ending_lba = 1023;
+	lkp.streaming_lba_count = 1024;
+	lkp.gpt_lba_count = 1024;
 	lkp.kernel_buffer = kernel_buffer;
 	lkp.kernel_buffer_size = sizeof(kernel_buffer);
 	lkp.disk_handle = (VbExDiskHandle_t)1;
@@ -215,6 +219,9 @@ int GptNextKernelEntry(GptData *gpt, uint64_t *start_sector, uint64_t *size)
 	if (!p->size)
 		return GPT_ERROR_NO_VALID_KERNEL;
 
+	if (gpt->flags & GPT_FLAG_EXTERNAL)
+		gpt_flag_external++;
+
 	gpt->current_kernel = mock_part_next;
 	*start_sector = p->start;
 	*size = p->size;
@@ -522,7 +529,7 @@ static void InvalidParamsTest(void)
 		"Bad lba size");
 
 	ResetMocks();
-	lkp.ending_lba = 0;
+	lkp.streaming_lba_count = 0;
 	TEST_EQ(LoadKernel(&lkp, &cparams), VBERROR_INVALID_PARAMETER,
 		"Bad lba count");
 
@@ -541,6 +548,11 @@ 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;
@@ -560,6 +572,7 @@ static void LoadKernelTest(void)
 	TEST_EQ(lkp.bootloader_address, 0xbeadd008, "  bootloader addr");
 	TEST_EQ(lkp.bootloader_size, 0x1234, "  bootloader size");
 	TEST_STR_EQ((char *)lkp.partition_guid, "FakeGuid", "  guid");
+	TEST_EQ(gpt_flag_external, 0, "GPT was internal");
 	VbNvGet(&vnc, VBNV_RECOVERY_REQUEST, &u);
 	TEST_EQ(u, 0, "  recovery request");
 
@@ -742,6 +755,12 @@ static void LoadKernelTest(void)
 	ResetMocks();
 	verify_data_fail = 1;
 	TEST_EQ(LoadKernel(&lkp, &cparams), VBERROR_INVALID_KERNEL_FOUND,	"Bad data");
+
+	/* Check that EXTERNAL_GPT flag makes it down */
+	ResetMocks();
+	lkp.boot_flags |= BOOT_FLAG_EXTERNAL_GPT;
+	TEST_EQ(LoadKernel(&lkp, &cparams), 0, "Succeed external GPT");
+	TEST_EQ(gpt_flag_external, 1, "GPT was external");
 }
 
 int main(void)

utility/load_kernel_test.c

@@ -37,10 +37,10 @@ VbError_t VbExDiskRead(VbExDiskHandle_t handle, uint64_t lba_start,
                        uint64_t lba_count, void *buffer) {
   printf("Read(%" PRIu64 ", %" PRIu64 ")\n", lba_start, lba_count);
 
-  if (lba_start > lkp.ending_lba ||
+  if (lba_start >= lkp.streaming_lba_count ||
-      lba_start + lba_count - 1 > lkp.ending_lba) {
+      lba_start + lba_count > lkp.streaming_lba_count) {
     fprintf(stderr, "Read overrun: %" PRIu64 " + %" PRIu64 " > %" PRIu64 "\n",
-            lba_start, lba_count, lkp.ending_lba);
+            lba_start, lba_count, lkp.streaming_lba_count);
     return 1;
   }
 
@@ -57,10 +57,10 @@ VbError_t VbExDiskWrite(VbExDiskHandle_t handle, uint64_t lba_start,
                         uint64_t lba_count, const void *buffer) {
   printf("Write(%" PRIu64 ", %" PRIu64 ")\n", lba_start, lba_count);
 
-  if (lba_start > lkp.ending_lba ||
+  if (lba_start >= lkp.streaming_lba_count ||
-      lba_start + lba_count - 1 > lkp.ending_lba) {
+      lba_start + lba_count > lkp.streaming_lba_count) {
     fprintf(stderr, "Read overrun: %" PRIu64 " + %" PRIu64 " > %" PRIu64 "\n",
-            lba_start, lba_count, lkp.ending_lba);
+            lba_start, lba_count, lkp.streaming_lba_count);
     return 1;
   }
 
@@ -204,9 +204,10 @@ int main(int argc, char* argv[]) {
     return 1;
   }
   fseek(image_file, 0, SEEK_END);
-  lkp.ending_lba = (ftell(image_file) / LBA_BYTES) - 1;
+  lkp.streaming_lba_count = (ftell(image_file) / LBA_BYTES);
+  lkp.gpt_lba_count = lkp.streaming_lba_count;
   rewind(image_file);
-  printf("Ending LBA: %" PRIu64 "\n", lkp.ending_lba);
+  printf("Streaming LBA count: %" PRIu64 "\n", lkp.streaming_lba_count);
 
   /* Allocate a buffer for the kernel */
   lkp.kernel_buffer = malloc(KERNEL_BUFFER_SIZE);