From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 5 Jun 2019 09:48:18 +0900 Subject: [U-Boot] [PATCH v3 3/7] efi_loader: variable: split UEFI variables from U-Boot environment In-Reply-To: References: <20190604065211.15907-1-takahiro.akashi@linaro.org> <20190604065211.15907-4-takahiro.akashi@linaro.org> Message-ID: <20190605004817.GK27279@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Jun 04, 2019 at 11:31:24PM +0200, Heinrich Schuchardt wrote: > On 6/4/19 8:52 AM, AKASHI Takahiro wrote: > >UEFI volatile variables are managed in efi_var_htab while UEFI non-volatile > >variables are in efi_nv_var_htab. At every SetVariable API, env_efi_save() > >will also be called to save data cache (hash table) to persistent storage. > > > >Signed-off-by: AKASHI Takahiro > >--- > > lib/efi_loader/Kconfig | 10 + > > lib/efi_loader/efi_variable.c | 342 ++++++++++++++++++++++++++-------- > > 2 files changed, 275 insertions(+), 77 deletions(-) > > > >diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >index cd5436c576b1..8bf4b1754d06 100644 > >--- a/lib/efi_loader/Kconfig > >+++ b/lib/efi_loader/Kconfig > >@@ -18,6 +18,16 @@ config EFI_LOADER > > > > if EFI_LOADER > > > >+choice > >+ prompt "Select variables storage" > >+ default EFI_VARIABLE_USE_ENV > >+ > >+config EFI_VARIABLE_USE_ENV > >+ bool "Same device as U-Boot environment" > >+ select ENV_EFI > >+ > >+endchoice > >+ > > config EFI_GET_TIME > > bool "GetTime() runtime service" > > depends on DM_RTC > >diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > >index e56053194dae..d9887be938c2 100644 > >--- a/lib/efi_loader/efi_variable.c > >+++ b/lib/efi_loader/efi_variable.c > >@@ -48,6 +48,66 @@ > > * converted to utf16? > > */ > > > >+/* > >+ * We will maintain two variable database: one for volatile variables, > >+ * the other for non-volatile variables. The former exists only in memory > >+ * and will go away at re-boot. The latter is currently backed up by the same > >+ * device as U-Boot environment and also works as variables cache. > >+ * > >+ * struct hsearch_data efi_var_htab > >+ * struct hsearch_data efi_nv_var_htab > >+ */ > >+ > >+static char *env_efi_get(const char *name, bool is_non_volatile) > >+{ > >+ struct hsearch_data *htab; > >+ ENTRY e, *ep; > >+ > >+ /* WATCHDOG_RESET(); */ > >+ > >+ if (is_non_volatile) > >+ htab = &efi_nv_var_htab; > >+ else > >+ htab = &efi_var_htab; > >+ > >+ e.key = name; > >+ e.data = NULL; > >+ hsearch_r(e, FIND, &ep, htab, 0); > >+ > >+ return ep ? ep->data : NULL; > >+} > >+ > >+static int env_efi_set(const char *name, const char *value, > >+ bool is_non_volatile) > >+{ > >+ struct hsearch_data *htab; > >+ ENTRY e, *ep; > >+ int ret; > >+ > >+ if (is_non_volatile) > >+ htab = &efi_nv_var_htab; > >+ else > >+ htab = &efi_var_htab; > >+ > >+ /* delete */ > >+ if (!value || *value == '\0') { > >+ ret = hdelete_r(name, htab, H_PROGRAMMATIC); > >+ return !ret; > >+ } > >+ > >+ /* set */ > >+ e.key = name; > >+ e.data = (char *)value; > >+ hsearch_r(e, ENTER, &ep, htab, H_PROGRAMMATIC); > >+ if (!ep) { > >+ printf("## Error inserting \"%s\" variable, errno=%d\n", > >+ name, errno); > >+ return 1; > >+ } > >+ > >+ return 0; > >+} > >+ > > #define PREFIX_LEN (strlen("efi_xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx_")) > > > > /** > >@@ -147,24 +207,12 @@ static const char *parse_attr(const char *str, u32 *attrp) > > return str; > > } > > > >-/** > >- * efi_efi_get_variable() - retrieve value of a UEFI variable > >- * > >- * This function implements the GetVariable runtime service. > >- * > >- * See the Unified Extensible Firmware Interface (UEFI) specification for > >- * details. > >- * > >- * @variable_name: name of the variable > >- * @vendor: vendor GUID > >- * @attributes: attributes of the variable > >- * @data_size: size of the buffer to which the variable value is copied > >- * @data: buffer to which the variable value is copied > >- * Return: status code > >- */ > >-efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > >- const efi_guid_t *vendor, u32 *attributes, > >- efi_uintn_t *data_size, void *data) > >+static > >+efi_status_t EFIAPI efi_get_variable_common(u16 *variable_name, > >+ const efi_guid_t *vendor, > >+ u32 *attributes, > >+ efi_uintn_t *data_size, void *data, > >+ bool is_non_volatile) > > { > > char *native_name; > > efi_status_t ret; > >@@ -172,22 +220,19 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > > const char *val, *s; > > u32 attr; > > > >- EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes, > >- data_size, data); > >- > > if (!variable_name || !vendor || !data_size) > > return EFI_EXIT(EFI_INVALID_PARAMETER); > > > > ret = efi_to_native(&native_name, variable_name, vendor); > > if (ret) > >- return EFI_EXIT(ret); > >+ return ret; > > > > EFI_PRINT("get '%s'\n", native_name); > > > >- val = env_get(native_name); > >+ val = env_efi_get(native_name, is_non_volatile); > > free(native_name); > > if (!val) > >- return EFI_EXIT(EFI_NOT_FOUND); > >+ return EFI_NOT_FOUND; > > > > val = parse_attr(val, &attr); > > > >@@ -198,7 +243,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > > > > /* number of hexadecimal digits must be even */ > > if (len & 1) > >- return EFI_EXIT(EFI_DEVICE_ERROR); > >+ return EFI_DEVICE_ERROR; > > > > /* two characters per byte: */ > > len /= 2; > >@@ -210,10 +255,10 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > > } > > > > if (!data) > >- return EFI_EXIT(EFI_INVALID_PARAMETER); > >+ return EFI_INVALID_PARAMETER; > > > > if (hex2bin(data, s, len)) > >- return EFI_EXIT(EFI_DEVICE_ERROR); > >+ return EFI_DEVICE_ERROR; > > > > EFI_PRINT("got value: \"%s\"\n", s); > > } else if ((s = prefix(val, "(utf8)"))) { > >@@ -227,7 +272,7 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > > } > > > > if (!data) > >- return EFI_EXIT(EFI_INVALID_PARAMETER); > >+ return EFI_INVALID_PARAMETER; > > > > memcpy(data, s, len); > > ((char *)data)[len] = '\0'; > >@@ -235,13 +280,67 @@ efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > > EFI_PRINT("got value: \"%s\"\n", (char *)data); > > } else { > > EFI_PRINT("invalid value: '%s'\n", val); > >- return EFI_EXIT(EFI_DEVICE_ERROR); > >+ return EFI_DEVICE_ERROR; > > } > > > > out: > > if (attributes) > > *attributes = attr & EFI_VARIABLE_MASK; > > > >+ return ret; > >+} > >+ > >+static > >+efi_status_t EFIAPI efi_get_volatile_variable(u16 *variable_name, > >+ const efi_guid_t *vendor, > >+ u32 *attributes, > >+ efi_uintn_t *data_size, > >+ void *data) > >+{ > >+ return efi_get_variable_common(variable_name, vendor, attributes, > >+ data_size, data, false); > >+} > >+ > >+efi_status_t EFIAPI efi_get_nonvolatile_variable(u16 *variable_name, > >+ const efi_guid_t *vendor, > >+ u32 *attributes, > >+ efi_uintn_t *data_size, > >+ void *data) > >+{ > >+ return efi_get_variable_common(variable_name, vendor, attributes, > >+ data_size, data, true); > >+} > >+ > >+/** > >+ * efi_efi_get_variable() - retrieve value of a UEFI variable > >+ * > >+ * This function implements the GetVariable runtime service. > >+ * > >+ * See the Unified Extensible Firmware Interface (UEFI) specification for > >+ * details. > >+ * > >+ * @variable_name: name of the variable > >+ * @vendor: vendor GUID > >+ * @attributes: attributes of the variable > >+ * @data_size: size of the buffer to which the variable value is copied > >+ * @data: buffer to which the variable value is copied > >+ * Return: status code > >+ */ > >+efi_status_t EFIAPI efi_get_variable(u16 *variable_name, > >+ const efi_guid_t *vendor, u32 *attributes, > >+ efi_uintn_t *data_size, void *data) > >+{ > >+ efi_status_t ret; > >+ > >+ EFI_ENTRY("\"%ls\" %pUl %p %p %p", variable_name, vendor, attributes, > >+ data_size, data); > >+ > >+ ret = efi_get_volatile_variable(variable_name, vendor, attributes, > >+ data_size, data); > >+ if (ret == EFI_NOT_FOUND) > >+ ret = efi_get_nonvolatile_variable(variable_name, vendor, > >+ attributes, data_size, data); > >+ > > return EFI_EXIT(ret); > > } > > > >@@ -331,7 +430,7 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, > > u16 *variable_name, > > const efi_guid_t *vendor) > > { > >- char *native_name, *variable; > >+ char *native_name, *variable, *tmp_list, *merged_list; > > ssize_t name_len, list_len; > > char regex[256]; > > char * const regexlist[] = {regex}; > >@@ -387,10 +486,39 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, > > efi_cur_variable = NULL; > > > > snprintf(regex, 256, "efi_.*-.*-.*-.*-.*_.*"); > >- list_len = hexport_r(&env_htab, '\n', > >+ list_len = hexport_r(&efi_var_htab, '\n', > > H_MATCH_REGEX | H_MATCH_KEY, > > &efi_variables_list, 0, 1, regexlist); > >- /* 1 indicates that no match was found */ > >+ /* > >+ * Note: '1' indicates that nothing is matched > >+ */ > >+ if (list_len <= 1) { > >+ free(efi_variables_list); > >+ efi_variables_list = NULL; > >+ list_len = hexport_r(&efi_nv_var_htab, '\n', > >+ H_MATCH_REGEX | H_MATCH_KEY, > >+ &efi_variables_list, 0, 1, > >+ regexlist); > >+ } else { > >+ tmp_list = NULL; > >+ list_len = hexport_r(&efi_nv_var_htab, '\n', > >+ H_MATCH_REGEX | H_MATCH_KEY, > >+ &tmp_list, 0, 1, > >+ regexlist); > >+ if (list_len <= 1) { > >+ list_len = 2; /* don't care actual number */ > >+ } else { > >+ /* merge two variables lists */ > >+ merged_list = malloc(strlen(efi_variables_list) > >+ + strlen(tmp_list) + 1); > >+ strcpy(merged_list, efi_variables_list); > >+ strcat(merged_list, tmp_list); > >+ free(efi_variables_list); > >+ free(tmp_list); > >+ efi_variables_list = merged_list; > >+ } > >+ } > >+ > > if (list_len <= 1) > > return EFI_EXIT(EFI_NOT_FOUND); > > > >@@ -403,77 +531,71 @@ efi_status_t EFIAPI efi_get_next_variable_name(efi_uintn_t *variable_name_size, > > return EFI_EXIT(ret); > > } > > > >-/** > >- * efi_efi_set_variable() - set value of a UEFI variable > >- * > >- * This function implements the SetVariable runtime service. > >- * > >- * See the Unified Extensible Firmware Interface (UEFI) specification for > >- * details. > >- * > >- * @variable_name: name of the variable > >- * @vendor: vendor GUID > >- * @attributes: attributes of the variable > >- * @data_size: size of the buffer with the variable value > >- * @data: buffer with the variable value > >- * Return: status code > >- */ > >-efi_status_t EFIAPI efi_set_variable(u16 *variable_name, > >- const efi_guid_t *vendor, u32 attributes, > >- efi_uintn_t data_size, const void *data) > >+static > >+efi_status_t EFIAPI efi_set_variable_common(u16 *variable_name, > >+ const efi_guid_t *vendor, > >+ u32 attributes, > >+ efi_uintn_t data_size, > >+ const void *data, > >+ bool is_non_volatile) > > { > > char *native_name = NULL, *val = NULL, *s; > >- efi_status_t ret = EFI_SUCCESS; > >+ efi_uintn_t size; > > u32 attr; > >- > >- EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes, > >- data_size, data); > >+ efi_status_t ret = EFI_SUCCESS; > > > > /* TODO: implement APPEND_WRITE */ > > if (!variable_name || !vendor || > > (attributes & EFI_VARIABLE_APPEND_WRITE)) { > > ret = EFI_INVALID_PARAMETER; > >- goto out; > >+ goto err; > > } > > > > ret = efi_to_native(&native_name, variable_name, vendor); > > if (ret) > >- goto out; > >+ goto err; > > > > #define ACCESS_ATTR (EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS) > > > >- if ((data_size == 0) || !(attributes & ACCESS_ATTR)) { > >- /* delete the variable: */ > >- env_set(native_name, NULL); > >- ret = EFI_SUCCESS; > >- goto out; > >+ /* check if a variable exists */ > >+ size = 0; > >+ ret = EFI_CALL(efi_get_variable(variable_name, vendor, &attr, > >+ &size, NULL)); > >+ if (ret == EFI_BUFFER_TOO_SMALL) { > >+ if ((is_non_volatile && !(attr & EFI_VARIABLE_NON_VOLATILE)) || > >+ (!is_non_volatile && (attr & EFI_VARIABLE_NON_VOLATILE))) { > >+ ret = EFI_INVALID_PARAMETER; > >+ goto err; > >+ } > > } > > > >- val = env_get(native_name); > >- if (val) { > >- parse_attr(val, &attr); > >- > >- /* We should not free val */ > >- val = NULL; > >- if (attr & READ_ONLY) { > >- ret = EFI_WRITE_PROTECTED; > >+ /* delete a variable */ > >+ if (data_size == 0 || !(attributes & ACCESS_ATTR)) { > >+ if (size) { > >+ if (attr & READ_ONLY) { > >+ ret = EFI_WRITE_PROTECTED; > >+ goto err; > >+ } > > goto out; > > } > >+ ret = EFI_SUCCESS; > >+ goto err; /* not error, but nothing to do */ > >+ } > > > >+ /* create/modify a variable */ > >+ if (size && attr != attributes) { > > /* > > * attributes won't be changed > > * TODO: take care of APPEND_WRITE once supported > > */ > >- if (attr != attributes) { > >- ret = EFI_INVALID_PARAMETER; > >- goto out; > >- } > >+ ret = EFI_INVALID_PARAMETER; > >+ goto err; > > } > > > > val = malloc(2 * data_size + strlen("{ro,run,boot,nv}(blob)") + 1); > > if (!val) { > > ret = EFI_OUT_OF_RESOURCES; > >- goto out; > >+ goto err; > > } > > > > s = val; > >@@ -487,7 +609,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, > > EFI_VARIABLE_RUNTIME_ACCESS); > > s += sprintf(s, "{"); > > while (attributes) { > >- u32 attr = 1 << (ffs(attributes) - 1); > >+ attr = 1 << (ffs(attributes) - 1); > > > > if (attr == EFI_VARIABLE_NON_VOLATILE) > > s += sprintf(s, "nv"); > >@@ -509,12 +631,78 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, > > > > EFI_PRINT("setting: %s=%s\n", native_name, val); > > > >- if (env_set(native_name, val)) > >+out: > >+ ret = EFI_SUCCESS; > >+ if (env_efi_set(native_name, val, is_non_volatile)) > > ret = EFI_DEVICE_ERROR; > > > >-out: > >+err: > > free(native_name); > > free(val); > > > >+ return ret; > >+} > >+ > >+static > >+efi_status_t EFIAPI efi_set_volatile_variable(u16 *variable_name, > > Once you have implemented this we can make the variable service > available at runtime. So I suggest to define everything here as __runtime. Good question! I intended so when I started this work, but we can't do that partly because bunch of code from U-Boot, which is not part of RUNTIME_CODE, will be exercised and partly because all the entries in a hash table are allocated by *malloc*, which are again not part of RUNTIME_DATA. It is quite painful to modify the code and data so that they are accessible at UEFI runtime. Instead, I implemented "caching" features for runtime access. I'm going to post the patch set (as RFC) later today. > Why do you any of these three functions (efi_set_volatile_variable(), > efi_set_nonvolatile_variable(), efi_set_nonvolatile_variable()). Just > use an if based on attributes in efi_set_variable() to call > env_efi_save, when a non-volatile function is changed. I don't get you point. Please clarify your comment. > What is the advantage of having two separate RAM stores? Can't the save > function sort out what to save and what not to save according to the NV > flag? Technically, we can do that, but it is nothing but inefficient. Just FYI, EDK2 also maintains separate buffers. > >+ const efi_guid_t *vendor, > >+ u32 attributes, > >+ efi_uintn_t data_size, > >+ const void *data) > >+{ > >+ return efi_set_variable_common(variable_name, vendor, attributes, > >+ data_size, data, false); > >+} > >+ > >+efi_status_t EFIAPI efi_set_nonvolatile_variable(u16 *variable_name, > >+ const efi_guid_t *vendor, > >+ u32 attributes, > >+ efi_uintn_t data_size, > >+ const void *data) > >+{ > >+ efi_status_t ret; > >+ > >+ ret = efi_set_variable_common(variable_name, vendor, attributes, > >+ data_size, data, true); > >+ if (ret == EFI_SUCCESS) > >+ /* FIXME: what if save failed? */ > >+ if (env_efi_save()) > > Somewhere we need the ability to switch between different backends. Will > that be in env_efi_save()? That is why I put the related code in "env." Thanks, -Takahiro Akashi > >+ ret = EFI_DEVICE_ERROR; > >+ > >+ return ret; > >+} > >+ > >+/** > >+ * efi_efi_set_variable() - set value of a UEFI variable > >+ * > >+ * This function implements the SetVariable runtime service. > >+ * > >+ * See the Unified Extensible Firmware Interface (UEFI) specification for > >+ * details. > >+ * > >+ * @variable_name: name of the variable > >+ * @vendor: vendor GUID > >+ * @attributes: attributes of the variable > >+ * @data_size: size of the buffer with the variable value > >+ * @data: buffer with the variable value > >+ * Return: status code > >+ */ > >+efi_status_t EFIAPI efi_set_variable(u16 *variable_name, > >+ const efi_guid_t *vendor, u32 attributes, > >+ efi_uintn_t data_size, const void *data) > >+{ > >+ efi_status_t ret; > >+ > >+ EFI_ENTRY("\"%ls\" %pUl %x %zu %p", variable_name, vendor, attributes, > >+ data_size, data); > >+ > >+ if (attributes & EFI_VARIABLE_NON_VOLATILE) > >+ ret = efi_set_nonvolatile_variable(variable_name, vendor, > >+ attributes, > >+ data_size, data); > >+ else > >+ ret = efi_set_volatile_variable(variable_name, vendor, > >+ attributes, data_size, data); > >+ > > return EFI_EXIT(ret); > > } > > >