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;
> +}

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to