[PATCH 0/2] Fix 'mbr' command with hybrid MBR/GPT
When dealing with a hybrid MBR/GPT partition table, the 'mbr' command would misbehave because it was reading the GPT partitions instead of reading from the MBR when verifying. Fix this by forcing 'mbr verify' to read MBR partitions.
Joshua Watt (2): disk: part: Add API to get partitions with specific driver cmd: mbr: Force DOS driver to be used for verify
cmd/mbr.c | 2 +- disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 2 ++ 3 files changed, 34 insertions(+), 8 deletions(-)
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 35300df590..1f8c786ca5 100644 --- a/disk/part.c +++ b/disk/part.c @@ -26,6 +26,22 @@ /* Check all partition types */ #define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type) +{ + struct part_driver *drv = + ll_entry_start(struct part_driver, part_driver); + const int n_ents = ll_entry_count(struct part_driver, part_driver); + struct part_driver *entry; + + for (entry = drv; entry != drv + n_ents; entry++) { + if (part_type == entry->part_type) + return entry; + } + + /* Not found */ + return NULL; +} + static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) { struct part_driver *drv = @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) } } } else { - for (entry = drv; entry != drv + n_ents; entry++) { - if (dev_desc->part_type == entry->part_type) - return entry; - } + return part_driver_get_type(dev_desc->part_type); }
/* Not found */ @@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc) drv->print(dev_desc); }
-int part_get_info(struct blk_desc *dev_desc, int part, - struct disk_partition *info) +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type, + struct disk_partition *info) { struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, info->type_guid[0] = 0; #endif
- drv = part_driver_lookup_type(dev_desc); + if (part_type == PART_TYPE_UNKNOWN) { + drv = part_driver_lookup_type(dev_desc); + } else { + drv = part_driver_get_type(part_type); + } + if (!drv) { debug("## Unknown partition table type %x\n", dev_desc->part_type); @@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -ENOENT; }
+int part_get_info(struct blk_desc *dev_desc, int part, + struct disk_partition *info) +{ + return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info); +} + int part_get_info_whole_disk(struct blk_desc *dev_desc, struct disk_partition *info) { diff --git a/include/part.h b/include/part.h index be75c73549..f150c84206 100644 --- a/include/part.h +++ b/include/part.h @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev); struct blk_desc *mg_disk_get_dev(int dev);
/* disk/part.c */ +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type, + struct disk_partition *info); int part_get_info(struct blk_desc *dev_desc, int part, struct disk_partition *info); /**
Hi Joshua,
On Fri, 23 Jun 2023 at 21:01, Joshua Watt jpewhacker@gmail.com wrote:
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 35300df590..1f8c786ca5 100644 --- a/disk/part.c +++ b/disk/part.c @@ -26,6 +26,22 @@ /* Check all partition types */ #define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type) +{
struct part_driver *drv =ll_entry_start(struct part_driver, part_driver);const int n_ents = ll_entry_count(struct part_driver, part_driver);struct part_driver *entry;for (entry = drv; entry != drv + n_ents; entry++) {if (part_type == entry->part_type)return entry;}/* Not found */return NULL;+}
static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) { struct part_driver *drv = @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) } } } else {
for (entry = drv; entry != drv + n_ents; entry++) {if (dev_desc->part_type == entry->part_type)return entry;}
return part_driver_get_type(dev_desc->part_type); } /* Not found */@@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc) drv->print(dev_desc); }
-int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info){ struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, info->type_guid[0] = 0; #endif
drv = part_driver_lookup_type(dev_desc);
if (part_type == PART_TYPE_UNKNOWN) {drv = part_driver_lookup_type(dev_desc);} else {drv = part_driver_get_type(part_type);}if (!drv) { debug("## Unknown partition table type %x\n", dev_desc->part_type);@@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -ENOENT; }
+int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+{
return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info);+}
int part_get_info_whole_disk(struct blk_desc *dev_desc, struct disk_partition *info) { diff --git a/include/part.h b/include/part.h index be75c73549..f150c84206 100644 --- a/include/part.h +++ b/include/part.h @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev); struct blk_desc *mg_disk_get_dev(int dev);
/* disk/part.c */ +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info);
Please can you add a full comment?
Also please add a test to test/dm/part.c for your new function[1]
int part_get_info(struct blk_desc *dev_desc, int part, struct disk_partition *info); /** -- 2.33.0
Regards, Simon
On Mon, Jun 26, 2023 at 4:07 AM Simon Glass sjg@chromium.org wrote:
Hi Joshua,
On Fri, 23 Jun 2023 at 21:01, Joshua Watt jpewhacker@gmail.com wrote:
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 35300df590..1f8c786ca5 100644 --- a/disk/part.c +++ b/disk/part.c @@ -26,6 +26,22 @@ /* Check all partition types */ #define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type) +{
struct part_driver *drv =ll_entry_start(struct part_driver, part_driver);const int n_ents = ll_entry_count(struct part_driver, part_driver);struct part_driver *entry;for (entry = drv; entry != drv + n_ents; entry++) {if (part_type == entry->part_type)return entry;}/* Not found */return NULL;+}
static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) { struct part_driver *drv = @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) } } } else {
for (entry = drv; entry != drv + n_ents; entry++) {if (dev_desc->part_type == entry->part_type)return entry;}
return part_driver_get_type(dev_desc->part_type); } /* Not found */@@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc) drv->print(dev_desc); }
-int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info){ struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, info->type_guid[0] = 0; #endif
drv = part_driver_lookup_type(dev_desc);
if (part_type == PART_TYPE_UNKNOWN) {drv = part_driver_lookup_type(dev_desc);} else {drv = part_driver_get_type(part_type);}if (!drv) { debug("## Unknown partition table type %x\n", dev_desc->part_type);@@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -ENOENT; }
+int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+{
return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info);+}
int part_get_info_whole_disk(struct blk_desc *dev_desc, struct disk_partition *info) { diff --git a/include/part.h b/include/part.h index be75c73549..f150c84206 100644 --- a/include/part.h +++ b/include/part.h @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev); struct blk_desc *mg_disk_get_dev(int dev);
/* disk/part.c */ +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info);Please can you add a full comment?
Also please add a test to test/dm/part.c for your new function[1]
Any trick to getting the current test/dm/part.c tests to pass? When I run them they fail with `** No device specified **` errors, which doesn't give me hope that I can write tests that I could actually run locally.
int part_get_info(struct blk_desc *dev_desc, int part, struct disk_partition *info); /** -- 2.33.0
Regards, Simon
Hi Joshua,
On Wed, 28 Jun 2023 at 14:34, Joshua Watt jpewhacker@gmail.com wrote:
On Mon, Jun 26, 2023 at 4:07 AM Simon Glass sjg@chromium.org wrote:
Hi Joshua,
On Fri, 23 Jun 2023 at 21:01, Joshua Watt jpewhacker@gmail.com wrote:
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 2 ++ 2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 35300df590..1f8c786ca5 100644 --- a/disk/part.c +++ b/disk/part.c @@ -26,6 +26,22 @@ /* Check all partition types */ #define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type) +{
struct part_driver *drv =ll_entry_start(struct part_driver, part_driver);const int n_ents = ll_entry_count(struct part_driver, part_driver);struct part_driver *entry;for (entry = drv; entry != drv + n_ents; entry++) {if (part_type == entry->part_type)return entry;}/* Not found */return NULL;+}
static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) { struct part_driver *drv = @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) } } } else {
for (entry = drv; entry != drv + n_ents; entry++) {if (dev_desc->part_type == entry->part_type)return entry;}
return part_driver_get_type(dev_desc->part_type); } /* Not found */@@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc) drv->print(dev_desc); }
-int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info){ struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, info->type_guid[0] = 0; #endif
drv = part_driver_lookup_type(dev_desc);
if (part_type == PART_TYPE_UNKNOWN) {drv = part_driver_lookup_type(dev_desc);} else {drv = part_driver_get_type(part_type);}if (!drv) { debug("## Unknown partition table type %x\n", dev_desc->part_type);@@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -ENOENT; }
+int part_get_info(struct blk_desc *dev_desc, int part,
struct disk_partition *info)+{
return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info);+}
int part_get_info_whole_disk(struct blk_desc *dev_desc, struct disk_partition *info) { diff --git a/include/part.h b/include/part.h index be75c73549..f150c84206 100644 --- a/include/part.h +++ b/include/part.h @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev); struct blk_desc *mg_disk_get_dev(int dev);
/* disk/part.c */ +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
struct disk_partition *info);Please can you add a full comment?
Also please add a test to test/dm/part.c for your new function[1]
Any trick to getting the current test/dm/part.c tests to pass? When I run them they fail with `** No device specified **` errors, which doesn't give me hope that I can write tests that I could actually run locally.
Yes you probably need to run the ''test_ut_dm_init_bootstd' test first. This pytest is run automatically if you run everything, but otherwise is not. It sets up the mmc1.img file.
Regards, Simon
Forces the DOS partition type driver to be used when verifying the MBR. This is particularly useful when using a hybrid MBR & GPT layout as otherwise MBR verification would mostly likely fail since the GPT partitions will be returned, even if the MBR is actually valid.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- cmd/mbr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/mbr.c b/cmd/mbr.c index c269833eb8..ec99b66283 100644 --- a/cmd/mbr.c +++ b/cmd/mbr.c @@ -244,7 +244,7 @@ static int do_verify_mbr(struct blk_desc *dev, const char *str) for (i = 0; i < count; i++) { struct disk_partition p;
- if (part_get_info(dev, i + 1, &p)) + if (part_get_info_by_type(dev, i + 1, PART_TYPE_DOS, &p)) goto fail;
if ((partitions[i].size && p.size != partitions[i].size) ||
When dealing with a hybrid MBR/GPT partition table, the 'mbr' command would misbehave because it was reading the GPT partitions instead of reading from the MBR when verifying. Fix this by forcing 'mbr verify' to read MBR partitions.
V2: Fixed up dm_test_part tests and added tests for new API
Joshua Watt (5): dm: test: Fix partition test to use mmc2 dm: test: Improve partition test error output disk: part: Add API to get partitions with specific driver dm: test: Add test for part_get_info_by_type cmd: mbr: Force DOS driver to be used for verify
cmd/mbr.c | 2 +- configs/sandbox_defconfig | 2 + disk/part.c | 38 ++++++++++--- include/part.h | 19 ++++++- test/dm/part.c | 115 +++++++++++++++++++++++++++++++++----- 5 files changed, 154 insertions(+), 22 deletions(-)
d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the test use mmc2, but the change was incomplete in that it didn't also change the strings that reference a specific partition. Fix these so that the test passes again
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- test/dm/part.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/test/dm/part.c b/test/dm/part.c index 35e99eeb01..d5c58d6e3f 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -76,15 +76,15 @@ static int dm_test_part(struct unit_test_state *uts) test(-EINVAL, "#test1", true); test(1, "2", false); test(1, "2", true); - test(-ENOENT, "1:0", false); - test(0, "1:0", true); - test(1, "1:1", false); - test(2, "1:2", false); - test(1, "1.0", false); - test(0, "1.0:0", true); - test(1, "1.0:1", false); - test(2, "1.0:2", false); - test(-EINVAL, "1#bogus", false); + test(-ENOENT, "2:0", false); + test(0, "2:0", true); + test(1, "2:1", false); + test(2, "2:2", false); + test(1, "2.0", false); + test(0, "2.0:0", true); + test(1, "2.0:1", false); + test(2, "2.0:2", false); + test(-EINVAL, "2#bogus", false); test(1, "2#test1", false); test(2, "2#test2", false); ret = 0;
On Mon, 3 Jul 2023 at 14:40, Joshua Watt jpewhacker@gmail.com wrote:
d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the test use mmc2, but the change was incomplete in that it didn't also change the strings that reference a specific partition. Fix these so that the test passes again
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
test/dm/part.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org Fixes: d94d9844bc ("dm: part: Update test to use mmc2")
Thanks.
-----Original Message----- From: U-Boot u-boot-bounces@lists.denx.de On Behalf Of Joshua Watt Sent: Monday, July 3, 2023 10:40 PM To: u-boot@lists.denx.de Cc: Joshua Watt JPEWhacker@gmail.com; Simon Glass sjg@chromium.org Subject: [PATCH v2 1/5] dm: test: Fix partition test to use mmc2
d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the test use mmc2, but the change was incomplete in that it didn't also change the strings that reference a specific partition. Fix these so that the test passes again
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Best Regards, Jaehoon Chung
test/dm/part.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/test/dm/part.c b/test/dm/part.c index 35e99eeb01..d5c58d6e3f 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -76,15 +76,15 @@ static int dm_test_part(struct unit_test_state *uts) test(-EINVAL, "#test1", true); test(1, "2", false); test(1, "2", true);
- test(-ENOENT, "1:0", false);
- test(0, "1:0", true);
- test(1, "1:1", false);
- test(2, "1:2", false);
- test(1, "1.0", false);
- test(0, "1.0:0", true);
- test(1, "1.0:1", false);
- test(2, "1.0:2", false);
- test(-EINVAL, "1#bogus", false);
- test(-ENOENT, "2:0", false);
- test(0, "2:0", true);
- test(1, "2:1", false);
- test(2, "2:2", false);
- test(1, "2.0", false);
- test(0, "2.0:0", true);
- test(1, "2.0:1", false);
- test(2, "2.0:2", false);
- test(-EINVAL, "2#bogus", false); test(1, "2#test1", false); test(2, "2#test2", false); ret = 0;
-- 2.33.0
On Mon, Jul 03, 2023 at 08:39:52AM -0500, Joshua Watt wrote:
d94d9844bc ("dm: part: Update test to use mmc2") attempted to make the test use mmc2, but the change was incomplete in that it didn't also change the strings that reference a specific partition. Fix these so that the test passes again
Signed-off-by: Joshua Watt JPEWhacker@gmail.com Reviewed-by: Simon Glass sjg@chromium.org Reviewed-by: Jaehoon Chung jh80.chung@samsung.com
Applied to u-boot/master, thanks!
Improve the logging when the partition test fails so that it is clear what went wrong, shown with actual values.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- test/dm/part.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/test/dm/part.c b/test/dm/part.c index d5c58d6e3f..7dd8bc7a3c 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -17,10 +17,12 @@ static int do_test(struct unit_test_state *uts, int expected, struct blk_desc *mmc_dev_desc; struct disk_partition part_info;
- ut_asserteq(expected, - part_get_info_by_dev_and_name_or_num("mmc", part_str, - &mmc_dev_desc, - &part_info, whole)); + int ret = part_get_info_by_dev_and_name_or_num("mmc", part_str, + &mmc_dev_desc, + &part_info, whole); + + ut_assertf(expected == ret, "test(%d, "%s", %d) == %d", expected, + part_str, whole, ret); return 0; }
On Mon, 3 Jul 2023 at 14:40, Joshua Watt jpewhacker@gmail.com wrote:
Improve the logging when the partition test fails so that it is clear what went wrong, shown with actual values.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
test/dm/part.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Jul 03, 2023 at 08:39:53AM -0500, Joshua Watt wrote:
Improve the logging when the partition test fails so that it is clear what went wrong, shown with actual values.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 19 ++++++++++++++++++- 2 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/disk/part.c b/disk/part.c index 35300df590..1f8c786ca5 100644 --- a/disk/part.c +++ b/disk/part.c @@ -26,6 +26,22 @@ /* Check all partition types */ #define PART_TYPE_ALL -1
+static struct part_driver *part_driver_get_type(int part_type) +{ + struct part_driver *drv = + ll_entry_start(struct part_driver, part_driver); + const int n_ents = ll_entry_count(struct part_driver, part_driver); + struct part_driver *entry; + + for (entry = drv; entry != drv + n_ents; entry++) { + if (part_type == entry->part_type) + return entry; + } + + /* Not found */ + return NULL; +} + static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) { struct part_driver *drv = @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc) } } } else { - for (entry = drv; entry != drv + n_ents; entry++) { - if (dev_desc->part_type == entry->part_type) - return entry; - } + return part_driver_get_type(dev_desc->part_type); }
/* Not found */ @@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc) drv->print(dev_desc); }
-int part_get_info(struct blk_desc *dev_desc, int part, - struct disk_partition *info) +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type, + struct disk_partition *info) { struct part_driver *drv;
@@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, info->type_guid[0] = 0; #endif
- drv = part_driver_lookup_type(dev_desc); + if (part_type == PART_TYPE_UNKNOWN) { + drv = part_driver_lookup_type(dev_desc); + } else { + drv = part_driver_get_type(part_type); + } + if (!drv) { debug("## Unknown partition table type %x\n", dev_desc->part_type); @@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part, return -ENOENT; }
+int part_get_info(struct blk_desc *dev_desc, int part, + struct disk_partition *info) +{ + return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info); +} + int part_get_info_whole_disk(struct blk_desc *dev_desc, struct disk_partition *info) { diff --git a/include/part.h b/include/part.h index be75c73549..8a0c8732a6 100644 --- a/include/part.h +++ b/include/part.h @@ -105,7 +105,24 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev);
struct blk_desc *mg_disk_get_dev(int dev);
-/* disk/part.c */ +/** + * part_get_info_by_type() - Get partitions from a block device using a specific + * partition driver + * + * Each interface allocates its own devices and typically struct blk_desc is + * contained with the interface's data structure. There is no global + * numbering for block devices, so the interface name must be provided. + * + * @dev_desc: Block device descriptor + * @part: Partition number to read + * @part_type: Partition driver to use, or PART_TYPE_UNKNOWN to automatically + * choose a driver + * @info: Returned partition information + * + * Return: 0 on success, negative errno on failure + */ +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type, + struct disk_partition *info); int part_get_info(struct blk_desc *dev_desc, int part, struct disk_partition *info); /**
On Mon, 3 Jul 2023 at 14:40, Joshua Watt jpewhacker@gmail.com wrote:
Adds part_driver_get_type() API which can be used to force a specific
Nit: Add
driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
disk/part.c | 38 +++++++++++++++++++++++++++++++------- include/part.h | 19 ++++++++++++++++++- 2 files changed, 49 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Jul 03, 2023 at 08:39:54AM -0500, Joshua Watt wrote:
Adds part_driver_get_type() API which can be used to force a specific driver to be used when getting partition information instead of relying on auto detection.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Adds a test suite to ensure that part_get_info_by_type works correctly by creating a hybrid GPT/MBR partition table and reading both.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- configs/sandbox_defconfig | 2 + test/dm/part.c | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 1ec44d5b33..cbc5d5b895 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -71,6 +71,7 @@ CONFIG_CMD_GPIO_READ=y CONFIG_CMD_PWM=y CONFIG_CMD_GPT=y CONFIG_CMD_GPT_RENAME=y +CONFIG_CMD_MBR=y CONFIG_CMD_IDE=y CONFIG_CMD_I2C=y CONFIG_CMD_LOADM=y @@ -131,6 +132,7 @@ CONFIG_CMD_MTDPARTS=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y CONFIG_AMIGA_PARTITION=y +CONFIG_DOS_PARTITION=y CONFIG_OF_CONTROL=y CONFIG_OF_LIVE=y CONFIG_ENV_IS_NOWHERE=y diff --git a/test/dm/part.c b/test/dm/part.c index 7dd8bc7a3c..d6e4345812 100644 --- a/test/dm/part.c +++ b/test/dm/part.c @@ -108,3 +108,90 @@ static int dm_test_part_bootable(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_part_bootable, UT_TESTF_SCAN_FDT); + +static int do_get_info_test(struct unit_test_state *uts, + struct blk_desc *dev_desc, int part, int part_type, + struct disk_partition const *reference) +{ + struct disk_partition p; + int ret; + + memset(&p, 0, sizeof(p)); + + ret = part_get_info_by_type(dev_desc, part, part_type, &p); + printf("part_get_info_by_type(%d, 0x%x) = %d\n", part, part_type, ret); + if (ut_assertok(ret)) { + return 0; + } + + ut_asserteq(reference->start, p.start); + ut_asserteq(reference->size, p.size); + ut_asserteq(reference->sys_ind, p.sys_ind); + + return 0; +} + +static int dm_test_part_get_info_by_type(struct unit_test_state *uts) +{ + char str_disk_guid[UUID_STR_LEN + 1]; + struct blk_desc *mmc_dev_desc; + struct disk_partition gpt_parts[] = { + { + .start = 48, /* GPT data takes up the first 34 blocks or so */ + .size = 1, + .name = "test1", + .sys_ind = 0, + }, + { + .start = 49, + .size = 1, + .name = "test2", + .sys_ind = 0, + }, + }; + struct disk_partition mbr_parts[] = { + { + .start = 1, + .size = 33, + .name = "gpt", + .sys_ind = EFI_PMBR_OSTYPE_EFI_GPT, + }, + { + .start = 48, + .size = 1, + .name = "test1", + .sys_ind = 0x83, + }, + }; + + ut_asserteq(2, blk_get_device_by_str("mmc", "2", &mmc_dev_desc)); + if (CONFIG_IS_ENABLED(RANDOM_UUID)) { + gen_rand_uuid_str(gpt_parts[0].uuid, UUID_STR_FORMAT_STD); + gen_rand_uuid_str(gpt_parts[1].uuid, UUID_STR_FORMAT_STD); + gen_rand_uuid_str(str_disk_guid, UUID_STR_FORMAT_STD); + } + ut_assertok(gpt_restore(mmc_dev_desc, str_disk_guid, gpt_parts, + ARRAY_SIZE(gpt_parts))); + + ut_assertok(write_mbr_partitions(mmc_dev_desc, mbr_parts, + ARRAY_SIZE(mbr_parts), 0)); + +#define get_info_test(_part, _part_type, _reference) \ + ut_assertok(do_get_info_test(uts, mmc_dev_desc, _part, _part_type, \ + _reference)) + + for (int i = 0; i < ARRAY_SIZE(gpt_parts); i++) { + get_info_test(i + 1, PART_TYPE_UNKNOWN, &gpt_parts[i]); + } + + for (int i = 0; i < ARRAY_SIZE(mbr_parts); i++) { + get_info_test(i + 1, PART_TYPE_DOS, &mbr_parts[i]); + } + + for (int i = 0; i < ARRAY_SIZE(gpt_parts); i++) { + get_info_test(i + 1, PART_TYPE_EFI, &gpt_parts[i]); + } + + return 0; +} +DM_TEST(dm_test_part_get_info_by_type, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
On Mon, 3 Jul 2023 at 14:40, Joshua Watt jpewhacker@gmail.com wrote:
Adds a test suite to ensure that part_get_info_by_type works correctly by creating a hybrid GPT/MBR partition table and reading both.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
configs/sandbox_defconfig | 2 + test/dm/part.c | 87 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Jul 03, 2023 at 08:39:55AM -0500, Joshua Watt wrote:
Adds a test suite to ensure that part_get_info_by_type works correctly by creating a hybrid GPT/MBR partition table and reading both.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Forces the DOS partition type driver to be used when verifying the MBR. This is particularly useful when using a hybrid MBR & GPT layout as otherwise MBR verification would mostly likely fail since the GPT partitions will be returned, even if the MBR is actually valid.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com --- cmd/mbr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/mbr.c b/cmd/mbr.c index c269833eb8..ec99b66283 100644 --- a/cmd/mbr.c +++ b/cmd/mbr.c @@ -244,7 +244,7 @@ static int do_verify_mbr(struct blk_desc *dev, const char *str) for (i = 0; i < count; i++) { struct disk_partition p;
- if (part_get_info(dev, i + 1, &p)) + if (part_get_info_by_type(dev, i + 1, PART_TYPE_DOS, &p)) goto fail;
if ((partitions[i].size && p.size != partitions[i].size) ||
On Mon, 3 Jul 2023 at 14:41, Joshua Watt jpewhacker@gmail.com wrote:
Forces the DOS partition type driver to be used when verifying the MBR. This is particularly useful when using a hybrid MBR & GPT layout as otherwise MBR verification would mostly likely fail since the GPT partitions will be returned, even if the MBR is actually valid.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com
cmd/mbr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Jul 03, 2023 at 08:39:56AM -0500, Joshua Watt wrote:
Forces the DOS partition type driver to be used when verifying the MBR. This is particularly useful when using a hybrid MBR & GPT layout as otherwise MBR verification would mostly likely fail since the GPT partitions will be returned, even if the MBR is actually valid.
Signed-off-by: Joshua Watt JPEWhacker@gmail.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
participants (4)
-
Jaehoon Chung -
Joshua Watt -
Simon Glass -
Tom Rini