[PATCH v2] cmd: mbr: Allow 4 MBR partitions without need for extended

9 Oct 2023

Current code allows up to 3 MBR partitions without extended one.
If more than 3 partitions are required, then extended partition(s)
must be used.
This commit allows up to 4 primary MBR partitions without the
need for extended partition.
Add mbr test unit. In order to run the test manually, mmc6.img file
of size 12 MiB or greater is required in the same directory as u-boot.
Test also runs automatically via ./test/py/test.py tool.
Running mbr test is only supported in sandbox mode.
Signed-off-by: Alex Gendin agendin@matrox.com
---
Changes for v2:
- Cleanup coding style
- Adjust commit message to the changes in v2
- Add mmc6 device to sandbox device tree
- Adjust boot/bootdev.c and dm/blk.c tests to include mmc6 in tests
- Auto-create mmc6.img in test_ut_dm_init()
- Add mbr test to the list of tests run by test.py. Test can be run manually
  too via 'ut mbr'.
- Change mbr test to use mmc6.img dedicated to the test, instead of mmc1.img.
- Clear read buffer before reading data from test file.
arch/sandbox/dts/test.dts |   7 +
 disk/part_dos.c           |   2 +-
 include/test/suites.h     |   1 +
 test/boot/bootdev.c       |   2 +-
 test/cmd/Makefile         |   1 +
 test/cmd/mbr.c            | 471 ++++++++++++++++++++++++++++++++++++++
 test/cmd_ut.c             |   4 +
 test/dm/blk.c             |  44 ++--
 test/py/tests/test_ut.py  |   7 +-
 9 files changed, 522 insertions(+), 17 deletions(-)
 create mode 100644 test/cmd/mbr.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 6abce9e396..59b7f8ef10 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -41,6 +41,7 @@
    	mmc3 = "/mmc3";
    	mmc4 = "/mmc4";
    	mmc5 = "/mmc5";
+		mmc6 = "/mmc6";
    	pci0 = &pci0;
    	pci1 = &pci1;
    	pci2 = &pci2;
@@ -1101,6 +1102,12 @@
    	filename = "mmc5.img";
    };
+	/* This is used for mbr tests */
+	mmc6 {
+		compatible = "sandbox,mmc";
+		filename = "mmc6.img";
+	};
+
    pch {
    	compatible = "sandbox,pch";
    };
diff --git a/disk/part_dos.c b/disk/part_dos.c
index 3337438437..567ead7511 100644
--- a/disk/part_dos.c
+++ b/disk/part_dos.c
@@ -466,7 +466,7 @@ int layout_mbr_partitions(struct disk_partition *p, int count,
    		ext = &p[i];
    }
-	if (count < 4)
+	if (count <= 4)
    	return 0;
if (!ext) {
diff --git a/include/test/suites.h b/include/test/suites.h
index 1c7dc65966..51acbe47b2 100644
--- a/include/test/suites.h
+++ b/include/test/suites.h
@@ -45,6 +45,7 @@ int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_loadm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_log(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
+int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
 int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc,
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 6b29213416..4d125104c7 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -128,7 +128,7 @@ static int bootdev_test_labels(struct unit_test_state *uts)
    	    bootdev_find_by_label("fred0", &dev, &mflags));
/* Check unknown sequence number */
-	ut_asserteq(-ENOENT, bootdev_find_by_label("mmc6", &dev, &mflags));
+	ut_asserteq(-ENOENT, bootdev_find_by_label("mmc7", &dev, &mflags));
return 0;
 }
diff --git a/test/cmd/Makefile b/test/cmd/Makefile
index 6e3d7e919e..2f251e07b4 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -23,6 +23,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o
 obj-$(CONFIG_CMD_PWM) += pwm.o
 obj-$(CONFIG_CMD_SEAMA) += seama.o
 ifdef CONFIG_SANDBOX
+obj-$(CONFIG_CMD_MBR) += mbr.o
 obj-$(CONFIG_CMD_READ) += rw.o
 obj-$(CONFIG_CMD_SETEXPR) += setexpr.o
 obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c
new file mode 100644
index 0000000000..4091ea687f
--- /dev/null
+++ b/test/cmd/mbr.c
@@ -0,0 +1,471 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Tests for mbr command
+ *
+ * Copyright 2023 Matrox Video
+ * Written by Alex Gendin agendin@matrox.com
+ */
+
+#include <common.h>
+#include <asm/global_data.h>
+#include <console.h>
+#include <dm/test.h>
+#include <mapmem.h>
+#include <part.h>
+#include <test/suites.h>
+#include <test/ut.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+/*
+ * Requirements for running test manually:
+ * mmc6.img - File size needs to be at least 12 MiB
+ *
+ * Command to create mmc6.img:
+ * $ dd if=/dev/zero of=mmc6.img bs=12M count=1
+ *
+ * To run this test manually, place mmc6.img into the same directory as u-boot,
+ * then run:
+ * $ ./u-boot -Tc 'ut mbr'
+ *
+ * To run this test as part of U-Boot test:
+ * $ ./test/py/test.py --bd sandbox --build -k ut_dm -v
+ * Note: mmc6.img will be created by the test suit.
+ */
+
+static char * mbr_parts_header = "setenv mbr_parts '";
+static char * mbr_parts_p1 = "uuid_disk=0x12345678;name=p1,start=8M,bootable,size=1M,id=0x0e";
+static char * mbr_parts_p2 = ";name=p2,size=1M,id=0x0e";
+static char * mbr_parts_p3 = ";name=p3,size=1M,id=0x0e";
+static char * mbr_parts_p4 = ";name=p4,size=1M,id=0x0e";
+static char * mbr_parts_p5 = ";name=[ext],size=2M,id=0x05;name=p5,size=1M,id=0x0e";
+static char * mbr_parts_tail = "'";
+
+/*
+ * One MBR partition
+000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 00  |...%$..@........|
+000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+*/
+static unsigned mbr_cmp_start = 0x1B8;
+static unsigned mbr_cmp_size = 0x48;
+static unsigned char mbr_parts_ref_p1[] = {
+                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
+};
+
+/*
+ * Two MBR partitions
+000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 00  |%..F...H........|
+000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+*/
+static unsigned char mbr_parts_ref_p2[] = {
+                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
+0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
+};
+
+/*
+ * Three MBR partitions
+000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 00  |...f%..P........|
+000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+*/
+static unsigned char mbr_parts_ref_p3[] = {
+                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
+0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
+0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
+};
+
+/*
+ * Four MBR partitions
+000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
+000001f0  26 01 0e 87 06 01 00 58  00 00 00 08 00 00 55 aa  |&......X......U.|
+*/
+static unsigned char mbr_parts_ref_p4[] = {
+                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
+0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
+0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x66,
+0x26, 0x01, 0x0e, 0x87, 0x06, 0x01, 0x00, 0x58, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x55, 0xaa
+};
+
+/*
+ * Five MBR partitions
+000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
+000001f0  26 01 05 a7 26 01 00 58  00 00 00 10 00 00 55 aa  |&...&..X......U.|
+*/
+static unsigned char mbr_parts_ref_p5[] = {
+                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
+0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
+0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
+0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x66,
+0x26, 0x01, 0x05, 0xa7, 0x26, 0x01, 0x00, 0x58, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x55, 0xaa
+};
+static unsigned ebr_cmp_start = 0x1B8;
+static unsigned ebr_cmp_size = 0x48;
+/*
+ * EBR
+00b001b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 87  |................|
+00b001c0  07 01 0e a7 26 01 00 08  00 00 00 08 00 00 00 00  |....&...........|
+00b001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+00b001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+00b001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+*/
+static unsigned char ebr_parts_ref_p5[] = {
+                                                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x87,
+0x07, 0x01, 0x0e, 0xa7, 0x26, 0x01, 0x00, 0x08, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
+};
+
+/* Fill write buffers with pseudo-random data */
+static void init_write_buffers(char *mbr_wb, size_t mbr_wb_size,
+			char *ebr_wb, size_t ebr_wb_size, unsigned seed)
+{
+	while (mbr_wb_size--) {
+		*mbr_wb++ = seed;
+		seed *= 43;
+		seed += 17 + mbr_wb_size/4;
+	}
+	while (ebr_wb_size--) {
+		*ebr_wb++ = seed;
+		seed *= 43;
+		seed += 17 + ebr_wb_size/4;
+	}
+}
+
+/* Build string with MBR partition(s) layout */
+static unsigned build_mbr_parts(char *buf, size_t buf_size, unsigned num_parts)
+{
+	size_t bytes_remaining, cur_str_size;
+	char * cur_buf;
+
+	if (!num_parts || num_parts > 5 || !buf)
+		return 1;
+
+	cur_buf = buf;
+	*cur_buf = '\0';
+	bytes_remaining = buf_size;
+
+	cur_str_size = sizeof(mbr_parts_header);
+	if (cur_str_size + 1 > bytes_remaining)
+		return 1;
+	strcat(cur_buf, mbr_parts_header);
+	bytes_remaining -= cur_str_size;
+
+	if (num_parts >= 1) {
+		cur_str_size = sizeof(mbr_parts_p1);
+		if (cur_str_size + 1 > bytes_remaining)
+			return 1;
+		strcat(cur_buf, mbr_parts_p1);
+		bytes_remaining -= cur_str_size;
+
+		if (num_parts >= 2) {
+			cur_str_size = sizeof(mbr_parts_p2);
+			if (cur_str_size + 1 > bytes_remaining)
+				return 1;
+			strcat(cur_buf, mbr_parts_p2);
+			bytes_remaining -= cur_str_size;
+
+			if (num_parts >= 3) {
+				cur_str_size = sizeof(mbr_parts_p3);
+				if (cur_str_size + 1 > bytes_remaining)
+					return 1;
+				strcat(cur_buf, mbr_parts_p3);
+				bytes_remaining -= cur_str_size;
+
+				if (num_parts == 4) {
+					cur_str_size = sizeof(mbr_parts_p4);
+					if (cur_str_size + 1 > bytes_remaining)
+						return 1;
+					strcat(cur_buf, mbr_parts_p4);
+					bytes_remaining -= cur_str_size;
+
+				}
+			else if (num_parts == 5) {
+				cur_str_size = sizeof(mbr_parts_p5);
+				if (cur_str_size + 1 > bytes_remaining)
+					return 1;
+				strcat(cur_buf, mbr_parts_p5);
+				bytes_remaining -= cur_str_size;
+
+			}
+			else if (num_parts > 5)
+				return 1;
+			}
+		}
+	}
+
+	cur_str_size = sizeof(mbr_parts_tail);
+	if (cur_str_size + 1 > bytes_remaining)
+		return 1;
+	strcat(cur_buf, mbr_parts_tail);
+
+	return 0;
+}
+
+static int mbr_test_run(struct unit_test_state *uts)
+{
+	struct blk_desc *mmc_dev_desc;
+	unsigned char mbr_wbuf[512], ebr_wbuf[512], rbuf[512];
+	char mbr_parts_buf[256];
+	ulong mbr_wa, ebr_wa, ra, ebr_blk, mbr_parts_max;
+
+	mbr_parts_max = sizeof('\0') + 2 +
+		strlen(mbr_parts_header) +
+		strlen(mbr_parts_p1) +
+		strlen(mbr_parts_p2) +
+		strlen(mbr_parts_p3) +
+		max(strlen(mbr_parts_p4), strlen(mbr_parts_p5)) +
+		strlen(mbr_parts_tail);
+	ut_assertf(sizeof(mbr_parts_buf) >= mbr_parts_max, "Buffer avail: %ld; buffer req: %ld\n",
+		sizeof(mbr_parts_buf), mbr_parts_max);
+
+	mbr_wa = map_to_sysmem(mbr_wbuf);
+	ebr_wa = map_to_sysmem(ebr_wbuf);
+	ra = map_to_sysmem(rbuf);
+	ebr_blk = (ulong)0xB00000 / 0x200;
+
+	/* Make sure mmc6 exists */
+	ut_asserteq(6, blk_get_device_by_str("mmc", "6", &mmc_dev_desc));
+	ut_assertok(console_record_reset_enable());
+	ut_assertok(run_commandf("mmc dev 6"));
+	ut_assert_nextline("switch to partitions #0, OK");
+	ut_assert_nextline("mmc6 is current device");
+	ut_assertok(ut_check_console_end(uts));
+
+	/* Make sure mmc6 is 12+ MiB in size */
+	ut_assertok(run_commandf("mmc read 0x%lx 0x%lx 1", ra, (ulong)0xBFFE00 / 0x200));
+
+	/* Test one MBR partition */
+	init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
+	ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 1));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(console_record_reset_enable());
+	ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assertok(run_commandf("mbr write mmc 6"));
+	ut_assert_nextline("MBR: write success!");
+	ut_assertok(run_commandf("mbr verify mmc 6"));
+	ut_assert_nextline("MBR: verify success!");
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(ut_check_console_end(uts));
+	/*
+	000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+	000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 00  |...%$..@........|
+	000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+	000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+	000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	for (unsigned i = 0; i < mbr_cmp_size; i++) {
+		ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p1[i],
+			"1P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			mbr_cmp_start + i, mbr_parts_ref_p1[i], rbuf[mbr_cmp_start + i]);
+	}
+
+	/* Test two MBR partitions */
+	init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
+	ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 2));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(console_record_reset_enable());
+	ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assertok(run_commandf("mbr write mmc 6"));
+	ut_assert_nextline("MBR: write success!");
+	ut_assertok(run_commandf("mbr verify mmc 6"));
+	ut_assert_nextline("MBR: verify success!");
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(ut_check_console_end(uts));
+	/*
+	000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+	000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+	000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 00  |%..F...H........|
+	000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+	000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	for (unsigned i = 0; i < mbr_cmp_size; i++) {
+		ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p2[i],
+			"2P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			mbr_cmp_start + i, mbr_parts_ref_p2[i], rbuf[mbr_cmp_start + i]);
+	}
+
+	/* Test three MBR partitions */
+	init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
+	ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 3));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(console_record_reset_enable());
+	ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assertok(run_commandf("mbr write mmc 6"));
+	ut_assert_nextline("MBR: write success!");
+	ut_assertok(run_commandf("mbr verify mmc 6"));
+	ut_assert_nextline("MBR: verify success!");
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(ut_check_console_end(uts));
+	/*
+	000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+	000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+	000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+	000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 00  |...f%..P........|
+	000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	for (unsigned i = 0; i < mbr_cmp_size; i++) {
+		ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p3[i],
+			"3P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			mbr_cmp_start + i, mbr_parts_ref_p3[i], rbuf[mbr_cmp_start + i]);
+	}
+
+	/* Test four MBR partitions */
+	init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
+	ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 4));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(console_record_reset_enable());
+	ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assertok(run_commandf("mbr write mmc 6"));
+	ut_assert_nextline("MBR: write success!");
+	ut_assertok(run_commandf("mbr verify mmc 6"));
+	ut_assert_nextline("MBR: verify success!");
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(ut_check_console_end(uts));
+	/*
+	000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+	000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+	000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+	000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
+	000001f0  26 01 0e 87 06 01 00 58  00 00 00 08 00 00 55 aa  |&......X......U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	for (unsigned i = 0; i < mbr_cmp_size; i++) {
+		ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p4[i],
+			"4P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			mbr_cmp_start + i, mbr_parts_ref_p4[i], rbuf[mbr_cmp_start + i]);
+	}
+
+	/* Test five MBR partitions */
+	init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
+	ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 5));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0 1", mbr_wa));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
+	ut_assertok(run_commandf("write mmc 6:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
+	ut_assertok(console_record_reset_enable());
+	ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assertf(0 == run_commandf("mbr write mmc 6"), "Invalid partitions string: %s\n", mbr_parts_buf);
+	ut_assert_nextline("MBR: write success!");
+	ut_assertok(run_commandf("mbr verify mmc 6"));
+	ut_assert_nextline("MBR: verify success!");
+	ut_assertok(ut_check_console_end(uts));
+	/*
+	000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
+	000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
+	000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
+	000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
+	000001f0  26 01 05 a7 26 01 00 58  00 00 00 10 00 00 55 aa  |&...&..X......U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0 1", ra));
+	for (unsigned i = 0; i < mbr_cmp_size; i++) {
+		ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p5[i],
+			"5P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			mbr_cmp_start + i, mbr_parts_ref_p5[i], rbuf[mbr_cmp_start + i]);
+	}
+	/*
+	00b001b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 87  |................|
+	00b001c0  07 01 0e a7 26 01 00 08  00 00 00 08 00 00 00 00  |....&...........|
+	00b001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+	00b001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
+	00b001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
+	*/
+	memset(rbuf, 0, sizeof(rbuf));
+	ut_assertok(run_commandf("read mmc 6:0 0x%lx 0x%lx 1", ra, ebr_blk));
+	for (unsigned i = 0; i < ebr_cmp_size; i++) {
+		ut_assertf(rbuf[ebr_cmp_start + i] == ebr_parts_ref_p5[i],
+			"5P EBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
+			ebr_cmp_start + i, ebr_parts_ref_p5[i], rbuf[ebr_cmp_start + i]);
+	}
+
+	return 0;
+}
+
+/* Declare mbr test */
+UNIT_TEST(mbr_test_run, UT_TESTF_CONSOLE_REC, mbr_test);
+
+int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
+{
+	struct unit_test *tests = UNIT_TEST_SUITE_START(mbr_test);
+	const int n_ents = UNIT_TEST_SUITE_COUNT(mbr_test);
+
+	return cmd_ut_category("mbr", "mbr_test_", tests, n_ents, argc, argv);
+}
+
+static int dm_test_cmd_mbr(struct unit_test_state *uts)
+{
+	return mbr_test_run(uts);
+}
+
+DM_TEST(dm_test_cmd_mbr, UT_TESTF_SCAN_FDT | UT_TESTF_CONSOLE_REC);
diff --git a/test/cmd_ut.c b/test/cmd_ut.c
index 0f56409e80..123db7cc7d 100644
--- a/test/cmd_ut.c
+++ b/test/cmd_ut.c
@@ -86,6 +86,10 @@ static struct cmd_tbl cmd_ut_sub[] = {
 #endif
 #ifdef CONFIG_UT_LOG
    U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_ut_log, "", ""),
+#endif
+#if defined(CONFIG_SANDBOX) && defined(CONFIG_CMD_MBR) && defined(CONFIG_CMD_MMC) \
+        && defined(CONFIG_MMC_SANDBOX) && defined(CONFIG_MMC_WRITE)
+	U_BOOT_CMD_MKENT(mbr, CONFIG_SYS_MAXARGS, 1, do_ut_mbr, "", ""),
 #endif
    U_BOOT_CMD_MKENT(mem, CONFIG_SYS_MAXARGS, 1, do_ut_mem, "", ""),
 #if defined(CONFIG_SANDBOX) && defined(CONFIG_CMD_SETEXPR)
diff --git a/test/dm/blk.c b/test/dm/blk.c
index 446c4423e6..8ec5087bf4 100644
--- a/test/dm/blk.c
+++ b/test/dm/blk.c
@@ -82,12 +82,12 @@ static int dm_test_blk_usb(struct unit_test_state *uts)
    ut_asserteq_ptr(usb_dev, dev_get_parent(dev));
/* Check we have one block device for each mass storage device */
-	ut_asserteq(6, count_blk_devices());
+	ut_asserteq(7, count_blk_devices());
/* Now go around again, making sure the old devices were unbound */
    ut_assertok(usb_stop());
    ut_assertok(usb_init());
-	ut_asserteq(6, count_blk_devices());
+	ut_asserteq(7, count_blk_devices());
    ut_assertok(usb_stop());
return 0;
@@ -190,6 +190,8 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
    ut_asserteq_str("mmc1.blk", dev->name);
    ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev));
    ut_asserteq_str("mmc0.blk", dev->name);
+	ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev));
+	ut_asserteq_str("mmc6.blk", dev->name);
    ut_asserteq(-ENODEV, blk_next_device_err(BLKF_REMOVABLE, &dev));
ut_assertok(blk_first_device_err(BLKF_BOTH, &dev));
@@ -201,8 +203,8 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
    ut_asserteq(-ENODEV, blk_next_device_err(BLKF_FIXED, &dev));
ut_asserteq(1, blk_count_devices(BLKF_FIXED));
-	ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE));
-	ut_asserteq(3, blk_count_devices(BLKF_BOTH));
+	ut_asserteq(3, blk_count_devices(BLKF_REMOVABLE));
+	ut_asserteq(4, blk_count_devices(BLKF_BOTH));
i = 0;
    blk_foreach_probe(BLKF_FIXED, dev)
@@ -211,14 +213,16 @@ static int dm_test_blk_iter(struct unit_test_state *uts)
i = 0;
    blk_foreach_probe(BLKF_REMOVABLE, dev)
-		ut_asserteq_str(i++ ? "mmc0.blk" : "mmc1.blk", dev->name);
-	ut_asserteq(2, i);
+		ut_asserteq_str((++i == 1 ? "mmc1.blk" : i == 2 ?
+			"mmc0.blk" : "mmc6.blk"), dev->name);
+	ut_asserteq(3, i);
i = 0;
    blk_foreach_probe(BLKF_BOTH, dev)
    	ut_asserteq_str((++i == 1 ? "mmc2.blk" : i == 2 ?
-			"mmc1.blk" : "mmc0.blk"), dev->name);
-	ut_asserteq(3, i);
+			"mmc1.blk" : i == 3 ? "mmc0.blk" : "mmc6.blk"),
+			dev->name);
+	ut_asserteq(4, i);
return 0;
 }
@@ -242,6 +246,10 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
    ut_assertnonnull(dev);
    ut_asserteq_str("mmc0.blk", dev->name);
+	ut_assertok(blk_find_next(BLKF_BOTH, &dev));
+	ut_assertnonnull(dev);
+	ut_asserteq_str("mmc6.blk", dev->name);
+
    ut_asserteq(-ENODEV, blk_find_next(BLKF_BOTH, &dev));
    ut_assertnull(dev);
@@ -265,6 +273,10 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
    ut_assertnonnull(dev);
    ut_asserteq_str("mmc0.blk", dev->name);
+	ut_assertok(blk_next_device_err(BLKF_BOTH, &dev));
+	ut_assertnonnull(dev);
+	ut_asserteq_str("mmc6.blk", dev->name);
+
    ut_asserteq(-ENODEV, blk_next_device_err(BLKF_BOTH, &dev));
/* Look only for fixed devices */
@@ -283,6 +295,10 @@ static int dm_test_blk_flags(struct unit_test_state *uts)
    ut_assertnonnull(dev);
    ut_asserteq_str("mmc0.blk", dev->name);
+	ut_assertok(blk_next_device_err(BLKF_REMOVABLE, &dev));
+	ut_assertnonnull(dev);
+	ut_asserteq_str("mmc6.blk", dev->name);
+
    ut_asserteq(-ENODEV, blk_next_device_err(BLKF_REMOVABLE, &dev));
return 0;
@@ -299,7 +315,7 @@ static int dm_test_blk_foreach(struct unit_test_state *uts)
    found = 0;
    blk_foreach(BLKF_BOTH, dev)
    	found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(7, found);
+	ut_asserteq(0x47, found);
/* All devices are removable until probed */
    found = 0;
@@ -310,14 +326,14 @@ static int dm_test_blk_foreach(struct unit_test_state *uts)
    found = 0;
    blk_foreach(BLKF_REMOVABLE, dev)
    	found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(7, found);
+	ut_asserteq(0x47, found);
/* Now try again with the probing functions */
    found = 0;
    blk_foreach_probe(BLKF_BOTH, dev)
    	found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(7, found);
-	ut_asserteq(3, blk_count_devices(BLKF_BOTH));
+	ut_asserteq(0x47, found);
+	ut_asserteq(4, blk_count_devices(BLKF_BOTH));
found = 0;
    blk_foreach_probe(BLKF_FIXED, dev)
@@ -328,8 +344,8 @@ static int dm_test_blk_foreach(struct unit_test_state *uts)
    found = 0;
    blk_foreach_probe(BLKF_REMOVABLE, dev)
    	found |= 1 << dectoul(&dev->name[3], NULL);
-	ut_asserteq(3, found);
-	ut_asserteq(2, blk_count_devices(BLKF_REMOVABLE));
+	ut_asserteq(0x43, found);
+	ut_asserteq(3, blk_count_devices(BLKF_REMOVABLE));
return 0;
 }
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index 82932a662b..1d9149a3f6 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -433,7 +433,6 @@ def setup_cedit_file(cons):
     u_boot_utils.run_and_log(
         cons, f'{expo_tool} -e {inhname} -l {infname} -o {outfname}')
-
 @pytest.mark.buildconfigspec('ut_dm')
 def test_ut_dm_init(u_boot_console):
     """Initialize data for ut dm tests."""
@@ -463,6 +462,12 @@ def test_ut_dm_init(u_boot_console):
     fs_helper.mk_fs(u_boot_console.config, 'ext2', 0x200000, '2MB')
     fs_helper.mk_fs(u_boot_console.config, 'fat32', 0x100000, '1MB')
+    mmc_dev = 6
+    fn = os.path.join(u_boot_console.config.source_dir, f'mmc{mmc_dev}.img')
+    data = b'\x00' * (12 * 1024 * 1024)
+    with open(fn, 'wb') as fh:
+        fh.write(data)
+
 @pytest.mark.buildconfigspec('cmd_bootflow')
 def test_ut_dm_init_bootstd(u_boot_console):
     """Initialise data for bootflow tests"""
-- 
2.41.0


On Sat, Oct 07, 2023 at 05:10:02PM -0600, Simon Glass sjg@google.com wrote:
> Hi Alexander,
> 
> On Fri, 6 Oct 2023 at 21:58, Alexander Gendin agendin@matrox.com wrote:
> >
> > Current code allows up to 3 MBR partitions without extended one.
> > If more than 3 partitions are required, then extended partition(s)
> > must be used.
> > This commit allows up to 4 primary MBR partitions without the
> > need for extended partition.
> >
> > Add mbr test unit. In order to use the test, mmc1.img file of size
> > 12 MiB or greater is required in the same directory as u-boot.
> > Running mbr test is only supported in sandbox mode.
> >
> > Signed-off-by: Alex Gendin agendin@matrox.com
> > ---
> >  disk/part_dos.c       |   2 +-
> >  include/test/suites.h |   1 +
> >  test/cmd/Makefile     |   1 +
> >  test/cmd/mbr.c        | 440 ++++++++++++++++++++++++++++++++++++++++++
> >  test/cmd_ut.c         |   4 +
> >  5 files changed, 447 insertions(+), 1 deletion(-)
> >  create mode 100644 test/cmd/mbr.c
> 
> This looks OK apart from one thing...also I have a few suggestions /
> questions below.
> 
> >
> > diff --git a/disk/part_dos.c b/disk/part_dos.c
> > index 3337438437..567ead7511 100644
> > --- a/disk/part_dos.c
> > +++ b/disk/part_dos.c
> > @@ -466,7 +466,7 @@ int layout_mbr_partitions(struct disk_partition *p, int count,
> >                         ext = &p[i];
> >         }
> >
> > -       if (count < 4)
> > +       if (count <= 4)
> >                 return 0;
> >
> >         if (!ext) {
> > diff --git a/include/test/suites.h b/include/test/suites.h
> > index 1c7dc65966..51acbe47b2 100644
> > --- a/include/test/suites.h
> > +++ b/include/test/suites.h
> > @@ -45,6 +45,7 @@ int do_ut_font(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_lib(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_loadm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_log(struct cmd_tbl *cmdtp, int flag, int argc, char * const argv[]);
> > +int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_mem(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_optee(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]);
> >  int do_ut_overlay(struct cmd_tbl *cmdtp, int flag, int argc,
> > diff --git a/test/cmd/Makefile b/test/cmd/Makefile
> > index 6e3d7e919e..2f251e07b4 100644
> > --- a/test/cmd/Makefile
> > +++ b/test/cmd/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_CMD_PINMUX) += pinmux.o
> >  obj-$(CONFIG_CMD_PWM) += pwm.o
> >  obj-$(CONFIG_CMD_SEAMA) += seama.o
> >  ifdef CONFIG_SANDBOX
> > +obj-$(CONFIG_CMD_MBR) += mbr.o
> >  obj-$(CONFIG_CMD_READ) += rw.o
> >  obj-$(CONFIG_CMD_SETEXPR) += setexpr.o
> >  obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o
> > diff --git a/test/cmd/mbr.c b/test/cmd/mbr.c
> > new file mode 100644
> > index 0000000000..bedcef0638
> > --- /dev/null
> > +++ b/test/cmd/mbr.c
> > @@ -0,0 +1,440 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Tests for mbr command
> > + *
> > + * Copyright 2023 Matrox Video
> > + * Written by Alex Gendin agendin@matrox.com
> > + */
> > +
> > +#include <common.h>
> > +#include <console.h>
> > +#include <mapmem.h>
> > +#include <part.h>
> > +#include <asm/global_data.h>
> > +#include <test/suites.h>
> > +#include <test/ut.h>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +/*
> > + * Test requirements:
> > + * mmc1.img - File size needs to be at least 12 MiB
> > + *
> > + * Command to create mmc1.img:
> > + * $ dd if=/dev/zero of=mmc1.img bs=12M count=1
> > + *
> > + * Place mmc1.img into the same directory as sandboxed u-boot
> > + *
> > + * To run this test manually:
> > + * $ ./u-boot -Tc 'ut mbr'
> 
> These requirements need to be dealt with by a test script. You can set
> up this device in test_ut_dm_init_bootstd(). You may need to create a
> new mmc as the others are in use.

Done, in test_ut_dm_init(). New mmc6 is added for this test.

> 
> > + */
> > +
> > +static char * mbr_parts_header = "setenv mbr_parts '";
> > +static char * mbr_parts_p1 = "uuid_disk=0x12345678;name=p1,start=8M,bootable,size=1M,id=0x0e";
> > +static char * mbr_parts_p2 = ";name=p2,size=1M,id=0x0e";
> > +static char * mbr_parts_p3 = ";name=p3,size=1M,id=0x0e";
> > +static char * mbr_parts_p4 = ";name=p4,size=1M,id=0x0e";
> > +static char * mbr_parts_p5 = ";name=[ext],size=2M,id=0x05;name=p5,size=1M,id=0x0e";
> > +static char * mbr_parts_tail = "'";
> > +
> > +/*
> > + * One MBR partition
> > +000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 00  |...%$..@........|
> > +000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +*/
> > +static unsigned mbr_cmp_start = 0x1B8;
> > +static unsigned mbr_cmp_size = 0x48;
> > +static unsigned char mbr_parts_ref_p1[] = {
> > +                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
> > +0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> > +};
> > +
> > +/*
> > + * Two MBR partitions
> > +000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 00  |%..F...H........|
> > +000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +*/
> > +static unsigned char mbr_parts_ref_p2[] = {
> > +                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
> > +0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
> > +0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> > +};
> > +
> > +/*
> > + * Three MBR partitions
> > +000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 00  |...f%..P........|
> > +000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +*/
> > +static unsigned char mbr_parts_ref_p3[] = {
> > +                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
> > +0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
> > +0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
> > +0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> > +};
> > +
> > +/*
> > + * Four MBR partitions
> > +000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
> > +000001f0  26 01 0e 87 06 01 00 58  00 00 00 08 00 00 55 aa  |&......X......U.|
> > +*/
> > +static unsigned char mbr_parts_ref_p4[] = {
> > +                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
> > +0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
> > +0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
> > +0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x66,
> > +0x26, 0x01, 0x0e, 0x87, 0x06, 0x01, 0x00, 0x58, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x55, 0xaa
> > +};
> > +
> > +/*
> > + * Five MBR partitions
> > +000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
> > +000001f0  26 01 05 a7 26 01 00 58  00 00 00 10 00 00 55 aa  |&...&..X......U.|
> > +*/
> > +static unsigned char mbr_parts_ref_p5[] = {
> > +                                                0x78, 0x56, 0x34, 0x12, 0x00, 0x00, 0x80, 0x05,
> > +0x05, 0x01, 0x0e, 0x25, 0x24, 0x01, 0x00, 0x40, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x25,
> > +0x25, 0x01, 0x0e, 0x46, 0x05, 0x01, 0x00, 0x48, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x46,
> > +0x06, 0x01, 0x0e, 0x66, 0x25, 0x01, 0x00, 0x50, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x66,
> > +0x26, 0x01, 0x05, 0xa7, 0x26, 0x01, 0x00, 0x58, 0x00, 0x00, 0x00, 0x10, 0x00, 0x00, 0x55, 0xaa
> > +};
> > +static unsigned ebr_cmp_start = 0x1B8;
> > +static unsigned ebr_cmp_size = 0x48;
> > +/*
> > + * EBR
> > +00b001b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 87  |................|
> > +00b001c0  07 01 0e a7 26 01 00 08  00 00 00 08 00 00 00 00  |....&...........|
> > +00b001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +00b001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +00b001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +*/
> > +static unsigned char ebr_parts_ref_p5[] = {
> > +                                                0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x87,
> > +0x07, 0x01, 0x0e, 0xa7, 0x26, 0x01, 0x00, 0x08, 0x00, 0x00, 0x00, 0x08, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x55, 0xaa
> > +};
> 
> What do you think about creating these in code? E.g. have a function
> which outputs the bytes for a partition and then call it 1 to 4 times?

I don't follow exact question.
The reason for having these in code was to make sure that all bytes in
partition table are properly programmed. The whole MBR sector is erased with
random data before partition table is written by U-Boot.
The main objective here is to check how partition entries are processed from a
text string. This is where the original bug was found, where it was impossible
to have exactly 4 primary partitions in the MBR. So, the idea of a test was to
create a partition layout as a text string with 1, 2, 3, then 4 primary
partitions, write the layout into test file and then read layout back to make
sure a proper result. Original code would fail attempting to create 4 primary
partitions.

> 
> > +
> > +static void init_buffers(char *rb, size_t rb_size, char *mbr_wb, size_t mbr_wb_size,
> > +                       char *ebr_wb, size_t ebr_wb_size, unsigned seed)
> 
> Needs a comment
> 
> > +{
> > +       memset(rb, 0, rb_size);
> > +       while (mbr_wb_size--) {
> > +               *mbr_wb++ = seed;
> > +               seed *= 43;
> > +               seed += 17 + mbr_wb_size/4;
> > +       }
> > +       while (ebr_wb_size--) {
> > +               *ebr_wb++ = seed;
> > +               seed *= 43;
> > +               seed += 17 + ebr_wb_size/4;
> > +       }
> > +}
> > +
> > +static unsigned build_mbr_parts(char *buf, size_t buf_size, unsigned num_parts)
> > +{
> > +       size_t bytes_remaining, cur_str_size;
> > +       char * cur_buf;
> > +
> > +       if (0 == num_parts || 5 < num_parts || NULL == buf)
> > +               return 1;
> 
> !num_parts
> !buf

Done.

> 
> (we normally put the var before the value, so numparts >= 5)
> 
> > +
> > +       cur_buf = buf;
> > +       *cur_buf = '\0';
> > +       bytes_remaining = buf_size;
> > +
> > +       cur_str_size = sizeof(mbr_parts_header);
> > +       if (1 + cur_str_size > bytes_remaining)
> 
> Again, better to use cur_str_size + 1

Changed the style accordingly for the whole patch.

> 
> > +               return 1;
> > +       strcat(cur_buf, mbr_parts_header);
> > +       bytes_remaining -= cur_str_size;
> > +
> > +       if (1 <= num_parts) {
> > +               cur_str_size = sizeof(mbr_parts_p1);
> > +               if (1 + cur_str_size > bytes_remaining)
> > +                       return 1;
> > +               strcat(cur_buf, mbr_parts_p1);
> > +               bytes_remaining -= cur_str_size;
> > +
> > +               if (2 <= num_parts) {
> > +                       cur_str_size = sizeof(mbr_parts_p2);
> > +                       if (1 + cur_str_size > bytes_remaining)
> > +                               return 1;
> > +                       strcat(cur_buf, mbr_parts_p2);
> > +                       bytes_remaining -= cur_str_size;
> > +
> > +                       if (3 <= num_parts) {
> > +                               cur_str_size = sizeof(mbr_parts_p3);
> > +                               if (1 + cur_str_size > bytes_remaining)
> > +                                       return 1;
> > +                               strcat(cur_buf, mbr_parts_p3);
> > +                               bytes_remaining -= cur_str_size;
> > +
> > +                               if (4 == num_parts) {
> > +                                       cur_str_size = sizeof(mbr_parts_p4);
> > +                                       if (1 + cur_str_size > bytes_remaining)
> > +                                               return 1;
> > +                                       strcat(cur_buf, mbr_parts_p4);
> > +                                       bytes_remaining -= cur_str_size;
> > +
> > +                               }
> > +                       else if (5 == num_parts) {
> > +                               cur_str_size = sizeof(mbr_parts_p5);
> > +                               if (1 + cur_str_size > bytes_remaining)
> > +                                       return 1;
> > +                               strcat(cur_buf, mbr_parts_p5);
> > +                               bytes_remaining -= cur_str_size;
> > +
> > +                       }
> > +                       else if (5 < num_parts)
> > +                               return 1;
> > +                       }
> > +               }
> > +       }
> > +
> > +       cur_str_size = sizeof(mbr_parts_tail);
> > +       if (1 + cur_str_size > bytes_remaining)
> > +               return 1;
> > +       strcat(cur_buf, mbr_parts_tail);
> > +
> > +       return 0;
> > +}
> > +
> > +static int mbr_test_run(struct unit_test_state *uts)
> > +{
> > +       struct blk_desc *mmc_dev_desc;
> > +       unsigned char mbr_wbuf[512], ebr_wbuf[512], rbuf[512];
> > +       char mbr_parts_buf[256];
> > +       ulong mbr_wa, ebr_wa, ra, ebr_blk, mbr_parts_max;
> > +
> > +       mbr_parts_max = 2 + sizeof('\0')
> > +               + strlen(mbr_parts_header)
> > +               + strlen(mbr_parts_p1)
> > +               + strlen(mbr_parts_p2)
> > +               + strlen(mbr_parts_p3)
> > +               + strlen(mbr_parts_p4)
> > +               + strlen(mbr_parts_p5)
> > +               + strlen(mbr_parts_tail);
> > +       ut_assertf(sizeof(mbr_parts_buf) >= mbr_parts_max, "Buffer avail: %ld; buffer req: %ld\n",
> > +               sizeof(mbr_parts_buf), mbr_parts_max);
> > +
> > +       mbr_wa = map_to_sysmem(mbr_wbuf);
> > +       ebr_wa = map_to_sysmem(ebr_wbuf);
> > +       ra = map_to_sysmem(rbuf);
> > +       ebr_blk = (ulong)0xB00000 / 0x200;
> > +
> > +       /* Make sure mmc1 exists */
> > +       ut_asserteq(1, blk_get_device_by_str("mmc", "1", &mmc_dev_desc));
> > +       ut_assertok(console_record_reset_enable());
> 
> You shouldn't need these

The code doesn't work without this line (changed 1 to 6 according to latest
patch):
    ut_asserteq(1, blk_get_device_by_str("mmc", "6", &mmc_dev_desc));
I do not have enough knowledge of U-Boot to figure out why.

> 
> > +       ut_assertok(run_commandf("mmc dev 1"));
> > +       ut_assert_nextline("switch to partitions #0, OK");
> > +       ut_assert_nextline("mmc1 is current device");
> > +       ut_assertok(ut_check_console_end(uts));
> > +
> > +       /* Make sure mmc1 is 12+ MiB in size */
> > +       ut_assertok(run_commandf("mmc read 0x%lx 0x%lx 1", ra, (ulong)0xBFFE00 / 0x200));
> > +
> > +       /* Test one MBR partition */
> > +       init_buffers(rbuf, sizeof(rbuf), mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
> > +       ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 1));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0 1", mbr_wa));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(console_record_reset_enable());
> > +       ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assertok(run_commandf("mbr write mmc 1"));
> > +       ut_assert_nextline("MBR: write success!");
> > +       ut_assertok(run_commandf("mbr verify mmc 1"));
> > +       ut_assert_nextline("MBR: verify success!");
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(ut_check_console_end(uts));
> > +       /*
> > +       000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +       000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 00  |...%$..@........|
> > +       000001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +       000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +       000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       for (unsigned i = 0; i < mbr_cmp_size; i++) {
> > +               ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p1[i],
> > +                       "1P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       mbr_cmp_start + i, mbr_parts_ref_p1[i], rbuf[mbr_cmp_start + i]);
> > +       }
> > +
> > +       /* Test two MBR partitions */
> > +       init_buffers(rbuf, sizeof(rbuf), mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
> > +       ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 2));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0 1", mbr_wa));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(console_record_reset_enable());
> > +       ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assertok(run_commandf("mbr write mmc 1"));
> > +       ut_assert_nextline("MBR: write success!");
> > +       ut_assertok(run_commandf("mbr verify mmc 1"));
> > +       ut_assert_nextline("MBR: verify success!");
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(ut_check_console_end(uts));
> > +       /*
> > +       000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +       000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +       000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 00  |%..F...H........|
> > +       000001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +       000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       for (unsigned i = 0; i < mbr_cmp_size; i++) {
> > +               ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p2[i],
> > +                       "2P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       mbr_cmp_start + i, mbr_parts_ref_p2[i], rbuf[mbr_cmp_start + i]);
> > +       }
> > +
> > +       /* Test three MBR partitions */
> > +       init_buffers(rbuf, sizeof(rbuf), mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
> > +       ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 3));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0 1", mbr_wa));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(console_record_reset_enable());
> > +       ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assertok(run_commandf("mbr write mmc 1"));
> > +       ut_assert_nextline("MBR: write success!");
> > +       ut_assertok(run_commandf("mbr verify mmc 1"));
> > +       ut_assert_nextline("MBR: verify success!");
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(ut_check_console_end(uts));
> > +       /*
> > +       000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +       000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +       000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +       000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 00  |...f%..P........|
> > +       000001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       for (unsigned i = 0; i < mbr_cmp_size; i++) {
> > +               ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p3[i],
> > +                       "3P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       mbr_cmp_start + i, mbr_parts_ref_p3[i], rbuf[mbr_cmp_start + i]);
> > +       }
> > +
> > +       /* Test four MBR partitions */
> > +       init_buffers(rbuf, sizeof(rbuf), mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
> > +       ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 4));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0 1", mbr_wa));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(console_record_reset_enable());
> > +       ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assertok(run_commandf("mbr write mmc 1"));
> > +       ut_assert_nextline("MBR: write success!");
> > +       ut_assertok(run_commandf("mbr verify mmc 1"));
> > +       ut_assert_nextline("MBR: verify success!");
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(ut_check_console_end(uts));
> > +       /*
> > +       000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +       000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +       000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +       000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
> > +       000001f0  26 01 0e 87 06 01 00 58  00 00 00 08 00 00 55 aa  |&......X......U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       for (unsigned i = 0; i < mbr_cmp_size; i++) {
> > +               ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p4[i],
> > +                       "4P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       mbr_cmp_start + i, mbr_parts_ref_p4[i], rbuf[mbr_cmp_start + i]);
> > +       }
> > +
> > +       /* Test five MBR partitions */
> > +       init_buffers(rbuf, sizeof(rbuf), mbr_wbuf, sizeof(mbr_wbuf), ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
> > +       ut_assertok(build_mbr_parts(mbr_parts_buf, sizeof(mbr_parts_buf), 5));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0 1", mbr_wa));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       ut_assertok(memcmp(mbr_wbuf, rbuf, 512));
> > +       ut_assertok(run_commandf("write mmc 1:0 0x%lx 0x%lx 1", ebr_wa, ebr_blk));
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       ut_assertok(memcmp(ebr_wbuf, rbuf, 512));
> > +       ut_assertok(console_record_reset_enable());
> > +       ut_assertf(0 == run_commandf(mbr_parts_buf), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assertf(0 == run_commandf("mbr write mmc 1"), "Invalid partitions string: %s\n", mbr_parts_buf);
> > +       ut_assert_nextline("MBR: write success!");
> > +       ut_assertok(run_commandf("mbr verify mmc 1"));
> > +       ut_assert_nextline("MBR: verify success!");
> > +       ut_assertok(ut_check_console_end(uts));
> > +       /*
> > +       000001b0  00 00 00 00 00 00 00 00  78 56 34 12 00 00 80 05  |........xV4.....|
> > +       000001c0  05 01 0e 25 24 01 00 40  00 00 00 08 00 00 00 25  |...%$..@.......%|
> > +       000001d0  25 01 0e 46 05 01 00 48  00 00 00 08 00 00 00 46  |%..F...H.......F|
> > +       000001e0  06 01 0e 66 25 01 00 50  00 00 00 08 00 00 00 66  |...f%..P.......f|
> > +       000001f0  26 01 05 a7 26 01 00 58  00 00 00 10 00 00 55 aa  |&...&..X......U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0 1", ra));
> > +       for (unsigned i = 0; i < mbr_cmp_size; i++) {
> > +               ut_assertf(rbuf[mbr_cmp_start + i] == mbr_parts_ref_p5[i],
> > +                       "5P MBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       mbr_cmp_start + i, mbr_parts_ref_p5[i], rbuf[mbr_cmp_start + i]);
> > +       }
> > +       /*
> > +       00b001b0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 87  |................|
> > +       00b001c0  07 01 0e a7 26 01 00 08  00 00 00 08 00 00 00 00  |....&...........|
> > +       00b001d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +       00b001e0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
> > +       00b001f0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 55 aa  |..............U.|
> > +       */
> > +       ut_assertok(run_commandf("read mmc 1:0 0x%lx 0x%lx 1", ra, ebr_blk));
> > +       for (unsigned i = 0; i < ebr_cmp_size; i++) {
> > +               ut_assertf(rbuf[ebr_cmp_start + i] == ebr_parts_ref_p5[i],
> > +                       "5P EBR+0x%04X: expected 0x%02X, actual: 0x%02X\n",
> > +                       ebr_cmp_start + i, ebr_parts_ref_p5[i], rbuf[ebr_cmp_start + i]);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/* Declare mbr test */
> > +UNIT_TEST(mbr_test_run, UT_TESTF_CONSOLE_REC, mbr_test);
> > +
> > +int do_ut_mbr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
> > +{
> > +       struct unit_test *tests = UNIT_TEST_SUITE_START(mbr_test);
> > +       const int n_ents = UNIT_TEST_SUITE_COUNT(mbr_test);
> > +
> > +       return cmd_ut_category("mbr", "mbr_test_", tests, n_ents, argc, argv);
> > +}
> > diff --git a/test/cmd_ut.c b/test/cmd_ut.c
> > index 0f56409e80..123db7cc7d 100644
> > --- a/test/cmd_ut.c
> > +++ b/test/cmd_ut.c
> > @@ -86,6 +86,10 @@ static struct cmd_tbl cmd_ut_sub[] = {
> >  #endif
> >  #ifdef CONFIG_UT_LOG
> >         U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_ut_log, "", ""),
> > +#endif
> > +#if defined(CONFIG_SANDBOX) && defined(CONFIG_CMD_MBR) && defined(CONFIG_CMD_MMC) \
> > +        && defined(CONFIG_MMC_SANDBOX) && defined(CONFIG_MMC_WRITE)
> > +       U_BOOT_CMD_MKENT(mbr, CONFIG_SYS_MAXARGS, 1, do_ut_mbr, "", ""),
> >  #endif
> >         U_BOOT_CMD_MKENT(mem, CONFIG_SYS_MAXARGS, 1, do_ut_mem, "", ""),
> >  #if defined(CONFIG_SANDBOX) && defined(CONFIG_CMD_SETEXPR)
> > --
> > 2.41.0
> 
> Regards,
> Simon

    