[PATCH v2 1/4] fs: qemu_fw_cfg: make use of the automount

[PATCH v2 1/4] fs: qemu_fw_cfg: make use of the automount

[Ahmad Fatoum]

We assume /mnt/fw_cfg to be the fw_cfg mount path at other places
already, so let's drop some complexity by just using the automount.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - new preparatory patch
---
 fs/devfs-core.c  |  6 ++++++
 fs/qemu_fw_cfg.c | 27 +++++----------------------
 include/driver.h |  2 ++
 3 files changed, 13 insertions(+), 22 deletions(-)

diff --git a/fs/devfs-core.c b/fs/devfs-core.c
index 92e7c5823ed0..4f8230e124eb 100644
--- a/fs/devfs-core.c
+++ b/fs/devfs-core.c
@@ -86,6 +86,12 @@ struct cdev *cdev_by_name(const char *filename)
 	return cdev_readlink(cdev);
 }
 
+struct device *device_find_by_file_path(const char *filename)
+{
+	struct cdev *cdev = cdev_by_name(devpath_to_name(filename));
+	return cdev ? cdev->dev : NULL;
+}
+
 struct cdev *cdev_by_device_node(struct device_node *node)
 {
 	struct cdev *cdev;
diff --git a/fs/qemu_fw_cfg.c b/fs/qemu_fw_cfg.c
index 8e61df1ebf22..5aaf14267f88 100644
--- a/fs/qemu_fw_cfg.c
+++ b/fs/qemu_fw_cfg.c
@@ -410,32 +410,15 @@ coredevice_initcall(qemu_fw_cfg_fs_init);
 
 static int qemu_fw_cfg_early_mount(void)
 {
-	struct cdev *cdev;
-	struct device *dev;
-	const char *mntpath;
-	int dirfd, fd;
-
-	cdev = cdev_by_name("fw_cfg");
-	if (!cdev)
-		return 0;
+	int fd;
 
 	/*
-	 * Trigger a mount, so ramfb device can be detected and
-	 * environment can be loaded
+	 * Also triggers the automount, so ramfb device can be detected
 	 */
-	mntpath = cdev_mount(cdev);
-	if (IS_ERR(mntpath))
-		return PTR_ERR(mntpath);
-
-	dirfd = open(mntpath, O_PATH | O_DIRECTORY);
-	if (dirfd < 0)
-		return dirfd;
-
-	fd = openat(dirfd, "by_name/etc/ramfb", O_WRONLY);
-	close(dirfd);
+	fd = open("/mnt/fw_cfg/by_name/etc/ramfb", O_WRONLY);
 	if (fd >= 0) {
-		dev = device_alloc("qemu-ramfb", DEVICE_ID_SINGLE);
-		dev->parent = cdev->dev;
+		struct device *dev = device_alloc("qemu-ramfb", DEVICE_ID_SINGLE);
+		dev->parent = device_find_by_file_path("/dev/fw_cfg");
 		dev->platform_data = (void *)(uintptr_t)fd;
 		platform_device_register(dev);
 	}
diff --git a/include/driver.h b/include/driver.h
index c130a3cd63fd..143ca18e4e39 100644
--- a/include/driver.h
+++ b/include/driver.h
@@ -477,6 +477,8 @@ int cdev_fdopen(struct cdev *cdev, unsigned long flags);
 int cdev_close(struct cdev *cdev);
 int cdev_flush(struct cdev *cdev);
 
+struct device *device_find_by_file_path(const char *filepath);
+
 typedef int (*cdev_alias_processor_t)(struct cdev *, void *data);
 
 #ifdef CONFIG_CDEV_ALIAS
-- 
2.47.3

[PATCH v2 2/4] fs: qemu_fw_cfg: handle non-existent opt/org.barebox.env key gracefully

[Ahmad Fatoum]

QEMU fw_cfg file system driver is the sole user of
defaultenv_append_runtime_directory() and it passes a path that's not
necessarily available.

When that happens, this is not noticed until the loop in defaultenv_load()
aborts early with an -ENOENT preventing load of an external environment.

Fix this as by not adding the directory if it doesn't exist.

Fixes: 31dfb561d1a4 ("fs: qemu_fw_cfg: support populating environment via QEMU fw_cfg")
Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - move stat outside of probe, otherwise we end up with the mount
    twice, because the automount triggers before the probe is complete
    and the DCACHE_MOUNTED flag is set.
---
 fs/qemu_fw_cfg.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/qemu_fw_cfg.c b/fs/qemu_fw_cfg.c
index 5aaf14267f88..d0414b127c59 100644
--- a/fs/qemu_fw_cfg.c
+++ b/fs/qemu_fw_cfg.c
@@ -20,6 +20,8 @@
 #include <linux/err.h>
 #include <linux/ctype.h>
 
+#define FW_CFG_BAREBOX_ENV	"/mnt/fw_cfg/by_name/opt/org.barebox.env"
+
 struct fw_cfg_fs_inode {
 	struct inode inode;
 	const char *name;
@@ -376,8 +378,6 @@ static int fw_cfg_fs_probe(struct device *dev)
 	if (ret)
 		goto free_data;
 
-	defaultenv_append_runtime_directory("/mnt/fw_cfg/by_name/opt/org.barebox.env");
-
 	return 0;
 free_data:
 	free(data);
@@ -410,10 +410,12 @@ coredevice_initcall(qemu_fw_cfg_fs_init);
 
 static int qemu_fw_cfg_early_mount(void)
 {
+	struct stat s;
 	int fd;
 
 	/*
-	 * Also triggers the automount, so ramfb device can be detected
+	 * Also triggers the automount, so both ramfb device can be detected
+	 * and environment can be loaded
 	 */
 	fd = open("/mnt/fw_cfg/by_name/etc/ramfb", O_WRONLY);
 	if (fd >= 0) {
@@ -423,6 +425,9 @@ static int qemu_fw_cfg_early_mount(void)
 		platform_device_register(dev);
 	}
 
+	if (!stat(FW_CFG_BAREBOX_ENV, &s) && S_ISDIR(s.st_mode))
+		defaultenv_append_runtime_directory(FW_CFG_BAREBOX_ENV);
+
 	return 0;
 }
 late_initcall(qemu_fw_cfg_early_mount);
-- 
2.47.3

[PATCH v2 3/4] defaultenv: don't abort defaultenv_load silently on error

[Ahmad Fatoum]

Failure to load one defaultenv has no bearing on whether subsequent
defaultenv loads would succeed, so do not abort the loop early in that
case.

Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 common/startup.c        | 8 ++++++--
 defaultenv/defaultenv.c | 8 ++++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/common/startup.c b/common/startup.c
index dfea8394fdee..8f5e77d4619c 100644
--- a/common/startup.c
+++ b/common/startup.c
@@ -92,8 +92,12 @@ static int load_environment(void)
 
 	default_environment_path = default_environment_path_get();
 
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT))
-		defaultenv_load("/env", 0);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT)) {
+		ret = defaultenv_load("/env", 0);
+		if (ret)
+			pr_warn("Failed loading (some) defaultenv overlays: %pe\n",
+				ERR_PTR(ret));
+	}
 
 	if (IS_ENABLED(CONFIG_ENV_HANDLING)) {
 		envfs_load(default_environment_path, "/env", 0);
diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index d6cca466a683..99f681f6520e 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -172,17 +172,17 @@ static int defaultenv_load_one(struct defaultenv *df, const char *dir,
 int defaultenv_load(const char *dir, unsigned flags)
 {
 	struct defaultenv *df;
-	int ret;
+	int err = 0;
 
 	defaultenv_add_base();
 
 	defaultenv_add_external();
 
 	list_for_each_entry(df, &defaultenv_list, list) {
-		ret = defaultenv_load_one(df, dir, flags);
+		int ret = defaultenv_load_one(df, dir, flags);
 		if (ret)
-			return ret;
+			err = ret;
 	}
 
-	return 0;
+	return err;
 }
-- 
2.47.3

[PATCH v2 4/4] defaultenv: add base and external defaultenv in pure_initcall

[Ahmad Fatoum]

We currently depend on the order of the calls to defaultenv_append() to
decide which overlays to apply first.

To ensure the base environment is always loaded first, all of
defaultenv_append(), defaultenv_append_runtime_directory() and
defaultenv_load() call defaultenv_add_base() first.

Simplify this by adding base and external (i.e. from the BSP) defaultenv
in the earliest initcall.

This has a welcome side effect: Previously, fw_cfg env was applied
before an external defaultenv, but now it's applied afterwards and thus
can override it.

Reported-by: Jan Lübbe <j.luebbe@pengutronix.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
v1 -> v2:
  - no change
---
 defaultenv/defaultenv.c | 78 +++++++++++++++--------------------------
 1 file changed, 28 insertions(+), 50 deletions(-)

diff --git a/defaultenv/defaultenv.c b/defaultenv/defaultenv.c
index 99f681f6520e..94858f2eff07 100644
--- a/defaultenv/defaultenv.c
+++ b/defaultenv/defaultenv.c
@@ -32,48 +32,6 @@ struct defaultenv {
 	size_t size;
 };
 
-static void defaultenv_add_base(void)
-{
-	static int base_added;
-
-	if (base_added)
-		return;
-
-	base_added = 1;
-
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW))
-		defaultenv_append_directory(defaultenv_2_base);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU))
-		defaultenv_append_directory(defaultenv_2_menu);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU))
-		defaultenv_append_directory(defaultenv_2_dfu);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE))
-		defaultenv_append_directory(defaultenv_2_reboot_mode);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_SECURITY_POLICY))
-		defaultenv_append_directory(defaultenv_2_security_policy);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_IKCONFIG))
-		defaultenv_append_directory(defaultenv_2_ikconfig);
-	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC))
-		defaultenv_append_directory(defaultenv_1);
-}
-
-static void defaultenv_add_external(void)
-{
-	static int external_added;
-
-	if (external_added)
-		return;
-
-	external_added = 1;
-
-	/*
-	 * The traditional or external environment given with
-	 * CONFIG_DEFAULT_ENVIRONMENT_PATH.
-	 */
-	defaultenv_append((void *)default_environment,
-			default_environment_size, "defaultenv");
-}
-
 /*
  * defaultenv_append - append a envfs buffer to the default environment
  * @buf:	The buffer containing the binary environment. If it is
@@ -87,8 +45,6 @@ void defaultenv_append(void *buf, unsigned int size, const char *name)
 {
 	struct defaultenv *df;
 
-	defaultenv_add_base();
-
 	df = xzalloc(sizeof(*df));
 	df->buf = buf;
 	df->size = size;
@@ -101,8 +57,6 @@ void defaultenv_append_runtime_directory(const char *srcdir)
 {
 	struct defaultenv *df;
 
-	defaultenv_add_base();
-
 	df = xzalloc(sizeof(*df));
 	df->srcdir = srcdir;
 	df->name = srcdir;
@@ -174,10 +128,6 @@ int defaultenv_load(const char *dir, unsigned flags)
 	struct defaultenv *df;
 	int err = 0;
 
-	defaultenv_add_base();
-
-	defaultenv_add_external();
-
 	list_for_each_entry(df, &defaultenv_list, list) {
 		int ret = defaultenv_load_one(df, dir, flags);
 		if (ret)
@@ -186,3 +136,31 @@ int defaultenv_load(const char *dir, unsigned flags)
 
 	return err;
 }
+
+static int defaultenv_init(void)
+{
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW))
+		defaultenv_append_directory(defaultenv_2_base);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_MENU))
+		defaultenv_append_directory(defaultenv_2_menu);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_DFU))
+		defaultenv_append_directory(defaultenv_2_dfu);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_REBOOT_MODE))
+		defaultenv_append_directory(defaultenv_2_reboot_mode);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_SECURITY_POLICY))
+		defaultenv_append_directory(defaultenv_2_security_policy);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC_NEW_IKCONFIG))
+		defaultenv_append_directory(defaultenv_2_ikconfig);
+	if (IS_ENABLED(CONFIG_DEFAULT_ENVIRONMENT_GENERIC))
+		defaultenv_append_directory(defaultenv_1);
+
+	/*
+	 * The traditional or external environment given with
+	 * CONFIG_DEFAULT_ENVIRONMENT_PATH.
+	 */
+	defaultenv_append((void *)default_environment,
+			default_environment_size, "defaultenv");
+
+	return 0;
+}
+pure_initcall(defaultenv_init);
-- 
2.47.3