Prev: drivers/net/wan/farsync.c: Use standard pr_<level>
Next: video: igafb: introduce lost 'return'
From: David Miller on 2 Aug 2010 19:10
From: Will Drewry <wad(a)chromium.org>
Date: Mon, 2 Aug 2010 14:17:03 -0500

> +int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid) {

Please put the openning brace on a new line.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Will Drewry on 2 Aug 2010 22:50
On Mon, Aug 2, 2010 at 6:00 PM, David Miller <davem(a)davemloft.net> wrote:
> From: Will Drewry <wad(a)chromium.org>
> Date: Mon, �2 Aug 2010 14:17:03 -0500
>
>> +int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid) {
>
> Please put the openning brace on a new line.

Thanks! I'll post a new version with that fixed as well as the
alternative approach I mentioned.

cheers!
will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Tejun Heo on 3 Aug 2010 12:10
(cc'ing Kay and quoting whole message for him)

Kay, you were talking about using GUID in GPT for finding out root
device and so on. Does this fit your use case too? If not it would
be nice to find out something which can be shared.

On 08/02/2010 09:17 PM, Will Drewry wrote:
> EFI's GPT partitioning scheme expects that all partitions have a unique
> identifiers. After initial partition scanning, this information is
> completely lost to the rest of the kernel.
>
> efi_partition_by_guid exposes GPT parsing support in a limited fashion
> to allow other portions of the kernel to map a partition from GUID to
> map.
>
> An alternate implementation (and more generic) would be to expose a function
> (efi_partition_walk) that iterates over the partition table providing access to
> each partitions information to a callback, much like device class iterators.
>
> The motivation for this change is the ability to have device mapper targets
> resolve backing devices by GUID instead of specifically by partition number.
> This could also be used in the init boot path (root=GUID) quite simply by
> modeling scanning code on printk_all_partitions(), or with another patchset
> allowing dm="" in the boot path: http://lkml.org/lkml/2010/6/7/418
>
> [ Additional context: http://codereview.chromium.org/3026039/show ]
>
> Would a change like this or something that exposes the GPT information via a
> walking function be something that would be of any interest, or is it explicitly
> against the current design/access goals with respect to partition information?
>
> Any and all feedback is truly appreciated.
>
> Signed-off-by: Will Drewry <wad(a)chromium.org>
> ---
> fs/partitions/efi.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/efi.h | 5 ++++
> 2 files changed, 66 insertions(+), 0 deletions(-)
>
> diff --git a/fs/partitions/efi.c b/fs/partitions/efi.c
> index 9efb2cf..4f642c5 100644
> --- a/fs/partitions/efi.c
> +++ b/fs/partitions/efi.c
> @@ -633,3 +633,64 @@ int efi_partition(struct parsed_partitions *state)
> printk("\n");
> return 1;
> }
> +
> +/**
> + * efi_partition_by_guid
> + * @bdev: Whole block device to scan for a GPT.
> + * @guid: Unique identifier for the partition to find.
> + *
> + * N.b., returns on the first match since it should be unique.
> + *
> + * Returns:
> + * -1 if an error occurred
> + * 0 if there was no match (or not GPT)
> + * >=1 is the index of the partition found.
> + *
> + */
> +int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid) {
> + gpt_header *gpt = NULL;
> + gpt_entry *ptes = NULL;
> + u32 i;
> + struct parsed_partitions *state;
> + int part = 0;
> +
> + if (!bdev || !guid)
> + return -1;
> +
> + state = kzalloc(sizeof(struct parsed_partitions), GFP_KERNEL);
> + if (!state)
> + return -1;
> +
> + state->limit = disk_max_parts(bdev->bd_disk);
> + pr_debug(KERN_WARNING "efi_find_partition looking for gpt\n");
> +
> + state->bdev = bdev;
> + if (!find_valid_gpt(state, &gpt, &ptes) || !gpt || !ptes) {
> + pr_debug(KERN_WARNING "efi_find_partition no GPT\n");
> + kfree(gpt);
> + kfree(ptes);
> + kfree(state);
> + return 0;
> + }
> +
> + pr_debug("GUID Partition Table is valid! Yea!\n");
> + pr_debug(KERN_WARNING "efi_find_partition: 0 -> %d (limit:%d)\n",
> + le32_to_cpu(gpt->num_partition_entries),
> + state->limit);
> + for (i = 0; i < le32_to_cpu(gpt->num_partition_entries) &&
> + i < state->limit-1; i++) {
> + if (!is_pte_valid(&ptes[i], last_lba(bdev)))
> + continue;
> +
> + /* Bails on first hit so duped "unique" GUIDs will be FCFS. */
> + if (!efi_guidcmp(ptes[i].unique_partition_guid,
> + *guid)) {
> + part = i + 1;
> + break;
> + }
> + }
> + kfree(ptes);
> + kfree(gpt);
> + kfree(state);
> + return part;
> +}
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index fb737bc..1a77259 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -301,6 +301,11 @@ extern unsigned long efi_get_time(void);
> extern int efi_set_rtc_mmss(unsigned long nowtime);
> extern struct efi_memory_map memmap;
>
> +#ifdef CONFIG_EFI_PARTITION
> +struct block_device;
> +extern int efi_partition_by_guid(struct block_device *bdev, efi_guid_t *guid);
> +#endif
> +
> /**
> * efi_range_is_wc - check the WC bit on an address range
> * @start: starting kvirt address

--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Kay Sievers on 3 Aug 2010 13:20
On Tue, Aug 3, 2010 at 18:08, Tejun Heo <tj(a)kernel.org> wrote:
> On 08/02/2010 09:17 PM, Will Drewry wrote:
>> EFI's GPT partitioning scheme expects that all partitions have a unique
>> identifiers.  After initial partition scanning, this information is
>> completely lost to the rest of the kernel.
>>
>> efi_partition_by_guid exposes GPT parsing support in a limited fashion
>> to allow other portions of the kernel to map a partition from GUID to
>> map.

> Kay, you were talking about using GUID in GPT for finding out root
> device and so on. Does this fit your use case too? If not it would
> be nice to find out something which can be shared.

Yeah, we have something similar in mind since a while, to be able to
safely boot a box without an initramfs, and to be able to to specify
something like:
root=PARTUUID=6547567-575-7567-567567-57
root=PARTLABEL=foo
on the kernel commandline.

The current 'blkid' already reports stuff like, to have the same
information in userspace:
$ blkid -p -oudev /dev/sde1
ID_FS_LABEL=10GB
ID_FS_LABEL_ENC=10GB
ID_FS_UUID=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
ID_FS_UUID_ENC=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
ID_FS_VERSION=1.0
ID_FS_TYPE=ext4
ID_FS_USAGE=filesystem
ID_PART_ENTRY_SCHEME=gpt
ID_PART_ENTRY_UUID=1f765dcb-5214-bd47-b1c5-f2f18848335e
ID_PART_ENTRY_TYPE=a2a0d0eb-e5b9-3344-87c0-68b6b72699c7
ID_PART_ENTRY_NUMBER=1

I guess we want to store these identifiers directly into the partition
structure, independent of the partition format, so any code can
register a callback for a new block device, and can just check if
that's the device in question. Walking the block devices would just be
something usual provided by the driver core, instead of having some
specific EFI walk functions.

Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Will Drewry on 3 Aug 2010 14:00
On Tue, Aug 3, 2010 at 12:17 PM, Kay Sievers <kay.sievers(a)vrfy.org> wrote:
> On Tue, Aug 3, 2010 at 18:08, Tejun Heo <tj(a)kernel.org> wrote:
>> On 08/02/2010 09:17 PM, Will Drewry wrote:
>>> EFI's GPT partitioning scheme expects that all partitions have a unique
>>> identifiers. �After initial partition scanning, this information is
>>> completely lost to the rest of the kernel.
>>>
>>> efi_partition_by_guid exposes GPT parsing support in a limited fashion
>>> to allow other portions of the kernel to map a partition from GUID to
>>> map.
>
>> Kay, you were talking about using GUID in GPT for finding out root
>> device and so on. �Does this fit your use case too? �If not it would
>> be nice to find out something which can be shared.
>
> Yeah, we have something similar in mind since a while, to be able to
> safely boot a box without an initramfs, and to be able to to specify
> something like:
> �root=PARTUUID=6547567-575-7567-567567-57
> �root=PARTLABEL=foo
> on the kernel commandline.

Cool. So I'd like this as well (at least the UUID part), and I'd like
this to be available for other consumers in the kernel, like
dm_get_device() or at least for mapped device targets to implement
support for themselves. (I have a separate patch for
mimicking md= for device mapper devices which I should probably post
to the lists again soon.)

> The current 'blkid' already reports stuff like, to have the same
> information in userspace:
> �$ blkid -p -oudev /dev/sde1
> �ID_FS_LABEL=10GB
> �ID_FS_LABEL_ENC=10GB
> �ID_FS_UUID=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
> �ID_FS_UUID_ENC=5aafa1bb-70a7-4fe6-b93f-30658ec99fac
> �ID_FS_VERSION=1.0
> �ID_FS_TYPE=ext4
> �ID_FS_USAGE=filesystem
> �ID_PART_ENTRY_SCHEME=gpt
> �ID_PART_ENTRY_UUID=1f765dcb-5214-bd47-b1c5-f2f18848335e
> �ID_PART_ENTRY_TYPE=a2a0d0eb-e5b9-3344-87c0-68b6b72699c7
> �ID_PART_ENTRY_NUMBER=1
>
> I guess we want to store these identifiers directly into the partition
> structure, independent of the partition format, so any code can
> register a callback for a new block device, and can just check if
> that's the device in question. Walking the block devices would just be
> something usual provided by the driver core, instead of having some
> specific EFI walk functions.

Yeah - when I use this function, I end up doing a walk over all the
block devices, checking if they are whole disk entries, then calling
the efi_partition_by_guid() function. (Or the walker which I posted
separately.) It's not ideal but it has the smallest impact on the
existing code. (Not having disk_type available is irritating though.)

Would the type GUID and unique GUID be viable additions to a more
public struct? If they were CONFIG_EFI_PARTITION guarded, then they
wouldn't waste memory for systems without GPT support, but it seems a
bit specific. Also, I don't think it'd make sense to put it in the
partition struct as that represents the on-disk format for some tables
(from a quick scan over the code). However, hd_struct looks the
sanest to me.

I'd be happy to pull together a potential change that exposes this
data once after disk (re)scan, but I'd hate to do so in a way that'd
be fundamentally unacceptable (but I don't want to end up down the
deep hole of adding support across all the part tables either if I can
:).

So I could see something like:

struct hd_struct {
....
#ifdef CONFIG_EFI_PARTITION
efi_guid_t type_guid;
efi_guid_t uuid;
u16 label[72 / ...];
};

Alternatively, a slightly more generic option might be:

#ifdef CONFIG_PARTITION_INFO
/* ASCII hex-formatted uuids inclusive of hyphens */
u8 type_guid[MAX_HD_STRUCT_UUID_SIZE];
u8 uuid[MAX_HD_STRUCT_UUID_SIZE];
u16 label[MAX_HD_STRUCT_NAME + sizeof(u16)];
#endif


Any way, if any of this seems slightly palatable, let me know. I'd
love to make this data accessible to the rest of the kernel.

thanks!
will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
 |  Next  |  Last
Pages: 1 2
Prev: drivers/net/wan/farsync.c: Use standard pr_<level>
Next: video: igafb: introduce lost 'return'