From mboxrd@z Thu Jan  1 00:00:00 1970
From: AKASHI Takahiro <takahiro.akashi@linaro.org>
Date: Wed, 12 Jun 2019 14:23:50 +0900
Subject: [U-Boot] [PATCH v3 1/7] env: save UEFI non-volatile variables
 in dedicated storage
In-Reply-To: <20190611105952.GA3633@apalos>
References: <20190604065211.15907-1-takahiro.akashi@linaro.org>
 <20190604065211.15907-2-takahiro.akashi@linaro.org>
 <20190611105952.GA3633@apalos>
Message-ID: <20190612052348.GB15190@linaro.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Ilias,

On Tue, Jun 11, 2019 at 01:59:52PM +0300, Ilias Apalodimas wrote:
> Hi Akashi-san
> 
> Thanks for doing this!
> 
> [...]
> > +	return 0;
> > +}
> > +
> > +int env_efi_save(void)
> > +{
> > +#ifdef CONFIG_ENV_IS_NOWHERE
> One of the 'features' we discussed is the ability to have CONFIG_ENV_IS_NOWHERE
> set (not allowing users to change the U-Boot ENV) and still be able to store 
> UEFI variables. Doesn't this ifdef prevent that from happening?

Yeah, I don't forget this requirement, but it is not yet agreed that
"driver" framework be shared between U-Boot environment and
UEFI variables. As you can see, env/fat.c has some duplicated code
in my current implementation and more will be added for other devices.

So I will fix this issue depending on the discussion.

-Takahiro Akashi

> > +	return 0;
> > +#else
> > +	struct env_driver *drv = NULL;
> > +	int ret;
> > +
> > +	if (!efi_nv_var_htab.table)
> > +		return 0;
> > +
> > +	if (gd->env_efi_prio == -1) {
> > +		pr_warn("No UEFI non-volatile variable storage\n");
> > +		return -1;
> > +	}
> > +
> > +	drv = _env_driver_lookup(env_get_location(ENVOP_EFI, gd->env_efi_prio));
> > +	if (!drv) {
> > +		pr_warn("No UEFI non-volatile variable storage\n");
> > +		return -1;
> > +	}
> > +
> > +	ret = drv->efi_save();
> > +	if (ret)
> > +		pr_err("Saving UEFI non-volatile variable failed\n");
> > +
> > +	return ret;
> > +#endif
> > +}
> > +
> > +/* This function should be called only once at init */
> > +int env_efi_load(void)
> > +{
> > +#ifndef CONFIG_ENV_IS_NOWHERE
> ditto
> > +	struct env_driver *drv;
> > +	int prio;
> > +	enum env_location loc;
> > +#endif
> > +	int ret;
> > +
> > +	/* volatile variables */
> > +	if (!efi_var_htab.table) {
> > +		ret = himport_r(&efi_var_htab, NULL, 0, '\0', 0, 0, 0, NULL);
> > +		if (!ret) {
> > +			pr_err("Creating UEFI volatile variables failed\n");
> > +			return -1;
> > +		}
> > +	}
> > +
> > +#ifndef CONFIG_ENV_IS_NOWHERE
> ditto
> > +	gd->env_efi_prio = -1;
> > +
> > +	/* non-volatile variables */
> > +	if (efi_nv_var_htab.table)
> > +		return 0;
> > +
> > +	for (drv = NULL, prio = 0; prio < ARRAY_SIZE(env_locations); prio++) {
> > +		loc = env_get_location(ENVOP_EFI, prio);
> > +		drv = _env_driver_lookup(loc);
> > +		if (!drv)
> > +			continue;
> > +
> > +		if (drv->efi_load && drv->efi_save)
> > +			break;
> > +	}
> > +	if (!drv || prio == ARRAY_SIZE(env_locations)) {
> > +		pr_warn("No UEFI non-volatile variable storage\n");
> > +		goto skip_load;
> > +	}
> > +
> > +	gd->env_efi_prio = prio;
> > +
> > +	ret = drv->efi_load();
> > +	if (ret) {
> > +		pr_err("Loading UEFI non-volatile variables failed\n");
> > +		return -1;
> > +	}
> > +skip_load:
> > +#endif /* CONFIG_ENV_IS_NOWHERE */
> [...]
> 
> Thanks
> /Ilias