[PATCH 0/2] Fix 'mbr' command with hybrid MBR/GPT

[PATCH 0/2] Fix 'mbr' command with hybrid MBR/GPT

[Joshua Watt]

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(-)

-- 
2.33.0

[PATCH 1/2] disk: part: Add API to get partitions with specific driver

[Joshua Watt]

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);
 /**
-- 
2.33.0

Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver

[Simon Glass]

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

Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver

[Joshua Watt]

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

Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver

[Simon Glass]

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

[PATCH 2/2] cmd: mbr: Force DOS driver to be used for verify

[Joshua Watt]

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) ||
-- 
2.33.0

[PATCH v2 0/5] Fix 'mbr' command with hybrid MBR/GPT

[Joshua Watt]

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(-)

-- 
2.33.0

[PATCH v2 1/5] dm: test: Fix partition test to use mmc2

[Joshua Watt]

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;
-- 
2.33.0

Re: [PATCH v2 1/5] dm: test: Fix partition test to use mmc2

[Simon Glass]

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.

RE: [PATCH v2 1/5] dm: test: Fix partition test to use mmc2

[Jaehoon Chung]

> -----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

Re: [PATCH v2 1/5] dm: test: Fix partition test to use mmc2

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 519 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 2/5] dm: test: Improve partition test error output

[Joshua Watt]

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

Re: [PATCH v2 2/5] dm: test: Improve partition test error output

[Simon Glass]

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>

Re: [PATCH v2 2/5] dm: test: Improve partition test error output

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 336 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 3/5] disk: part: Add API to get partitions with specific driver

[Joshua Watt]

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);
 /**
-- 
2.33.0

Re: [PATCH v2 3/5] disk: part: Add API to get partitions with specific driver

[Simon Glass]

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>

Re: [PATCH v2 3/5] disk: part: Add API to get partitions with specific driver

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 4/5] dm: test: Add test for part_get_info_by_type

[Joshua Watt]

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);
-- 
2.33.0

Re: [PATCH v2 4/5] dm: test: Add test for part_get_info_by_type

[Simon Glass]

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>

Re: [PATCH v2 4/5] dm: test: Add test for part_get_info_by_type

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 357 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

[PATCH v2 5/5] cmd: mbr: Force DOS driver to be used for verify

[Joshua Watt]

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) ||
-- 
2.33.0

Re: [PATCH v2 5/5] cmd: mbr: Force DOS driver to be used for verify

[Simon Glass]

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>

Re: [PATCH v2 5/5] cmd: mbr: Force DOS driver to be used for verify

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

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!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]