* [PATCH 0/5] add createnv command to create environment partition
@ 2025-06-02 13:28 Sascha Hauer
2025-06-02 13:28 ` [PATCH 1/5] partitions: efi: calculate instead of hardcode gpt header fields Sascha Hauer
` (4 more replies)
0 siblings, 5 replies; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
We want to move away from describing the barebox environment explicitly
in the device tree and instead motivate usage of GPT partitions for the
envrionment. This series creates a createnv command to facilitate this.
It creates an environment partition on the specified device and if
necessary also a GPT partition table. In the simplest case a "createnv"
without arguments will create a partition on the device barebox itself
booted from. Both the device and the size of the partition can be
specified on the command line.
As the first GPT partition as well as the GPT partition entries might
conflict with a barebox written on the device on certain SoCs this
series also includes patches to move the first partition up to the 8MiB
boundary leaving space for a barebox binary written onto the raw device.
On Some SoCs (i.MX7 and earlier) the GPT partition entries also collide
with the barebox binary, so these are moved up to just below the 8MiB
boundary.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
Sascha Hauer (5):
partitions: efi: calculate instead of hardcode gpt header fields
partitions: Start partitions at 8MiB offset
cdev: fix cdev_open_by_name() misuse
commands: create createnv command
mci: add option to detect non-removable cards during startup
commands/Kconfig | 18 +++++
commands/Makefile | 1 +
commands/createnv.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++
commands/devlookup.c | 4 +-
commands/findmnt.c | 2 +-
commands/parted.c | 2 +-
common/partitions.c | 9 +++
common/partitions/efi.c | 21 ++++--
drivers/mci/Kconfig | 21 +++++-
drivers/mci/mci-core.c | 6 +-
fs/devfs-core.c | 17 ++++-
fs/fs.c | 6 +-
include/driver.h | 5 ++
include/mci.h | 1 +
include/partitions.h | 6 ++
15 files changed, 279 insertions(+), 19 deletions(-)
---
base-commit: c31204804b17f2c07608329a8df6d88e4196cb73
change-id: 20250602-createnv-11c4279116d4
Best regards,
--
Sascha Hauer <s.hauer@pengutronix.de>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] partitions: efi: calculate instead of hardcode gpt header fields
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
@ 2025-06-02 13:28 ` Sascha Hauer
2025-06-02 13:28 ` [PATCH 2/5] partitions: Start partitions at 8MiB offset Sascha Hauer
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
Calculate the lba values of the GPT header instead of hardcoding them.
This will allow us to modify some fields while preserving a consistent
partition table in the next step.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/partitions/efi.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index 840d6a83b7950501debc943864e20e797628720d..92868e4a77e5688d547ee3f2cac3ab9534a4ebdb 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -583,10 +583,13 @@ static __maybe_unused struct partition_desc *efi_partition_create_table(struct b
{
struct efi_partition_desc *epd = xzalloc(sizeof(*epd));
gpt_header *gpt;
+ unsigned int num_partition_entries = 128;
+ unsigned int gpt_size = (sizeof(gpt_entry) * num_partition_entries) / SECTOR_SIZE;
+ unsigned int first_usable_lba = gpt_size + 2;
partition_desc_init(&epd->pd, blk);
- epd->gpt = xzalloc(512);
+ epd->gpt = xzalloc(SECTOR_SIZE);
gpt = epd->gpt;
gpt->signature = cpu_to_le64(GPT_HEADER_SIGNATURE);
@@ -594,18 +597,18 @@ static __maybe_unused struct partition_desc *efi_partition_create_table(struct b
gpt->header_size = cpu_to_le32(sizeof(*gpt));
gpt->my_lba = cpu_to_le64(1);
gpt->alternate_lba = cpu_to_le64(last_lba(blk));
- gpt->first_usable_lba = cpu_to_le64(34);
- gpt->last_usable_lba = cpu_to_le64(last_lba(blk) - 34);;
+ gpt->first_usable_lba = cpu_to_le64(first_usable_lba);
+ gpt->last_usable_lba = cpu_to_le64(last_lba(blk) - (gpt_size + 2));;
generate_random_guid((unsigned char *)&gpt->disk_guid);
- gpt->partition_entry_lba = cpu_to_le64(2);
- gpt->num_partition_entries = cpu_to_le32(128);
+ gpt->partition_entry_lba = cpu_to_le64(first_usable_lba - gpt_size);
+ gpt->num_partition_entries = cpu_to_le32(num_partition_entries);
gpt->sizeof_partition_entry = cpu_to_le32(sizeof(gpt_entry));
add_gpt_diskuuid_param(epd, blk);
pr_info("Created new disk label with GUID %pU\n", &gpt->disk_guid);
- epd->ptes = xzalloc(128 * sizeof(gpt_entry));
+ epd->ptes = xzalloc(num_partition_entries * sizeof(gpt_entry));
return &epd->pd;
}
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] partitions: Start partitions at 8MiB offset
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
2025-06-02 13:28 ` [PATCH 1/5] partitions: efi: calculate instead of hardcode gpt header fields Sascha Hauer
@ 2025-06-02 13:28 ` Sascha Hauer
2025-06-02 22:16 ` Marco Felsch
2025-06-02 13:28 ` [PATCH 3/5] cdev: fix cdev_open_by_name() misuse Sascha Hauer
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
There are many SoCs that store the bootloader on the raw eMMC/SD device
outside of partitions. Examples are all i.MX SoCs and Rockchip SoCs.
Until now the first partition starts at offset 1MiB and with this we
risk conflicting with the bootloader.
It's cumbersome to sort out all SoCs that have this problem. With
nowadays eMMC/SD card sizes it doesn't really matter anymore, so change
the 1MiB offset to 8MiB which is still only a small fraction of the
card size, but leaves enough space for putting the bootloader into.
Note the change in the GPT partition handling introduced with this
change. The layout previously was:
LBA0: MBR
LBA1: GPT header
LBA2-33: GPT partition entries
which now becomes:
LBA0: MBR
LBA1: GPT header
LBA16352-16383: GPT partition entries
This means the GPT partition entries moved from LBA2 to just below the
8MiB boundary and LBA2-16351 are now free for the bootloader. This helps
with with older SoCs on which the bootloader overlaps with the normal
location of the GPT partition entries, like i.MX7 and earlier.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
common/partitions.c | 9 +++++++++
common/partitions/efi.c | 8 ++++++--
include/partitions.h | 6 ++++++
3 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/common/partitions.c b/common/partitions.c
index 25d5f15721fc5980c927863b2745c23d7149da92..b3129ddd5f203c906908adb422e4b8a6dfac25b2 100644
--- a/common/partitions.c
+++ b/common/partitions.c
@@ -186,6 +186,9 @@ int partition_find_free_space(struct partition_desc *pdesc, uint64_t sectors, ui
struct partition *p;
uint64_t min_sec = PARTITION_ALIGN_SECTORS;
+ if (min_sec < partition_first_usable_lba())
+ min_sec = partition_first_usable_lba();
+
min_sec = ALIGN(min_sec, PARTITION_ALIGN_SECTORS);
if (partition_is_free(pdesc, min_sec, sectors)) {
@@ -223,6 +226,12 @@ int partition_create(struct partition_desc *pdesc, const char *name,
return -EINVAL;
}
+ if (lba_start < partition_first_usable_lba()) {
+ pr_err("partition starts before first usable lba: %llu < %llu\n",
+ lba_start, partition_first_usable_lba());
+ return -EINVAL;
+ }
+
list_for_each_entry(part, &pdesc->partitions, list) {
if (region_overlap_end(part->first_sec,
part->first_sec + part->size - 1,
diff --git a/common/partitions/efi.c b/common/partitions/efi.c
index 92868e4a77e5688d547ee3f2cac3ab9534a4ebdb..d237f734df50987b2d8ae41f83edc3e744fcf604 100644
--- a/common/partitions/efi.c
+++ b/common/partitions/efi.c
@@ -585,7 +585,7 @@ static __maybe_unused struct partition_desc *efi_partition_create_table(struct b
gpt_header *gpt;
unsigned int num_partition_entries = 128;
unsigned int gpt_size = (sizeof(gpt_entry) * num_partition_entries) / SECTOR_SIZE;
- unsigned int first_usable_lba = gpt_size + 2;
+ unsigned int first_usable_lba = partition_first_usable_lba();
partition_desc_init(&epd->pd, blk);
@@ -719,7 +719,11 @@ static int efi_protective_mbr(struct block_device *blk)
return PTR_ERR(pdesc);
}
- ret = partition_create(pdesc, "primary", "0xee", 1, last_lba(blk));
+ /*
+ * For the protective MBR call directly into the mkpart hook. partition_create()
+ * does not allow us to create a partition starting at LBA1
+ */
+ ret = pdesc->parser->mkpart(pdesc, "primary", "0xee", 1, last_lba(blk));
if (ret) {
pr_err("Cannot create partition: %pe\n", ERR_PTR(ret));
goto out;
diff --git a/include/partitions.h b/include/partitions.h
index dd3e631c5aa337d752312cd94be5e7dbb4f527a5..6d97af824845eaf8c73a39e42bd4e9ed88a8e367 100644
--- a/include/partitions.h
+++ b/include/partitions.h
@@ -8,6 +8,7 @@
#define __PARTITIONS_PARSER_H__
#include <block.h>
+#include <disks.h>
#include <filetype.h>
#include <linux/uuid.h>
#include <linux/list.h>
@@ -71,4 +72,9 @@ void partition_table_free(struct partition_desc *pdesc);
bool partition_is_free(struct partition_desc *pdesc, uint64_t start, uint64_t size);
int partition_find_free_space(struct partition_desc *pdesc, uint64_t sectors, uint64_t *start);
+static inline uint64_t partition_first_usable_lba(void)
+{
+ return SZ_8M / SECTOR_SIZE;
+}
+
#endif /* __PARTITIONS_PARSER_H__ */
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] cdev: fix cdev_open_by_name() misuse
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
2025-06-02 13:28 ` [PATCH 1/5] partitions: efi: calculate instead of hardcode gpt header fields Sascha Hauer
2025-06-02 13:28 ` [PATCH 2/5] partitions: Start partitions at 8MiB offset Sascha Hauer
@ 2025-06-02 13:28 ` Sascha Hauer
2025-06-02 13:28 ` [PATCH 4/5] commands: create createnv command Sascha Hauer
2025-06-02 13:28 ` [PATCH 5/5] mci: add option to detect non-removable cards during startup Sascha Hauer
4 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
cdev_open_by_name() opens a cdev by its name or path. The open by path
is implemented with a simple skip the "/dev/" part of the string and use
the remaining part as name.
The effect is that several commands in barebox work with "/dev/mmc0" and
"mmc0" as argument, but not with "dev/mmc0" or even "../dev/mmc0".
Create a new cdev_open_by_path_name() function which uses
canonicalize_path() to resolve the full path and skip the "/dev/" part
afterwards.
Convert the places where a path or a name is expected to use the new
function. With this we can drop the "/dev/" skipping from
cdev_open_by_name() so that this function does what the name suggests.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
commands/devlookup.c | 4 +---
commands/findmnt.c | 2 +-
commands/parted.c | 2 +-
fs/devfs-core.c | 17 ++++++++++++++++-
fs/fs.c | 6 +++---
include/driver.h | 5 +++++
6 files changed, 27 insertions(+), 9 deletions(-)
diff --git a/commands/devlookup.c b/commands/devlookup.c
index b7fd7be3b624fa2625a8d6968ba95f76e2c75b7d..8f34687352677c4541685c69229ef321e60d1669 100644
--- a/commands/devlookup.c
+++ b/commands/devlookup.c
@@ -71,9 +71,7 @@ static int do_devlookup(int argc, char *argv[])
devicefile = aliasbuf;
}
- devicefile = devpath_to_name(devicefile);
-
- cdev = cdev_open_by_name(devicefile, O_RDONLY);
+ cdev = cdev_open_by_path_name(devicefile, O_RDONLY);
if (!cdev) {
printf("devlookup: cdev %s not found\n", devicefile);
ret = -ENOENT;
diff --git a/commands/findmnt.c b/commands/findmnt.c
index c64ef8760684ca075458921ddb382d6a4353e009..a531b1a95a835b3f3d5351733c481c58d7df034e 100644
--- a/commands/findmnt.c
+++ b/commands/findmnt.c
@@ -87,7 +87,7 @@ static int do_findmnt(int argc, char *argv[])
const char *backingstore;
struct cdev *cdev;
- cdev = cdev_open_by_name(devpath_to_name(device), O_RDONLY);
+ cdev = cdev_open_by_path_name(device, O_RDONLY);
if (!cdev)
continue;
diff --git a/commands/parted.c b/commands/parted.c
index 6872a8414c79e8e116c9ecce864888cec542ad18..7ec56da4c15fe05bb0c78af86c2a9d708aa53e6b 100644
--- a/commands/parted.c
+++ b/commands/parted.c
@@ -366,7 +366,7 @@ static int do_parted(int argc, char *argv[])
if (argc < 3)
return COMMAND_ERROR_USAGE;
- cdev = cdev_open_by_name(argv[1], O_RDWR);
+ cdev = cdev_open_by_path_name(argv[1], O_RDWR);
if (!cdev) {
printf("Cannot open %s\n", argv[1]);
return COMMAND_ERROR;
diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 5fafcaecc70d63287e0b716cc0133198c28e79e9..1c1666c55382f6e33830f7df46d1027e810d3e18 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -262,7 +262,7 @@ struct cdev *cdev_open_by_name(const char *name, unsigned long flags)
struct cdev *cdev;
int ret;
- cdev = cdev_by_name(devpath_to_name(name));
+ cdev = cdev_by_name(name);
if (!cdev)
return NULL;
@@ -273,6 +273,21 @@ struct cdev *cdev_open_by_name(const char *name, unsigned long flags)
return cdev;
}
+struct cdev *cdev_open_by_path_name(const char *name, unsigned long flags)
+{
+ char *canon = canonicalize_path(AT_FDCWD, name);
+ struct cdev *cdev;
+
+ if (!canon)
+ return cdev_open_by_name(name, flags);
+
+ cdev = cdev_open_by_name(devpath_to_name(canon), flags);
+
+ free(canon);
+
+ return cdev;
+}
+
int cdev_close(struct cdev *cdev)
{
if (cdev->ops->close) {
diff --git a/fs/fs.c b/fs/fs.c
index 465fd617c2baa268ea3e5da784d31436629abd51..bca99b1338c6b05f10c8445599170c2d3331883b 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -910,7 +910,7 @@ const char *fs_detect(const char *filename, const char *fsoptions)
ret = file_name_detect_type_offset(filename, offset, &type,
file_detect_fs_type);
} else {
- struct cdev *cdev = cdev_open_by_name(filename, O_RDONLY);
+ struct cdev *cdev = cdev_open_by_path_name(filename, O_RDONLY);
if (cdev) {
ret = cdev_detect_type(cdev, &type);
cdev_close(cdev);
@@ -955,7 +955,7 @@ int fsdev_open_cdev(struct fs_device *fsdev)
}
}
} else {
- fsdev->cdev = cdev_open_by_name(fsdev->backingstore, O_RDWR);
+ fsdev->cdev = cdev_open_by_path_name(fsdev->backingstore, O_RDWR);
}
if (!fsdev->cdev) {
path_put(&path);
@@ -3197,7 +3197,7 @@ int umount(const char *pathname)
path_put(&path);
if (!fsdev) {
- struct cdev *cdev = cdev_open_by_name(pathname, O_RDWR);
+ struct cdev *cdev = cdev_open_by_path_name(pathname, O_RDWR);
if (cdev) {
cdev_close(cdev);
diff --git a/include/driver.h b/include/driver.h
index b5f4de01e42468ae9245671e130a2285e672f2c0..7606f7d2c241e296ab956b199d4d4ff1bb641a2b 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -540,6 +540,7 @@ ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, ulo
ssize_t cdev_write(struct cdev *cdev, const void *buf, size_t count, loff_t offset, ulong flags);
struct cdev *cdev_by_name(const char *filename);
struct cdev *cdev_open_by_name(const char *name, unsigned long flags);
+struct cdev *cdev_open_by_path_name(const char *name, unsigned long flags);
#else
static inline ssize_t cdev_read(struct cdev *cdev, void *buf, size_t count, loff_t offset, ulong flags)
{
@@ -557,6 +558,10 @@ static inline struct cdev *cdev_open_by_name(const char *name, unsigned long fla
{
return NULL;
}
+static inline struct cdev *cdev_open_by_path_name(const char *name, unsigned long flags)
+{
+ return NULL;
+}
#endif
int cdev_ioctl(struct cdev *cdev, unsigned int cmd, void *buf);
int cdev_erase(struct cdev *cdev, loff_t count, loff_t offset);
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] commands: create createnv command
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
` (2 preceding siblings ...)
2025-06-02 13:28 ` [PATCH 3/5] cdev: fix cdev_open_by_name() misuse Sascha Hauer
@ 2025-06-02 13:28 ` Sascha Hauer
2025-06-02 13:48 ` Ahmad Fatoum
2025-06-02 13:28 ` [PATCH 5/5] mci: add option to detect non-removable cards during startup Sascha Hauer
4 siblings, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
We want to move away from describing the barebox environment explicitly
in the device tree and instead motivate usage of GPT partitions for the
envrionment. This patch creates a createnv command to facilitate this.
It creates an environment partition on the specified device and if
necessary also a GPT partition table. In the simplest case a "createnv"
without arguments will create a partition on the device barebox itself
booted from. Both the device and the size of the partition can be
specified on the command line.
We can't create environment partitions on a device containing a MBR
partition table, so a MBR partitioned device won't be touched.
As an additional safety net the command will ask the user if altering
the partition table is really desired. This behaviour can be overwritten
by specifying -f on the command line.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
commands/Kconfig | 18 ++++++
commands/Makefile | 1 +
commands/createnv.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 198 insertions(+)
diff --git a/commands/Kconfig b/commands/Kconfig
index 0f3155d123ab6eebd6a8b4585ff8c496318c9efe..27a935d4d2e55f0384a28ab1d8718c0cf90fb746 100644
--- a/commands/Kconfig
+++ b/commands/Kconfig
@@ -771,6 +771,24 @@ endmenu
menu "Environment"
+config CMD_CREATENV
+ tristate
+ depends on PARTITION_DISK_EFI
+ select PARTITION_MANIPULATION
+ prompt "createnv"
+ help
+ createnv - Create a barebox environment partition
+
+ Usage: createnv [-sf] [device]
+
+ Create a barebox environment partition.
+ This creates a barebox environment partition and eventually
+ a GPT partition table when it does not exist.
+
+ Options:
+ -s <size> specify partition size (default 1MiB)
+ -f force. Do not ask
+
config CMD_NV
depends on NVVAR
tristate
diff --git a/commands/Makefile b/commands/Makefile
index 8c2749b5ebddb5ef22691d2007f556af195b432b..2780a26dc2daf86ebdc11011fed0849bafaa982a 100644
--- a/commands/Makefile
+++ b/commands/Makefile
@@ -157,6 +157,7 @@ obj-$(CONFIG_CMD_BTHREAD) += bthread.o
obj-$(CONFIG_CMD_UBSAN) += ubsan.o
obj-$(CONFIG_CMD_SELFTEST) += selftest.o
obj-$(CONFIG_CMD_TUTORIAL) += tutorial.o
+obj-$(CONFIG_CMD_CREATENV) += createnv.o
obj-$(CONFIG_CMD_STACKSMASH) += stacksmash.o
obj-$(CONFIG_CMD_PARTED) += parted.o
obj-$(CONFIG_CMD_EFI_HANDLE_DUMP) += efi_handle_dump.o
diff --git a/commands/createnv.c b/commands/createnv.c
new file mode 100644
index 0000000000000000000000000000000000000000..df03664be11251165287bd2291d09bee7408f553
--- /dev/null
+++ b/commands/createnv.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include <command.h>
+#include <malloc.h>
+#include <stdlib.h>
+#include <partitions.h>
+#include <driver.h>
+#include <disks.h>
+#include <xfuncs.h>
+#include <getopt.h>
+#include <efi/partition.h>
+#include <bootsource.h>
+#include <linux/kstrtox.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <envfs.h>
+
+static int do_createnv(int argc, char *argv[])
+{
+ uint64_t start, size = SZ_1M;
+ struct cdev *cdev;
+ struct block_device *blk;
+ struct partition *part;
+ struct partition_desc *pdesc = NULL;
+ enum filetype filetype;
+ guid_t partition_barebox_env_guid = PARTITION_BAREBOX_ENVIRONMENT_GUID;
+ void *buf = NULL;
+ bool force = false;
+ int opt, ret;
+
+ while ((opt = getopt(argc, argv, "s:f")) > 0) {
+ switch (opt) {
+ case 's':
+ size = strtoull_suffix(optarg, NULL, 0);
+ break;
+ case 'f':
+ force = true;
+ break;
+ default:
+ return COMMAND_ERROR_USAGE;
+ }
+ }
+
+ argc -= optind;
+ argv += optind;
+
+ if (argc < 1) {
+ cdev = bootsource_of_cdev_find();
+ if (!cdev) {
+ printf("Cannot find bootsource cdev\n");
+ return COMMAND_ERROR;
+ }
+ ret = cdev_open(cdev, O_RDWR);
+ if (ret) {
+ printf("Cannot open %s: %pe\n", cdev->name, ERR_PTR(ret));
+ return COMMAND_ERROR;
+ }
+ } else {
+ cdev = cdev_open_by_path_name(argv[0], O_RDWR);
+ if (!cdev) {
+ printf("Cannot open %s\n", argv[0]);
+ return COMMAND_ERROR;
+ }
+ }
+
+ blk = cdev_get_block_device(cdev);
+ if (!blk) {
+ ret = -ENOENT;
+ goto err;
+ }
+
+ buf = xzalloc(2 * SECTOR_SIZE);
+
+ ret = cdev_read(cdev, buf, 2 * SECTOR_SIZE, 0, 0);
+ if (ret < 0) {
+ printf("Cannot read from %s: %pe\n", cdev->name, ERR_PTR(ret));
+ goto err;
+ }
+
+ filetype = file_detect_partition_table(buf, 2 * SECTOR_SIZE);
+
+ switch (filetype) {
+ case filetype_gpt:
+ pdesc = partition_table_read(blk);
+ if (!pdesc) {
+ printf("Cannot read partition table\n");
+ ret = -EINVAL;
+ goto err;
+ }
+ break;
+ case filetype_mbr:
+ printf("%s contains a DOS partition table. Cannot create a barebox environment here\n",
+ cdev->name);
+ ret = -EINVAL;
+ goto err;
+ default:
+ printf("Will create a GPT on %s\n", cdev->name);
+ pdesc = partition_table_new(blk, "gpt");
+ break;
+ }
+
+ list_for_each_entry(part, &pdesc->partitions, list) {
+ if (guid_equal(&part->typeuuid, &partition_barebox_env_guid)) {
+ printf("%s already contains a barebox environment partition\n",
+ cdev->name);
+ ret = -EINVAL;
+ goto err;
+ }
+ }
+
+ size >>= SECTOR_SHIFT;
+
+ ret = partition_find_free_space(pdesc, size, &start);
+ if (ret) {
+ printf("No free space available\n");
+ goto err;
+ }
+
+ ret = partition_create(pdesc, "barebox-environment", "bbenv", start, start + size - 1);
+ if (ret) {
+ printf("Creating partition failed: %pe\n", ERR_PTR(ret));
+ goto err;
+ }
+
+ printf("Will create a barebox environment partition of size %llu bytes on %s\n",
+ size << SECTOR_SHIFT, cdev->name);
+
+ if (!force) {
+ char c;
+
+ printf("Do you wish to continue? (y/n)\n");
+
+ c = getchar();
+ if (c != 'y') {
+ ret = -EINTR;
+ goto err;
+ }
+ }
+
+ ret = partition_table_write(pdesc);
+ if (ret) {
+ printf("Cannot write partition table: %pe\n", ERR_PTR(ret));
+ goto err;
+ }
+
+ if (!default_environment_path_get()) {
+ char *path = xasprintf("/dev/%s.barebox-environment", cdev->name);
+ default_environment_path_set(path);
+ free(path);
+ }
+
+ printf("Created barebox environment partition on %s\n", cdev->name);
+
+ ret = 0;
+err:
+ if (pdesc)
+ partition_table_free(pdesc);
+
+ free(buf);
+ cdev_close(cdev);
+
+ return ret ? COMMAND_ERROR : 0;
+}
+
+BAREBOX_CMD_HELP_START(createnv)
+BAREBOX_CMD_HELP_TEXT("Create a barebox environment partition.")
+BAREBOX_CMD_HELP_TEXT("This creates a barebox environment partition and eventually")
+BAREBOX_CMD_HELP_TEXT("a GPT partition table when it does not exist.")
+BAREBOX_CMD_HELP_OPT("-s <size>", "specify partition size (default 1MiB)")
+BAREBOX_CMD_HELP_OPT("-f\t", "force. Do not ask")
+BAREBOX_CMD_HELP_END
+
+BAREBOX_CMD_START(createnv)
+ .cmd = do_createnv,
+ BAREBOX_CMD_DESC("Create a barebox environment partition")
+ BAREBOX_CMD_OPTS("[-sf] [device]")
+ BAREBOX_CMD_GROUP(CMD_GRP_CONSOLE)
+ BAREBOX_CMD_HELP(cmd_createnv_help)
+BAREBOX_CMD_END
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] mci: add option to detect non-removable cards during startup
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
` (3 preceding siblings ...)
2025-06-02 13:28 ` [PATCH 4/5] commands: create createnv command Sascha Hauer
@ 2025-06-02 13:28 ` Sascha Hauer
2025-06-02 13:42 ` Ahmad Fatoum
4 siblings, 1 reply; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 13:28 UTC (permalink / raw)
To: BAREBOX
We can optionally detect all MMC/SD cards during startup which might
cost some boot time, so this is best disabled. This adds another option
to detect only cards that are marked non-removable in the device tree
during startup. The rationale is that we will likely boot from that
device anyway, so detecting it during startup does not add to the boot
time.
We are aiming to find a good place for the barebox environment without
having to explicitly add it to the device tree. Putting the environment
into a regular partition is a good start, but the device that has the
environment needs to be available when we are searching for it. Enabling
this new option is one way to it.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
drivers/mci/Kconfig | 21 +++++++++++++++++++--
drivers/mci/mci-core.c | 6 +++++-
include/mci.h | 1 +
3 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/mci/Kconfig b/drivers/mci/Kconfig
index 44fd4716aade0882264d389cc08547c3f14986c9..417309cdf502cdf1c2b7e584c886dd94661bb0fb 100644
--- a/drivers/mci/Kconfig
+++ b/drivers/mci/Kconfig
@@ -19,14 +19,31 @@ config MCI_TUNING
higher clock speeds than 52 MHz SDR. MMC only; SD-Card max
frequency is 50MHz SDR at present.
+choice
+ prompt "MMC startup behaviour"
+
config MCI_STARTUP
- bool "Force probe on system start"
+ bool "detect cards on system start"
help
- Say 'y' here if the MCI framework should always probe for all attached
+ Choose this if the MCI framework should always probe for all attached
MCI cards on system start up. This may required for some legacy boards.
When this is 'n', probing happens on demand either with "mci*.probe=1"
or with driver/board code calling device_detect.
+config MCI_STARTUP_NONREMOVABLE
+ bool "detect non removable cards on system start"
+ help
+ Choose this if the MCI framework should detect non removable cards (i.e.
+ devices which have the non-removable device tree property).
+
+config MCI_STARTUP_NONE
+ bool "detect not detect cards during system start"
+ help
+ Choose this if the MCI framework should never detect cards during system
+ start
+
+endchoice
+
config MCI_INFO
bool "MCI Info"
depends on CMD_DEVINFO
diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
index 23cde58c0c5b1fae4e3692c9ac0989362376d06d..3984852a1d8d21226947aa59bc73a68e79f11d8c 100644
--- a/drivers/mci/mci-core.c
+++ b/drivers/mci/mci-core.c
@@ -3101,7 +3101,9 @@ int mci_register(struct mci_host *host)
devinfo_add(&mci->dev, mci_info);
/* if enabled, probe the attached card immediately */
- if (IS_ENABLED(CONFIG_MCI_STARTUP))
+ if (IS_ENABLED(CONFIG_MCI_STARTUP) ||
+ (IS_ENABLED(CONFIG_MCI_STARTUP_NONREMOVABLE) &&
+ (host->host_caps & MMC_CAP_NONREMOVABLE)))
mci_card_probe(mci);
if (!(host->caps2 & MMC_CAP2_NO_SD) && dev_of_node(host->hw_dev)) {
@@ -3195,6 +3197,8 @@ void mci_of_parse_node(struct mci_host *host,
host->caps2 |= MMC_CAP2_NO_SD;
if (of_property_read_bool(np, "no-mmc"))
host->caps2 |= MMC_CAP2_NO_MMC;
+ if (of_property_read_bool(np, "non-removable"))
+ host->host_caps |= MMC_CAP_NONREMOVABLE;
if (IS_ENABLED(CONFIG_MCI_TUNING)) {
u32 drv_type;
diff --git a/include/mci.h b/include/mci.h
index 207fb9779a14495bdbed782c6d18245d95077055..05114fd72e6cc08025e86fe99021ff041e02101c 100644
--- a/include/mci.h
+++ b/include/mci.h
@@ -58,6 +58,7 @@
#define MMC_CAP_1_2V_DDR (1 << 9) /* Host supports eMMC DDR 1.2V */
#define MMC_CAP_DDR (MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
MMC_CAP_1_2V_DDR)
+#define MMC_CAP_NONREMOVABLE (1 << 8) /* Nonremovable e.g. eMMC */
#define MMC_CAP_UHS_SDR12 (1 << 16) /* Host supports UHS SDR12 mode */
#define MMC_CAP_UHS_SDR25 (1 << 17) /* Host supports UHS SDR25 mode */
#define MMC_CAP_UHS_SDR50 (1 << 18) /* Host supports UHS SDR50 mode */
--
2.39.5
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] mci: add option to detect non-removable cards during startup
2025-06-02 13:28 ` [PATCH 5/5] mci: add option to detect non-removable cards during startup Sascha Hauer
@ 2025-06-02 13:42 ` Ahmad Fatoum
0 siblings, 0 replies; 11+ messages in thread
From: Ahmad Fatoum @ 2025-06-02 13:42 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
Hi,
On 02.06.25 15:28, Sascha Hauer wrote:
> We can optionally detect all MMC/SD cards during startup which might
> cost some boot time, so this is best disabled. This adds another option
> to detect only cards that are marked non-removable in the device tree
> during startup. The rationale is that we will likely boot from that
> device anyway, so detecting it during startup does not add to the boot
> time.
>
> We are aiming to find a good place for the barebox environment without
> having to explicitly add it to the device tree. Putting the environment
> into a regular partition is a good start, but the device that has the
> environment needs to be available when we are searching for it. Enabling
> this new option is one way to it.
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> drivers/mci/Kconfig | 21 +++++++++++++++++++--
> drivers/mci/mci-core.c | 6 +++++-
> include/mci.h | 1 +
> 3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mci/Kconfig b/drivers/mci/Kconfig
> index 44fd4716aade0882264d389cc08547c3f14986c9..417309cdf502cdf1c2b7e584c886dd94661bb0fb 100644
> --- a/drivers/mci/Kconfig
> +++ b/drivers/mci/Kconfig
> @@ -19,14 +19,31 @@ config MCI_TUNING
> higher clock speeds than 52 MHz SDR. MMC only; SD-Card max
> frequency is 50MHz SDR at present.
>
> +choice
> + prompt "MMC startup behaviour"
"MMC detection on startup"?
Does not having a default makes MCI_STARTUP the default now whereas before it was
default n? I think MCI_STARTUP_NONE should be the default instead and
MCI_STARTUP_NONREMOVABLE is what we enable in defconfigs that don't already
have MCI_STARTUP.
> +
> config MCI_STARTUP
> - bool "Force probe on system start"
> + bool "detect cards on system start"
detect all cards on system start
> help
> - Say 'y' here if the MCI framework should always probe for all attached
> + Choose this if the MCI framework should always probe for all attached
> MCI cards on system start up. This may required for some legacy boards.
> When this is 'n', probing happens on demand either with "mci*.probe=1"
> or with driver/board code calling device_detect.
>
> +config MCI_STARTUP_NONREMOVABLE
> + bool "detect non removable cards on system start"
> + help
> + Choose this if the MCI framework should detect non removable cards (i.e.
> + devices which have the non-removable device tree property).
> +
> +config MCI_STARTUP_NONE
> + bool "detect not detect cards during system start"
do not detect cards during system start
> + help
> + Choose this if the MCI framework should never detect cards during system
> + start
should not detect cards on its own during system start. If this is enabled, cards
will need to be explicitly detected by the user by calling detect or by explicit
reference to the device.
> +
> +endchoice
> +
> config MCI_INFO
> bool "MCI Info"
> depends on CMD_DEVINFO
> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
> index 23cde58c0c5b1fae4e3692c9ac0989362376d06d..3984852a1d8d21226947aa59bc73a68e79f11d8c 100644
> --- a/drivers/mci/mci-core.c
> +++ b/drivers/mci/mci-core.c
> @@ -3101,7 +3101,9 @@ int mci_register(struct mci_host *host)
> devinfo_add(&mci->dev, mci_info);
>
> /* if enabled, probe the attached card immediately */
> - if (IS_ENABLED(CONFIG_MCI_STARTUP))
> + if (IS_ENABLED(CONFIG_MCI_STARTUP) ||
> + (IS_ENABLED(CONFIG_MCI_STARTUP_NONREMOVABLE) &&
> + (host->host_caps & MMC_CAP_NONREMOVABLE)))
> mci_card_probe(mci);
>
> if (!(host->caps2 & MMC_CAP2_NO_SD) && dev_of_node(host->hw_dev)) {
> @@ -3195,6 +3197,8 @@ void mci_of_parse_node(struct mci_host *host,
> host->caps2 |= MMC_CAP2_NO_SD;
> if (of_property_read_bool(np, "no-mmc"))
> host->caps2 |= MMC_CAP2_NO_MMC;
> + if (of_property_read_bool(np, "non-removable"))
> + host->host_caps |= MMC_CAP_NONREMOVABLE;
We already have struct mci_host::non-removable. I don't mind copying the Linux flag,
but we should then drop the duplication.
> if (IS_ENABLED(CONFIG_MCI_TUNING)) {
> u32 drv_type;
>
> diff --git a/include/mci.h b/include/mci.h
> index 207fb9779a14495bdbed782c6d18245d95077055..05114fd72e6cc08025e86fe99021ff041e02101c 100644
> --- a/include/mci.h
> +++ b/include/mci.h
> @@ -58,6 +58,7 @@
> #define MMC_CAP_1_2V_DDR (1 << 9) /* Host supports eMMC DDR 1.2V */
> #define MMC_CAP_DDR (MMC_CAP_3_3V_DDR | MMC_CAP_1_8V_DDR | \
> MMC_CAP_1_2V_DDR)
> +#define MMC_CAP_NONREMOVABLE (1 << 8) /* Nonremovable e.g. eMMC */
> #define MMC_CAP_UHS_SDR12 (1 << 16) /* Host supports UHS SDR12 mode */
> #define MMC_CAP_UHS_SDR25 (1 << 17) /* Host supports UHS SDR25 mode */
> #define MMC_CAP_UHS_SDR50 (1 << 18) /* Host supports UHS SDR50 mode */
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] commands: create createnv command
2025-06-02 13:28 ` [PATCH 4/5] commands: create createnv command Sascha Hauer
@ 2025-06-02 13:48 ` Ahmad Fatoum
2025-06-02 14:50 ` Sascha Hauer
0 siblings, 1 reply; 11+ messages in thread
From: Ahmad Fatoum @ 2025-06-02 13:48 UTC (permalink / raw)
To: Sascha Hauer, BAREBOX
Hi,
On 02.06.25 15:28, Sascha Hauer wrote:
> We want to move away from describing the barebox environment explicitly
> in the device tree and instead motivate usage of GPT partitions for the
> envrionment. This patch creates a createnv command to facilitate this.
> It creates an environment partition on the specified device and if
> necessary also a GPT partition table. In the simplest case a "createnv"
> without arguments will create a partition on the device barebox itself
> booted from. Both the device and the size of the partition can be
> specified on the command line.
>
> We can't create environment partitions on a device containing a MBR
> partition table, so a MBR partitioned device won't be touched.
>
> As an additional safety net the command will ask the user if altering
> the partition table is really desired. This behaviour can be overwritten
> by specifying -f on the command line.
Could we have saveenv/loadenv nudge the user towards createnv if CMD_CREATENV
is enabled?
Something along the lines of:
saveenv: no environment was registered
saveenv: run createnv to create an ad-hoc environment
Cheers,
Ahmad
>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> commands/Kconfig | 18 ++++++
> commands/Makefile | 1 +
> commands/createnv.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 198 insertions(+)
>
> diff --git a/commands/Kconfig b/commands/Kconfig
> index 0f3155d123ab6eebd6a8b4585ff8c496318c9efe..27a935d4d2e55f0384a28ab1d8718c0cf90fb746 100644
> --- a/commands/Kconfig
> +++ b/commands/Kconfig
> @@ -771,6 +771,24 @@ endmenu
>
> menu "Environment"
>
> +config CMD_CREATENV
> + tristate
> + depends on PARTITION_DISK_EFI
> + select PARTITION_MANIPULATION
> + prompt "createnv"
> + help
> + createnv - Create a barebox environment partition
> +
> + Usage: createnv [-sf] [device]
> +
> + Create a barebox environment partition.
> + This creates a barebox environment partition and eventually
> + a GPT partition table when it does not exist.
> +
> + Options:
> + -s <size> specify partition size (default 1MiB)
> + -f force. Do not ask
> +
> config CMD_NV
> depends on NVVAR
> tristate
> diff --git a/commands/Makefile b/commands/Makefile
> index 8c2749b5ebddb5ef22691d2007f556af195b432b..2780a26dc2daf86ebdc11011fed0849bafaa982a 100644
> --- a/commands/Makefile
> +++ b/commands/Makefile
> @@ -157,6 +157,7 @@ obj-$(CONFIG_CMD_BTHREAD) += bthread.o
> obj-$(CONFIG_CMD_UBSAN) += ubsan.o
> obj-$(CONFIG_CMD_SELFTEST) += selftest.o
> obj-$(CONFIG_CMD_TUTORIAL) += tutorial.o
> +obj-$(CONFIG_CMD_CREATENV) += createnv.o
> obj-$(CONFIG_CMD_STACKSMASH) += stacksmash.o
> obj-$(CONFIG_CMD_PARTED) += parted.o
> obj-$(CONFIG_CMD_EFI_HANDLE_DUMP) += efi_handle_dump.o
> diff --git a/commands/createnv.c b/commands/createnv.c
> new file mode 100644
> index 0000000000000000000000000000000000000000..df03664be11251165287bd2291d09bee7408f553
> --- /dev/null
> +++ b/commands/createnv.c
> @@ -0,0 +1,179 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <command.h>
> +#include <malloc.h>
> +#include <stdlib.h>
> +#include <partitions.h>
> +#include <driver.h>
> +#include <disks.h>
> +#include <xfuncs.h>
> +#include <getopt.h>
> +#include <efi/partition.h>
> +#include <bootsource.h>
> +#include <linux/kstrtox.h>
> +#include <fcntl.h>
> +#include <stdio.h>
> +#include <envfs.h>
> +
> +static int do_createnv(int argc, char *argv[])
> +{
> + uint64_t start, size = SZ_1M;
> + struct cdev *cdev;
> + struct block_device *blk;
> + struct partition *part;
> + struct partition_desc *pdesc = NULL;
> + enum filetype filetype;
> + guid_t partition_barebox_env_guid = PARTITION_BAREBOX_ENVIRONMENT_GUID;
> + void *buf = NULL;
> + bool force = false;
> + int opt, ret;
> +
> + while ((opt = getopt(argc, argv, "s:f")) > 0) {
> + switch (opt) {
> + case 's':
> + size = strtoull_suffix(optarg, NULL, 0);
> + break;
> + case 'f':
> + force = true;
> + break;
> + default:
> + return COMMAND_ERROR_USAGE;
> + }
> + }
> +
> + argc -= optind;
> + argv += optind;
> +
> + if (argc < 1) {
> + cdev = bootsource_of_cdev_find();
> + if (!cdev) {
> + printf("Cannot find bootsource cdev\n");
> + return COMMAND_ERROR;
> + }
> + ret = cdev_open(cdev, O_RDWR);
> + if (ret) {
> + printf("Cannot open %s: %pe\n", cdev->name, ERR_PTR(ret));
> + return COMMAND_ERROR;
> + }
> + } else {
> + cdev = cdev_open_by_path_name(argv[0], O_RDWR);
> + if (!cdev) {
> + printf("Cannot open %s\n", argv[0]);
> + return COMMAND_ERROR;
> + }
> + }
> +
> + blk = cdev_get_block_device(cdev);
> + if (!blk) {
> + ret = -ENOENT;
> + goto err;
> + }
> +
> + buf = xzalloc(2 * SECTOR_SIZE);
> +
> + ret = cdev_read(cdev, buf, 2 * SECTOR_SIZE, 0, 0);
> + if (ret < 0) {
> + printf("Cannot read from %s: %pe\n", cdev->name, ERR_PTR(ret));
> + goto err;
> + }
> +
> + filetype = file_detect_partition_table(buf, 2 * SECTOR_SIZE);
> +
> + switch (filetype) {
> + case filetype_gpt:
> + pdesc = partition_table_read(blk);
> + if (!pdesc) {
> + printf("Cannot read partition table\n");
> + ret = -EINVAL;
> + goto err;
> + }
> + break;
> + case filetype_mbr:
> + printf("%s contains a DOS partition table. Cannot create a barebox environment here\n",
> + cdev->name);
> + ret = -EINVAL;
> + goto err;
> + default:
> + printf("Will create a GPT on %s\n", cdev->name);
> + pdesc = partition_table_new(blk, "gpt");
> + break;
> + }
> +
> + list_for_each_entry(part, &pdesc->partitions, list) {
> + if (guid_equal(&part->typeuuid, &partition_barebox_env_guid)) {
> + printf("%s already contains a barebox environment partition\n",
> + cdev->name);
> + ret = -EINVAL;
> + goto err;
> + }
> + }
> +
> + size >>= SECTOR_SHIFT;
> +
> + ret = partition_find_free_space(pdesc, size, &start);
> + if (ret) {
> + printf("No free space available\n");
> + goto err;
> + }
> +
> + ret = partition_create(pdesc, "barebox-environment", "bbenv", start, start + size - 1);
> + if (ret) {
> + printf("Creating partition failed: %pe\n", ERR_PTR(ret));
> + goto err;
> + }
> +
> + printf("Will create a barebox environment partition of size %llu bytes on %s\n",
> + size << SECTOR_SHIFT, cdev->name);
> +
> + if (!force) {
> + char c;
> +
> + printf("Do you wish to continue? (y/n)\n");
> +
> + c = getchar();
> + if (c != 'y') {
> + ret = -EINTR;
> + goto err;
> + }
> + }
> +
> + ret = partition_table_write(pdesc);
> + if (ret) {
> + printf("Cannot write partition table: %pe\n", ERR_PTR(ret));
> + goto err;
> + }
> +
> + if (!default_environment_path_get()) {
> + char *path = xasprintf("/dev/%s.barebox-environment", cdev->name);
> + default_environment_path_set(path);
> + free(path);
> + }
> +
> + printf("Created barebox environment partition on %s\n", cdev->name);
> +
> + ret = 0;
> +err:
> + if (pdesc)
> + partition_table_free(pdesc);
> +
> + free(buf);
> + cdev_close(cdev);
> +
> + return ret ? COMMAND_ERROR : 0;
> +}
> +
> +BAREBOX_CMD_HELP_START(createnv)
> +BAREBOX_CMD_HELP_TEXT("Create a barebox environment partition.")
> +BAREBOX_CMD_HELP_TEXT("This creates a barebox environment partition and eventually")
> +BAREBOX_CMD_HELP_TEXT("a GPT partition table when it does not exist.")
> +BAREBOX_CMD_HELP_OPT("-s <size>", "specify partition size (default 1MiB)")
> +BAREBOX_CMD_HELP_OPT("-f\t", "force. Do not ask")
> +BAREBOX_CMD_HELP_END
> +
> +BAREBOX_CMD_START(createnv)
> + .cmd = do_createnv,
> + BAREBOX_CMD_DESC("Create a barebox environment partition")
> + BAREBOX_CMD_OPTS("[-sf] [device]")
> + BAREBOX_CMD_GROUP(CMD_GRP_CONSOLE)
> + BAREBOX_CMD_HELP(cmd_createnv_help)
> +BAREBOX_CMD_END
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 4/5] commands: create createnv command
2025-06-02 13:48 ` Ahmad Fatoum
@ 2025-06-02 14:50 ` Sascha Hauer
0 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2025-06-02 14:50 UTC (permalink / raw)
To: Ahmad Fatoum; +Cc: BAREBOX
On Mon, Jun 02, 2025 at 03:48:26PM +0200, Ahmad Fatoum wrote:
> Hi,
>
> On 02.06.25 15:28, Sascha Hauer wrote:
> > We want to move away from describing the barebox environment explicitly
> > in the device tree and instead motivate usage of GPT partitions for the
> > envrionment. This patch creates a createnv command to facilitate this.
> > It creates an environment partition on the specified device and if
> > necessary also a GPT partition table. In the simplest case a "createnv"
> > without arguments will create a partition on the device barebox itself
> > booted from. Both the device and the size of the partition can be
> > specified on the command line.
> >
> > We can't create environment partitions on a device containing a MBR
> > partition table, so a MBR partitioned device won't be touched.
> >
> > As an additional safety net the command will ask the user if altering
> > the partition table is really desired. This behaviour can be overwritten
> > by specifying -f on the command line.
>
> Could we have saveenv/loadenv nudge the user towards createnv if CMD_CREATENV
> is enabled?
>
> Something along the lines of:
>
> saveenv: no environment was registered
> saveenv: run createnv to create an ad-hoc environment
I was more thinking towards issuing a message like that during startup,
because by then we know already that we don't have an environment
partition. The corresponding patch is not yet ready though, so I sent
without it.
Do you see any advantages putting the message into the saveenv command?
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] partitions: Start partitions at 8MiB offset
2025-06-02 13:28 ` [PATCH 2/5] partitions: Start partitions at 8MiB offset Sascha Hauer
@ 2025-06-02 22:16 ` Marco Felsch
2025-06-03 7:46 ` Sascha Hauer
0 siblings, 1 reply; 11+ messages in thread
From: Marco Felsch @ 2025-06-02 22:16 UTC (permalink / raw)
To: Sascha Hauer; +Cc: BAREBOX
Hi Sascha,
On 25-06-02, Sascha Hauer wrote:
> There are many SoCs that store the bootloader on the raw eMMC/SD device
> outside of partitions. Examples are all i.MX SoCs and Rockchip SoCs.
> Until now the first partition starts at offset 1MiB and with this we
> risk conflicting with the bootloader.
>
> It's cumbersome to sort out all SoCs that have this problem. With
> nowadays eMMC/SD card sizes it doesn't really matter anymore, so change
> the 1MiB offset to 8MiB which is still only a small fraction of the
> card size, but leaves enough space for putting the bootloader into.
>
> Note the change in the GPT partition handling introduced with this
> change. The layout previously was:
>
> LBA0: MBR
> LBA1: GPT header
> LBA2-33: GPT partition entries
>
> which now becomes:
>
> LBA0: MBR
> LBA1: GPT header
> LBA16352-16383: GPT partition entries
>
> This means the GPT partition entries moved from LBA2 to just below the
> 8MiB boundary and LBA2-16351 are now free for the bootloader. This helps
> with with older SoCs on which the bootloader overlaps with the normal
> location of the GPT partition entries, like i.MX7 and earlier.
I know that eMMC became quite large these days but always 'wasting' 8MiB
on systems which utilize the eMMC-boot partitions is not very user
friendly.
I liked the approach which I posted which gives the user the ability to
decide how much space is required if any.
Regards,
Marco
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
> common/partitions.c | 9 +++++++++
> common/partitions/efi.c | 8 ++++++--
> include/partitions.h | 6 ++++++
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/common/partitions.c b/common/partitions.c
> index 25d5f15721fc5980c927863b2745c23d7149da92..b3129ddd5f203c906908adb422e4b8a6dfac25b2 100644
> --- a/common/partitions.c
> +++ b/common/partitions.c
> @@ -186,6 +186,9 @@ int partition_find_free_space(struct partition_desc *pdesc, uint64_t sectors, ui
> struct partition *p;
> uint64_t min_sec = PARTITION_ALIGN_SECTORS;
>
> + if (min_sec < partition_first_usable_lba())
> + min_sec = partition_first_usable_lba();
> +
> min_sec = ALIGN(min_sec, PARTITION_ALIGN_SECTORS);
>
> if (partition_is_free(pdesc, min_sec, sectors)) {
> @@ -223,6 +226,12 @@ int partition_create(struct partition_desc *pdesc, const char *name,
> return -EINVAL;
> }
>
> + if (lba_start < partition_first_usable_lba()) {
> + pr_err("partition starts before first usable lba: %llu < %llu\n",
> + lba_start, partition_first_usable_lba());
> + return -EINVAL;
> + }
> +
> list_for_each_entry(part, &pdesc->partitions, list) {
> if (region_overlap_end(part->first_sec,
> part->first_sec + part->size - 1,
> diff --git a/common/partitions/efi.c b/common/partitions/efi.c
> index 92868e4a77e5688d547ee3f2cac3ab9534a4ebdb..d237f734df50987b2d8ae41f83edc3e744fcf604 100644
> --- a/common/partitions/efi.c
> +++ b/common/partitions/efi.c
> @@ -585,7 +585,7 @@ static __maybe_unused struct partition_desc *efi_partition_create_table(struct b
> gpt_header *gpt;
> unsigned int num_partition_entries = 128;
> unsigned int gpt_size = (sizeof(gpt_entry) * num_partition_entries) / SECTOR_SIZE;
> - unsigned int first_usable_lba = gpt_size + 2;
> + unsigned int first_usable_lba = partition_first_usable_lba();
>
> partition_desc_init(&epd->pd, blk);
>
> @@ -719,7 +719,11 @@ static int efi_protective_mbr(struct block_device *blk)
> return PTR_ERR(pdesc);
> }
>
> - ret = partition_create(pdesc, "primary", "0xee", 1, last_lba(blk));
> + /*
> + * For the protective MBR call directly into the mkpart hook. partition_create()
> + * does not allow us to create a partition starting at LBA1
> + */
> + ret = pdesc->parser->mkpart(pdesc, "primary", "0xee", 1, last_lba(blk));
> if (ret) {
> pr_err("Cannot create partition: %pe\n", ERR_PTR(ret));
> goto out;
> diff --git a/include/partitions.h b/include/partitions.h
> index dd3e631c5aa337d752312cd94be5e7dbb4f527a5..6d97af824845eaf8c73a39e42bd4e9ed88a8e367 100644
> --- a/include/partitions.h
> +++ b/include/partitions.h
> @@ -8,6 +8,7 @@
> #define __PARTITIONS_PARSER_H__
>
> #include <block.h>
> +#include <disks.h>
> #include <filetype.h>
> #include <linux/uuid.h>
> #include <linux/list.h>
> @@ -71,4 +72,9 @@ void partition_table_free(struct partition_desc *pdesc);
> bool partition_is_free(struct partition_desc *pdesc, uint64_t start, uint64_t size);
> int partition_find_free_space(struct partition_desc *pdesc, uint64_t sectors, uint64_t *start);
>
> +static inline uint64_t partition_first_usable_lba(void)
> +{
> + return SZ_8M / SECTOR_SIZE;
> +}
> +
> #endif /* __PARTITIONS_PARSER_H__ */
>
> --
> 2.39.5
>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] partitions: Start partitions at 8MiB offset
2025-06-02 22:16 ` Marco Felsch
@ 2025-06-03 7:46 ` Sascha Hauer
0 siblings, 0 replies; 11+ messages in thread
From: Sascha Hauer @ 2025-06-03 7:46 UTC (permalink / raw)
To: Marco Felsch; +Cc: BAREBOX
On Tue, Jun 03, 2025 at 12:16:33AM +0200, Marco Felsch wrote:
> Hi Sascha,
>
> On 25-06-02, Sascha Hauer wrote:
> > There are many SoCs that store the bootloader on the raw eMMC/SD device
> > outside of partitions. Examples are all i.MX SoCs and Rockchip SoCs.
> > Until now the first partition starts at offset 1MiB and with this we
> > risk conflicting with the bootloader.
> >
> > It's cumbersome to sort out all SoCs that have this problem. With
> > nowadays eMMC/SD card sizes it doesn't really matter anymore, so change
> > the 1MiB offset to 8MiB which is still only a small fraction of the
> > card size, but leaves enough space for putting the bootloader into.
> >
> > Note the change in the GPT partition handling introduced with this
> > change. The layout previously was:
> >
> > LBA0: MBR
> > LBA1: GPT header
> > LBA2-33: GPT partition entries
> >
> > which now becomes:
> >
> > LBA0: MBR
> > LBA1: GPT header
> > LBA16352-16383: GPT partition entries
> >
> > This means the GPT partition entries moved from LBA2 to just below the
> > 8MiB boundary and LBA2-16351 are now free for the bootloader. This helps
> > with with older SoCs on which the bootloader overlaps with the normal
> > location of the GPT partition entries, like i.MX7 and earlier.
>
> I know that eMMC became quite large these days but always 'wasting' 8MiB
> on systems which utilize the eMMC-boot partitions is not very user
> friendly.
That's 0.78% of 1GiB SD card, it's really not much. Anyway, I can add a
command line option for parted.
Sascha
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-06-03 7:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-02 13:28 [PATCH 0/5] add createnv command to create environment partition Sascha Hauer
2025-06-02 13:28 ` [PATCH 1/5] partitions: efi: calculate instead of hardcode gpt header fields Sascha Hauer
2025-06-02 13:28 ` [PATCH 2/5] partitions: Start partitions at 8MiB offset Sascha Hauer
2025-06-02 22:16 ` Marco Felsch
2025-06-03 7:46 ` Sascha Hauer
2025-06-02 13:28 ` [PATCH 3/5] cdev: fix cdev_open_by_name() misuse Sascha Hauer
2025-06-02 13:28 ` [PATCH 4/5] commands: create createnv command Sascha Hauer
2025-06-02 13:48 ` Ahmad Fatoum
2025-06-02 14:50 ` Sascha Hauer
2025-06-02 13:28 ` [PATCH 5/5] mci: add option to detect non-removable cards during startup Sascha Hauer
2025-06-02 13:42 ` Ahmad Fatoum
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox