[U-Boot] [PATCH 4/6] gpt: Support for GPT (GUID Partition Table) restoration

Stephen Warren
2012-09-05T20:19:49Z

On 08/24/2012 02:13 AM, Lukasz Majewski wrote:
> The restoration of GPT table (both primary and secondary) is now possible.
> Simple GUID generation is supported.

> +/**
> + * guid_gen(): Generate UUID
> + *
> + * @param dev_desc - block device descriptor
> + *
> + * @return - generated UUID table
> + *
> + * NOTE: The entrophy of this function is small
> + */
> +static u8 *guid_gen(block_dev_desc_t * dev_desc)
> +{
> + int k = 0;
> + static int i = 1;
> + static u8 __aligned(CONFIG_SYS_CACHELINE_SIZE) guid[16];

Hmmm. Wouldn't it be better to take a pointer to the GUID as a parameter
rather than returning a pointer to the same static over and over again.
That way, the caller won't cause problems if they call this function 4
times, and cache the pointer each time.

> + static u8 __aligned(CONFIG_SYS_CACHELINE_SIZE) ent_pool[512];
> + u32 *ptr = (u32 *) guid;
> +
> + /* Entrophy initialization - read random content of one SD sector */
> + if (i == 1) {
> + debug("Init entropy:%x\n", (u32)(dev_desc->lba >> 14));
> +
> + if (dev_desc->block_read(dev_desc->dev, (dev_desc->lba >> 14),
> + 1, (u32 *) ent_pool) != 1) {

I imagine you might get more entropy out of just reading sector 0, or 1
(or both) than some random sector in the middle of the disk, which quite
possibly won't ever have been written to.

Is there any particular reason why ">> 14" rather than any other shift?

> + printf("** Can't read from device %d **\n",
> + dev_desc->dev);
> + }
> + }
> +
> + for (k = 0; k < 4; k++) {
> + *(ptr + k) = efi_crc32((const void *) ent_pool,
> + sizeof(ent_pool));
> + ent_pool[511 - k] = *(ptr + k);
> + }
> +
> + ent_pool[0] = ((u8) i) & 0xff;

That doesn't quite implement a fully compliant UUID. According to:

http://en.wikipedia.org/wiki/Universally_unique_identifier

the "variant" (UUID) and "version" (4; random) should be encoded into a
few specific bits of the final UUID value.

> + debug("GUID: ");
> + for (k = 0; k < sizeof(guid); k++)
> + debug(" %x ", guid[k]);

I think inventing (stealing from Linux) a proper UUID formatting
function would be useful; that way it could be shared by
print_part_efi() if that function was ever enhanced to print the
partition UUID and partition type UUID.

> + debug(" i:%d,\n", i);
> +
> + i++;
> + return guid;
> +}