disk: part_efi: range-check partition number

From: Stephen Warren <[email protected]>

Each EFI partition table entry contains a UUID. Extend U-Boot's struct disk_partition to be able to store this information, and modify get_partition_info_efi() to fill it in.

The implementation of uuid_string() was stolen from the Linux kernel.

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

Signed-off-by Stephen Warren <[email protected]>

---
v2: validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is

Note: If Rob Herring's proposed patch "disk/part: introduce get_device_and_partition" is applied, the body of do_partuuid() should be reworked to use Rob's new function get_device_and_partition().

---
 common/Makefile   |    1 +
 common/cmd_part.c |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_part.c

common/Makefile +1 -0

index 3d62775..449b390 100644
@@ -129,6 +129,7 @@ COBJS-$(CONFIG_CMD_NAND) += cmd_nand.o
 COBJS-$(CONFIG_CMD_NET) += cmd_net.o
 COBJS-$(CONFIG_CMD_ONENAND) += cmd_onenand.o
 COBJS-$(CONFIG_CMD_OTP) += cmd_otp.o
+COBJS-$(CONFIG_CMD_PART) += cmd_part.o
 ifdef CONFIG_PCI
 COBJS-$(CONFIG_CMD_PCI) += cmd_pci.o
 endif

common/cmd_part.c new file +104 -0

new file mode 100644
index 0000000..1b15ae9
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2012, NVIDIA CORPORATION.  All rights reserved.
+ *
+ * made from cmd_ext2, which was:
+ *
+ * (C) Copyright 2004
+ * esd gmbh <www.esd-electronics.com>
+ * Reinhard Arlt <[email protected]>
+ *
+ * made from cmd_reiserfs by
+ *
+ * (C) Copyright 2003 - 2004
+ * Sysgo Real-Time Solutions, AG <www.elinos.com>
+ * Pavel Bartusek <[email protected]>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <common.h>
+#include <config.h>
+#include <command.h>
+#include <part.h>
+#include <vsprintf.h>
+
+#ifndef CONFIG_PARTITION_UUIDS
+#error CONFIG_PARTITION_UUIDS must be enabled for CONFIG_CMD_PART to be enabled
+#endif
+
+int do_partuuid(int argc, char * const argv[])
+{
+	int dev;
+	int part;
+	char *ep;
+	block_dev_desc_t *dev_desc;
+	disk_partition_t info;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+	if (argc > 3)
+		return CMD_RET_USAGE;
+
+	dev = (int)simple_strtoul(argv[1], &ep, 16);
+	dev_desc = get_dev(argv[0], dev);
+	if (dev_desc == NULL) {
+		printf("Block device %s %d not supported\n", argv[0], dev);
+		return 1;
+	}
+
+	if (*ep) {
+		if (*ep != ':') {
+			puts("Invalid device; use dev[:part]\n");
+			return 1;
+		}
+		part = (int)simple_strtoul(++ep, NULL, 16);
+	} else {
+		part = 1;
+	}
+
+	if (get_partition_info(dev_desc, part, &info)) {
+		printf("Bad partition %d\n", part);
+		return 1;
+	}
+
+	if (argc > 2)
+		setenv(argv[2], info.uuid);
+	else
+		printf("%s\n", info.uuid);
+
+	return 0;
+}
+
+int do_part(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	if (!strcmp(argv[1], "uuid"))
+		return do_partuuid(argc - 2, argv + 2);
+
+	return CMD_RET_USAGE;
+}
+
+U_BOOT_CMD(
+	part,	5,	1,	do_part,
+	"disk partition related commands",
+	"part uuid <interface> <dev[:part]>\n"
+	"    - print partition UUID\n"
+	"part uuid <interface> <dev[:part]> <varname>\n"
+	"    - set environment variable to partition UUID"
+);

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

Quoted 8 lines

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

Quoted 3 lines

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

A list command would be useful and would be better located here than under scsi or other interface commands. Perhaps instead of printing a single part uuid, you should make a list command that prints all partitions and their UUIDs. That would address my first question.

Quoted 7 lines

Signed-off-by Stephen Warren <[email protected]>

---
v2: validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is

Note: If Rob Herring's proposed patch "disk/part: introduce get_device_and_partition" is applied, the body of do_partuuid() should be reworked to use Rob's new function get_device_and_partition().

Quoted 27 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

Quoted 11 lines

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

Quoted 4 lines

A list command would be useful and would be better located here than under scsi or other interface commands. Perhaps instead of printing a single part uuid, you should make a list command that prints all partitions and their UUIDs. That would address my first question.

Sounds like a good idea to me as well.

Quoted 5 lines

Quoted 5 lines

v2: validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is

Note: If Rob Herring's proposed patch "disk/part: introduce get_device_and_partition" is applied, the body of do_partuuid() should be reworked to use Rob's new function get_device_and_partition().

I think the best idea here would be to make the next version just depend on Rob's series.

Quoted 23 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

Quoted 8 lines

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

Quoted 8 lines

Quoted 3 lines

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

A list command would be useful and would be better located here than under scsi or other interface commands. Perhaps instead of printing a single part uuid, you should make a list command that prints all partitions and their UUIDs. That would address my first question.

Sounds like a good idea to me as well.

[snip]

Quoted 4 lines

Quoted 4 lines

Signed-off-by Stephen Warren <[email protected]>

---
v2: validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is

Quoted 3 lines

Quoted 3 lines

Note: If Rob Herring's proposed patch "disk/part: introduce get_device_and_partition" is applied, the body of do_partuuid() should be reworked to use Rob's new function get_device_and_partition().

I think the best idea here would be to make the next version just depend on Rob's series.

Rob's series depends on Wolfgang(?)'s u-boot/ext4 branch at present. I'm not sure what the status of that branch is right now - is it something that's ready to be submitted, or is more work there needed, so the branch won't be pulled into u-boot/master in the near future? I'm mainly asking so Rob and I know if Rob's patches should be rebased first onto something else, before I rebase my patches on his.

Thanks.

Hi Stephen, Tom,

Quoted 7 lines

Rob's series depends on Wolfgang(?)'s u-boot/ext4 branch at present. I'm not sure what the status of that branch is right now - is it something that's ready to be submitted, or is more work there needed, so the branch won't be pulled into u-boot/master in the near future? I'm mainly asking so Rob and I know if Rob's patches should be rebased first onto something else, before I rebase my patches on his.

Quoted 1 line

Thanks.

I'd also like to know if those patches will be accepted soon. I'm working on a GPT restoration support and those patches might be a pre-requisite for my development (if were pulled into u-boot/master).

Quoted 40 lines · Tom Rini

Quoted 23 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

Quoted 8 lines

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

Quoted 8 lines

Quoted 3 lines

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

A list command would be useful and would be better located here than under scsi or other interface commands. Perhaps instead of printing a single part uuid, you should make a list command that prints all partitions and their UUIDs. That would address my first question.

Sounds like a good idea to me as well.

[snip]

Quoted 4 lines

Quoted 4 lines

Signed-off-by Stephen Warren <[email protected]>

---
v2: validate that CONFIG_PARTITION_UUID is defined when CONFIG_CMD_PART is

Quoted 6 lines

Quoted 3 lines

Quoted 3 lines

Note: If Rob Herring's proposed patch "disk/part: introduce get_device_and_partition" is applied, the body of do_partuuid() should be reworked to use Rob's new function get_device_and_partition().

I think the best idea here would be to make the next version just depend on Rob's series.

Tom,

Rob's series depends on Wolfgang(?)'s u-boot/ext4 branch at present. I'm not sure what the status of that branch is right now - is it something that's ready to be submitted, or is more work there needed, so the branch won't be pulled into u-boot/master in the near future? I'm mainly asking so Rob and I know if Rob's patches should be rebased first onto something else, before I rebase my patches on his.

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

Quoted 7 lines

This can be useful when writing a bootcmd which searches all known devices for something bootable, and then wants the kernel to use the same partition as the root device, e.g.:

part uuid ${devtype} ${devnum}:${rootpart} uuid
setenv bootargs root=PARTUUID=${uuid} ...

Quoted 3 lines

It is expected that further part sub-commands will be added later, e.g. to find which partition on a disk is marked bootable, to write new partition tables to disk, etc.

A list command would be useful and would be better located here than under scsi or other interface commands. Perhaps instead of printing a single part uuid, you should make a list command that prints all partitions and their UUIDs. That would address my first question.

Quoted 10 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

Quoted 4 lines

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

The idea is that if you're running the command interactively, you won't pass a variable name on the command-line, so the command will print out the UUID for you to read. In this case, it's pointless to set any environment variable.

However, if you're writing a script, you want to capture the UUID into an environment variable, and it's quite unlikely you want to litter stdout with that content too. Hence, either-or, not both.

Quoted 21 lines · Stephen Warren

Quoted 10 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

Quoted 4 lines

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

The idea is that if you're running the command interactively, you won't pass a variable name on the command-line, so the command will print out the UUID for you to read. In this case, it's pointless to set any environment variable.

Quoted 3 lines

However, if you're writing a script, you want to capture the UUID into an environment variable, and it's quite unlikely you want to litter stdout with that content too. Hence, either-or, not both.

Do other commands have a "I'm being scripted, probably, don't stdout" and "I'm being interactive, use stdout" distinction like this? IMHO, always printing out makes sense so you can "see" that your script is working as you expect.

Quoted 22 lines · Tom Rini

Quoted 21 lines · Stephen Warren

Quoted 10 lines · Rob Herring

Quoted 8 lines · Stephen Warren

From: Stephen Warren <[email protected]>

This implements the following:

part uuid mmc 0:1
-> print partition UUID
part uuid mmc 0:1 uuid
-> set environment variable to partition UUID

Quoted 4 lines

What's the reason to not always both print out and set the uuid env var?

Perhaps the env name should be partuuid or part_uuid as you could have uuid's for other purposes?

The idea is that if you're running the command interactively, you won't pass a variable name on the command-line, so the command will print out the UUID for you to read. In this case, it's pointless to set any environment variable.

Quoted 8 lines

Quoted 3 lines

However, if you're writing a script, you want to capture the UUID into an environment variable, and it's quite unlikely you want to litter stdout with that content too. Hence, either-or, not both.

Do other commands have a "I'm being scripted, probably, don't stdout" and "I'm being interactive, use stdout" distinction like this? IMHO, always printing out makes sense so you can "see" that your script is working as you expect.

In general, as a script writer, yes you do have the ability to choose. Typically, I'd write:

part uuid ....

vs.

var=part uuid ....

in order to control this. However, U-Boot's shell doesn't support backticks. As a script writer, I certainly desire the ability to control what commands spam to the console, and really don't think it's useful to print the UUID from a script (does the user really care, and any script developer can just echo it for debugging if they need it).

I'm not aware of other U-Boot commands whose purpose it is to set environment variables, so can't really compare.

Still, if you're insistent on this point, I can change the code to always print, and optionally write an environment variable.