On Mon, Jun 17, 2019 at 09:52:34PM +0200, Heinrich Schuchardt wrote: > On 6/17/19 3:51 AM, AKASHI Takahiro wrote: > >On Sat, Jun 15, 2019 at 09:01:56PM +0200, Heinrich Schuchardt wrote: > >>On 6/5/19 6:21 AM, AKASHI Takahiro wrote: > >>>With this patch, cache buffer for UEFI variables will be created > >>>so that we will still be able to access, at least retrieve, > >>>UEFI variables when we exit from boottime services, > >>> > >>>This feature is a "should" behavior described in EBBR v1.0 > >>>section 2.5.3. > >>> > >>>Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org> > >>>--- > >>> include/efi_loader.h | 17 ++ > >>> lib/efi_loader/Kconfig | 9 + > >>> lib/efi_loader/efi_boottime.c | 10 +- > >>> lib/efi_loader/efi_runtime.c | 13 + > >>> lib/efi_loader/efi_variable.c | 467 ++++++++++++++++++++++++++++++++++ > >>> 5 files changed, 515 insertions(+), 1 deletion(-) > >> > >>Please, put the cache into a separate file. > > > >Why? > > It is a separate set of functions. In C++ programming you wouldn't put > two classes into the same file.
? I don't get your point. This is not C++, but C. > > > >>> > >>>diff --git a/include/efi_loader.h b/include/efi_loader.h > >>>index 93f7ece814a0..acab657b9d70 100644 > >>>--- a/include/efi_loader.h > >>>+++ b/include/efi_loader.h > >>>@@ -620,6 +620,23 @@ 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); > >>> > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+efi_status_t efi_freeze_variable_table(void); > >>>+ > >>>+/* runtime version of APIs */ > >>>+efi_status_t > >>>+__efi_runtime EFIAPI efi_get_variable_runtime(u16 *variable_name, > >> > >>I think one version of the functions serving at runtime and boottime is > >>enough. > >> > >>The cache should be used both at runtime and at boottime. > > > >So do you mean that we should replace the existing "boottime" version > >of get/set_variable with my code (algorithm)? > > > >This is a bit complicated work because we should be able to *udpate* > >UEFI variables at boottime, but my version of hsearch_runtime() is > >a stripped (and modified) version and doesn't support it. > > Do we really need a multilevel hash table? I would not expect hundreds > of variables. Please don't change your point suddenly. Here we are discussing whether "The cache should be used both at runtime and at boottime" or not. > > > >Making the existing hsearch_r() executable at UEFI runtime is, > >as I said before, quite painful. > > You could start the cache implementation with a less complicated data > structure like a linked list. This is totally a different issue. I listed this issue in my cover letter. > > > >>Essentially I > >>expect three modules working together: > >> > >>UEFI API implementation <-> Cache <-> Persistence driver > >> > >>I would suggest to put each of these into a separate file. > >> > >>Both the API implementation and the Cache have to be available at > >>Boottime and at Runtime. A first version of the persistence driver may > >>only be working at boottime. > > > >Unfortunately, this is not practical right now because there is > >already some sort of assumption (and consensus) that we would re-use > >"Standalone MM services", which is already there in EDK2, as > >secure storage for UEFI variables. > >In the case, all the cache would be bypassed. > >In my old prototype, I utilized the cache but dropped that feature > >for several reasons. > > What has EDK2 code to do with it? Did you follow my comment below? > >Unfortunately, this is not practical right now because there is > >already some sort of assumption (and consensus) that we would re-use > >"Standalone MM services", which is already there in EDK2, as > >secure storage for UEFI variables. > In case of write you could do a write-through in your cache if needed. > > > > >>The NV-cache content should be written to non-volatile memory on Reset() > >>and on ExitBootServices() and if possible when updating variables at > >>runtime. > > > >I'm not sure your intent here, but are you going to write back > >the cache only once? > >It won't work as every change of UEFI variable must be flushed > >to persistent storage instantly. > > The cache should support write and read. Only NV variables have to be Why? I don't think it make sense that we support volatile variable, but not non-volatile variable at runtime. Even UEFI specification doesn't describe such an irregular behavior. See EFI_RT_SUPPORTED_XXX definitions. Do you have a meaningful use case that you want to support? > written to a medium. If you do not support this currently just return > some error code vor NV variables. But you could accept still accept > changes to non-NV variables. This way we can test the code at runtime > even before implementing runtime persistence. > > > > >>>+ const efi_guid_t *vendor, > >>>+ u32 *attributes, > >>>+ efi_uintn_t *data_size, > >>>+ void *data); > >>>+efi_status_t > >>>+__efi_runtime EFIAPI efi_get_next_variable_name_runtime( > >>>+ efi_uintn_t *variable_name_size, > >>>+ u16 *variable_name, > >>>+ const efi_guid_t *vendor); > >>>+#endif /* CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING */ > >>>+ > >>> /* > >>> * See section 3.1.3 in the v2.7 UEFI spec for more details on > >>> * the layout of EFI_LOAD_OPTION. In short it is: > >>>diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > >>>index e2ef43157568..3f284795648f 100644 > >>>--- a/lib/efi_loader/Kconfig > >>>+++ b/lib/efi_loader/Kconfig > >>>@@ -59,6 +59,15 @@ config EFI_RUNTIME_CONVERT_POINTER > >>> to be called by UEFI drivers in relocating themselves to virtual > >>> address space. > >>> > >>>+config EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+ bool "runtime_service: GetVariable: Enable runtime access via cache > >>>(read-only)" > >>>+ default y > >>>+ help > >>>+ Select this option if you want to access UEFI variables at > >>>+ runtime even though you cannot update values on the fly. > >>>+ With or without this option, you can access UEFI variables > >>>+ at boottime. > >> > >>Updates of volatile variables should always be possible. > > > >Why "should"? > >Give me any use case. > >UEFI spec does not describe such a variant implementation at all. > > See above. I would like to be able to test setting variables at runtime > even if persisting NV variables is not yet implemented. Just for testing? No real use case? Please note that if we *have* persistent storage at runtime, we don't need cache because we can access that storage. > > > >>>+ > >>> config EFI_DEVICE_PATH_TO_TEXT > >>> bool "Device path to text protocol" > >>> default y > >>>diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > >>>index e4abaf3601d9..14e343abbd43 100644 > >>>--- a/lib/efi_loader/efi_boottime.c > >>>+++ b/lib/efi_loader/efi_boottime.c > >>>@@ -1892,6 +1892,9 @@ static efi_status_t EFIAPI > >>>efi_exit_boot_services(efi_handle_t image_handle, > >>> efi_uintn_t map_key) > >>> { > >>> struct efi_event *evt; > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+ efi_status_t ret; > >>>+#endif > >>> > >>> EFI_ENTRY("%p, %zx", image_handle, map_key); > >>> > >>>@@ -1921,7 +1924,12 @@ static efi_status_t EFIAPI > >>>efi_exit_boot_services(efi_handle_t image_handle, > >>> } > >>> } > >>> > >>>- /* TODO: Should persist EFI variables here */ > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >> > >>Can we have weak functions for initializing and persisting the cache, e.g. > >> > >>efi_status_t __weak > >>efi_load_variable_cache(cache_entry *cache, size_t *size) > >>{ > >> cache->len = 0; > >> return EFI_SUCCESS; > >>} > >> > >>efi_status_t __runtime __weak > >>efi_write_variable_cache(cache_entry *cache, size_t size) > >>{ > >> return EFI_UNSUPPORTED; > >>} > >> > >>Then we can override these in whatever driver we implement. > > > >What is the difference between yours and my env_efi_load/save() > >with backing-storage driver? > > I cannot see that you clearly separate the functions API, cache, > persistence. I don't get your point. For instance, efi_boottime.c has a lot of different type of functions. Do you want to separate them into different files? > I would like to see clearly define interfaces into which we can plug > different persistence implementations. Your assertion doesn't clarify why you want to separate API and cache. Regarding persistent storage, as I said, efi_[get|set]_variable() will be completely replaced with the corresponding functions (at least, in case of Standalone MM services). > > > >> > >>>+ /* No more variable update */ > >>>+ ret = efi_freeze_variable_table(); > >>>+ if (ret != EFI_SUCCESS) > >>>+ return EFI_EXIT(ret); > >>>+#endif > >>> > >>> board_quiesce_devices(); > >>> > >>>diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c > >>>index fc5bdee80e00..b60f70f04613 100644 > >>>--- a/lib/efi_loader/efi_runtime.c > >>>+++ b/lib/efi_loader/efi_runtime.c > >>>@@ -111,6 +111,11 @@ efi_status_t efi_init_runtime_supported(void) > >>> efi_runtime_services_supported |= > >>> EFI_RT_SUPPORTED_CONVERT_POINTER; > >>> #endif > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+ efi_runtime_services_supported |= > >>>+ (EFI_RT_SUPPORTED_GET_VARIABLE | > >>>+ EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME); > >>>+#endif > >>> > >>> return EFI_CALL(efi_set_variable(L"RuntimeServicesSupported", > >>> &efi_global_variable_guid, > >>>@@ -469,10 +474,18 @@ static struct efi_runtime_detach_list_struct > >>>efi_runtime_detach_list[] = { > >>> .patchto = NULL, > >>> }, { > >>> .ptr = &efi_runtime_services.get_variable, > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+ .patchto = &efi_get_variable_runtime, > >>>+#else > >>> .patchto = &efi_device_error, > >>>+#endif > >>> }, { > >>> .ptr = &efi_runtime_services.get_next_variable_name, > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+ .patchto = &efi_get_next_variable_name, > >>>+#else > >>> .patchto = &efi_device_error, > >>>+#endif > >>> }, { > >>> .ptr = &efi_runtime_services.set_variable, > >>> .patchto = &efi_device_error, > >>>diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > >>>index d9887be938c2..ee21892dd291 100644 > >>>--- a/lib/efi_loader/efi_variable.c > >>>+++ b/lib/efi_loader/efi_variable.c > >>>@@ -706,3 +706,470 @@ efi_status_t EFIAPI efi_set_variable(u16 > >>>*variable_name, > >>> > >>> return EFI_EXIT(ret); > >>> } > >>>+ > >>>+#ifdef CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING > >>>+/* > >>>+ * runtime version of APIs > >>>+ * We only support read-only variable access. > >>>+ * The table is in U-Boot's hash table format, but has its own > >>>+ * _ENTRY structure for specific use. > >>>+ * > >>>+ * Except for efi_freeze_variable_table(), which is to be called in > >>>+ * exit_boot_services(), all the functions and data below must be > >>>+ * placed in either RUNTIME_SERVICES_CODE or RUNTIME_SERVICES_DATA. > >>>+ */ > >>>+typedef struct _ENTRY { > >>>+ unsigned int used; /* hash value; 0 for not used */ > >>>+ size_t name; /* name offset from itself */ > >>>+ efi_guid_t vendor; > >>>+ u32 attributes; > >>>+ size_t data; /* data offset from itself */ > >>>+ size_t data_size; > >>>+} _ENTRY; > >>>+ > >>>+static inline u16 *entry_name(_ENTRY *e) { return (void *)e + e->name; } > >>>+static inline u16 *entry_data(_ENTRY *e) { return (void *)e + e->data; } > >>>+ > >>>+static struct hsearch_data *efi_variable_table __efi_runtime_data; > >>>+ > >>>+static size_t __efi_runtime u16_strlen_runtime(const u16 *s1) > >> > >> > >>Please, do not duplicate existing functions. If they have to be runtime > >>simply change the existing function to __runtime. > > > >We should do that if possible, but please note that [str|mem]xxx() functions > >are *architecture* dependent. > >Do you want to mark all the functions across all the architectures? Agree or not agree? > >>>+{ > >>>+ size_t n = 0; > >>>+ > >>>+ while (*s1) { > >>>+ n++; > >>>+ s1++; > >>>+ } > >>>+ > >>>+ return n; > >>>+} > >>>+ > >>>+static int __efi_runtime memcmp_runtime(const void *m1, const void *m2, > >>>+ size_t n) > >> > >>I dislike duplicate code. Can't we simply define the existing memcmp > >>function as __runtime? > > > >ditto > > > >>>+{ > >>>+ while (n && *(u8 *)m1 == *(u8 *)m2) { > >>>+ n--; > >>>+ m1++; > >>>+ m2++; > >>>+ } > >>>+ > >>>+ if (n) > >>>+ return *(u8 *)m1 - *(u8 *)m2; > >>>+ > >>>+ return 0; > >>>+} > >>>+ > >>>+static void __efi_runtime memcpy_runtime(void *m1, const void *m2, size_t > >>>n) > >>>+{ > >> > >>Can't we simply define the existing memcpy function as __runtime? > >> > >>>+ for (; n; n--, m1++, m2++) > >>>+ *(u8 *)m1 = *(u8 *)m2; > >>>+} > >>>+ > >>>+static int __efi_runtime efi_cmpkey(_ENTRY *e, const u16 *name, > >>>+ const efi_guid_t *vendor) > >>>+{ > >>>+ size_t name_len; > >>>+ > >>>+ name_len = u16_strlen_runtime(entry_name(e)); > >>>+ > >>>+ /* return zero if matched */ > >>>+ return name_len != u16_strlen_runtime(name) || > >>>+ memcmp_runtime(entry_name(e), name, name_len * 2) || > >>>+ memcmp_runtime(e->vendor.b, vendor->b, sizeof(vendor)); > >>>+} > >>>+ > >>>+/* simplified and slightly different version of hsearch_r() */ > >> > >>These hash functions are so complicated that they really need a unit > >>test testing them rigorously. > > > >Do you know that there are no any tests for hsearch_r()? > >Moreover, this function would better be exercised well > >if we could add *runtime* tests. > > Have a look at test/env/hashtable.c Okay, but why not re-use this test for my stripped version? > I think efi_selftest could work after SetVirtualMemoryMap if we do not > change the map for memory actually used by U-Boot. Maybe. -Takahiro Akashi > Or we use a minimum Linux kernel and put our tests into an init binary. > > > > >> > >>>+static int __efi_runtime hsearch_runtime(const u16 *name, > >>>+ const efi_guid_t *vendor, > >>>+ ACTION action, > >>>+ _ENTRY **retval, > >>>+ struct hsearch_data *htab) > >>>+{ > >>>+ unsigned int hval; > >>>+ unsigned int count; > >>>+ unsigned int len; > >>>+ unsigned int idx, new; > >>>+ > >>>+ /* Compute an value for the given string. */ > >>>+ len = u16_strlen_runtime(name); > >> > >>Can't the same variable name exist for different GUIDs? Why is the GUID > >>not considered in the hash? > > > >efi_cmpkey() does take into consideration GUID as well as the name. > > My question concerned the hash. > > > > >>>+ hval = len; > >>>+ count = len; > >>>+ while (count-- > 0) { > >>>+ hval <<= 4; > >>>+ hval += name[count]; > >>>+ } > >>>+ > >>>+ /* > >>>+ * First hash function: > >>>+ * simply take the modulo but prevent zero. > >>>+ */ > >>>+ hval %= htab->size; > >>>+ if (hval == 0) > >>>+ ++hval; > >>>+ > >>>+ /* The first index tried. */ > >>>+ new = -1; /* not found */ > >>>+ idx = hval; > >>>+ > >>>+ if (htab->table[idx].used) { > >>>+ /* > >>>+ * Further action might be required according to the > >>>+ * action value. > >>>+ */ > >>>+ unsigned int hval2; > >>>+ > >>>+ if (htab->table[idx].used == hval && > >>>+ !efi_cmpkey(&htab->table[idx], name, vendor)) { > >>>+ if (action == FIND) { > >>>+ *retval = &htab->table[idx]; > >>>+ return idx; > >>>+ } > >>>+ /* we don't need to support overwrite */ > >>>+ return -1; > >>>+ } > >>>+ > >>>+ /* > >>>+ * Second hash function: > >>>+ * as suggested in [Knuth] > >>>+ */ > >>>+ hval2 = 1 + hval % (htab->size - 2); > >>>+ > >>>+ do { > >>>+ /* > >>>+ * Because SIZE is prime this guarantees to > >>>+ * step through all available indices. > >>>+ */ > >>>+ if (idx <= hval2) > >>>+ idx = htab->size + idx - hval2; > >>>+ else > >>>+ idx -= hval2; > >>>+ > >>>+ /* > >>>+ * If we visited all entries leave the loop > >>>+ * unsuccessfully. > >>>+ */ > >>>+ if (idx == hval) > >>>+ break; > >>>+ > >>>+ /* If entry is found use it. */ > >>>+ if (htab->table[idx].used == hval && > >>>+ !efi_cmpkey(&htab->table[idx], name, vendor)) { > >>>+ if (action == FIND) { > >>>+ *retval = &htab->table[idx]; > >>>+ return idx; > >>>+ } > >>>+ /* we don't need to support overwrite */ > >>>+ return -1; > >>>+ } > >>>+ } while (htab->table[idx].used); > >>>+ > >>>+ if (!htab->table[idx].used) > >>>+ new = idx; > >>>+ } else { > >>>+ new = idx; > >>>+ } > >>>+ > >>>+ /* > >>>+ * An empty bucket has been found. > >>>+ * The following code should never be executed after > >>>+ * exit_boot_services() > >>>+ */ > >>>+ if (action == ENTER) { > >>>+ /* > >>>+ * If table is full and another entry should be > >>>+ * entered return with error. > >>>+ */ > >>>+ if (htab->filled == htab->size) { > >>>+ *retval = NULL; > >>>+ return 0; > >>>+ } > >>>+ > >>>+ /* Create new entry */ > >>>+ htab->table[new].used = hval; > >>>+ ++htab->filled; > >>>+ > >>>+ /* return new entry */ > >>>+ *retval = &htab->table[new]; > >>>+ return 1; > >>>+ } > >>>+ > >>>+ *retval = NULL; > >>>+ return 0; > >>>+} > >>>+ > >>>+/* from lib/hashtable.c */ > >>>+static inline int isprime(unsigned int number) > >>>+{ > >>>+ /* no even number will be passed */ > >>>+ unsigned int div = 3; > >>>+ > >>>+ while (div * div < number && number % div != 0) > >>>+ div += 2; > >>>+ > >>>+ return number % div != 0; > >>>+} > >>>+ > >>>+efi_status_t efi_freeze_variable_table(void) > >> > >>Please, add comments to your functions. It is not self-evident what this > >>function is meant to do. > >> > >>I cannot imagine why a variable cache should be frozen. It is a living > >>data structure until the system is switched off. > > > >I don't get your point. > > Please, explain what you mean by freeze. > > I suggest that the cache is read/write at all times. > > Best regards > > Heinrich > > > > >>For a variable cache I expect that you allocate memory before handling > >>the first variable and never again. At runtime you will not have chance > >>to allocate memory anyway. > > > >I don't get your point. > > > >>This function is way too long. Pleae, break it down. > > > >Freezing is implemented in 2-phase steps, and some complexity > >is inevitable. I suppose adding some comments would be enough. > > > >-Takahiro Akashi > > > >>Best regards > >> > >>Heinrich > >> > >>>+{ > >>>+ int var_num = 0; > >>>+ size_t var_data_size = 0; > >>>+ u16 *name; > >>>+ efi_uintn_t name_buf_len, name_len; > >>>+ efi_guid_t vendor; > >>>+ u32 attributes; > >>>+ u8 *mem_pool, *var_buf = NULL; > >>>+ size_t table_size, var_size, var_buf_size; > >>>+ _ENTRY *new = NULL; > >>>+ efi_status_t ret; > >>>+ > >>>+ /* phase-1 loop */ > >>>+ name_buf_len = 128; > >>>+ name = malloc(name_buf_len); > >>>+ if (!name) > >>>+ return EFI_OUT_OF_RESOURCES; > >>>+ name[0] = 0; > >>>+ for (;;) { > >>>+ name_len = name_buf_len; > >>>+ ret = EFI_CALL(efi_get_next_variable_name(&name_len, name, > >>>+ &vendor)); > >>>+ if (ret == EFI_NOT_FOUND) { > >>>+ break; > >>>+ } else if (ret == EFI_BUFFER_TOO_SMALL) { > >>>+ u16 *buf; > >>>+ > >>>+ name_buf_len = name_len; > >>>+ buf = realloc(name, name_buf_len); > >>>+ if (!buf) { > >>>+ free(name); > >>>+ return EFI_OUT_OF_RESOURCES; > >>>+ } > >>>+ name = buf; > >>>+ name_len = name_buf_len; > >>>+ ret = EFI_CALL(efi_get_next_variable_name(&name_len, > >>>+ name, > >>>+ &vendor)); > >>>+ } > >>>+ > >>>+ if (ret != EFI_SUCCESS) > >>>+ return ret; > >>>+ > >>>+ var_size = 0; > >>>+ ret = EFI_CALL(efi_get_variable(name, &vendor, &attributes, > >>>+ &var_size, NULL)); > >>>+ if (ret != EFI_BUFFER_TOO_SMALL) > >>>+ return ret; > >>>+ > >>>+ if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS)) > >>>+ continue; > >>>+ > >>>+ var_num++; > >>>+ var_data_size += (u16_strlen_runtime(name) + 1) * sizeof(u16); > >>>+ var_data_size += var_size; > >>>+ /* mem_pool must 2-byte aligned for u16 variable name */ > >>>+ if (var_data_size & 0x1) > >>>+ var_data_size++; > >>>+ } > >>>+ > >>>+ /* > >>>+ * total of entries in hash table must be a prime number. > >>>+ * The logic below comes from lib/hashtable.c > >>>+ */ > >>>+ var_num |= 1; /* make odd */ > >>>+ while (!isprime(var_num)) > >>>+ var_num += 2; > >>>+ > >>>+ /* We need table[var_num] for hsearch_runtime algo */ > >>>+ table_size = sizeof(*efi_variable_table) > >>>+ + sizeof(_ENTRY) * (var_num + 1) + var_data_size; > >>>+ ret = efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA, > >>>+ table_size, (void **)&efi_variable_table); > >>>+ if (ret != EFI_SUCCESS) > >>>+ return ret; > >>>+ > >>>+ efi_variable_table->size = var_num; > >>>+ efi_variable_table->table = (void *)efi_variable_table > >>>+ + sizeof(*efi_variable_table); > >>>+ mem_pool = (u8 *)efi_variable_table->table > >>>+ + sizeof(_ENTRY) * (var_num + 1); > >>>+ > >>>+ var_buf_size = 128; > >>>+ var_buf = malloc(var_buf_size); > >>>+ if (!var_buf) { > >>>+ ret = EFI_OUT_OF_RESOURCES; > >>>+ goto err; > >>>+ } > >>>+ > >>>+ /* phase-2 loop */ > >>>+ name[0] = 0; > >>>+ name_len = name_buf_len; > >>>+ for (;;) { > >>>+ name_len = name_buf_len; > >>>+ ret = EFI_CALL(efi_get_next_variable_name(&name_len, name, > >>>+ &vendor)); > >>>+ if (ret == EFI_NOT_FOUND) > >>>+ break; > >>>+ else if (ret != EFI_SUCCESS) > >>>+ goto err; > >>>+ > >>>+ var_size = var_buf_size; > >>>+ ret = EFI_CALL(efi_get_variable(name, &vendor, &attributes, > >>>+ &var_size, var_buf)); > >>>+ if (ret == EFI_BUFFER_TOO_SMALL) { > >>>+ free(var_buf); > >>>+ var_buf_size = var_size; > >>>+ var_buf = malloc(var_buf_size); > >>>+ if (!var_buf) { > >>>+ ret = EFI_OUT_OF_RESOURCES; > >>>+ goto err; > >>>+ } > >>>+ ret = EFI_CALL(efi_get_variable(name, &vendor, > >>>+ &attributes, > >>>+ &var_size, var_buf)); > >>>+ } > >>>+ if (ret != EFI_SUCCESS) > >>>+ goto err; > >>>+ > >>>+ if (!(attributes & EFI_VARIABLE_RUNTIME_ACCESS)) > >>>+ continue; > >>>+ > >>>+ if (hsearch_runtime(name, &vendor, ENTER, &new, > >>>+ efi_variable_table) <= 0) { > >>>+ /* This should not happen */ > >>>+ ret = EFI_INVALID_PARAMETER; > >>>+ goto err; > >>>+ } > >>>+ > >>>+ /* allocate space from RUNTIME DATA */ > >>>+ name_len = (u16_strlen_runtime(name) + 1) * sizeof(u16); > >>>+ memcpy_runtime(mem_pool, name, name_len); > >>>+ new->name = mem_pool - (u8 *)new; /* offset */ > >>>+ mem_pool += name_len; > >>>+ > >>>+ memcpy_runtime(&new->vendor.b, &vendor.b, sizeof(vendor)); > >>>+ > >>>+ new->attributes = attributes; > >>>+ > >>>+ memcpy_runtime(mem_pool, var_buf, var_size); > >>>+ new->data = mem_pool - (u8 *)new; /* offset */ > >>>+ new->data_size = var_size; > >>>+ mem_pool += var_size; > >>>+ > >>>+ /* mem_pool must 2-byte aligned for u16 variable name */ > >>>+ if ((uintptr_t)mem_pool & 0x1) > >>>+ mem_pool++; > >>>+ } > >>>+#ifdef DEBUG > >>>+ name[0] = 0; > >>>+ name_len = name_buf_len; > >>>+ for (;;) { > >>>+ name_len = name_buf_len; > >>>+ ret = efi_get_next_variable_name_runtime(&name_len, name, > >>>+ &vendor); > >>>+ if (ret == EFI_NOT_FOUND) > >>>+ break; > >>>+ else if (ret != EFI_SUCCESS) > >>>+ goto err; > >>>+ > >>>+ var_size = var_buf_size; > >>>+ ret = efi_get_variable_runtime(name, &vendor, &attributes, > >>>+ &var_size, var_buf); > >>>+ if (ret != EFI_SUCCESS) > >>>+ goto err; > >>>+ > >>>+ printf("%ls_%pUl:\n", name, &vendor); > >>>+ printf(" attributes: 0x%x\n", attributes); > >>>+ printf(" value (size: 0x%lx)\n", var_size); > >>>+ } > >>>+#endif > >>>+ ret = EFI_SUCCESS; > >>>+ > >>>+err: > >>>+ free(name); > >>>+ free(var_buf); > >>>+ if (ret != EFI_SUCCESS && efi_variable_table) { > >>>+ efi_free_pool(efi_variable_table); > >>>+ efi_variable_table = NULL; > >>>+ } > >>>+ > >>>+ return ret; > >>>+} > >>>+ > >>>+efi_status_t > >>>+__efi_runtime EFIAPI efi_get_variable_runtime(u16 *variable_name, > >>>+ const efi_guid_t *vendor, > >>>+ u32 *attributes, > >>>+ efi_uintn_t *data_size, > >>>+ void *data) > >>>+{ > >>>+ _ENTRY *new; > >>>+ > >>>+ if (!variable_name || !vendor || !data_size) > >>>+ return EFI_EXIT(EFI_INVALID_PARAMETER); > >>>+ > >>>+ if (hsearch_runtime(variable_name, vendor, FIND, &new, > >>>+ efi_variable_table) <= 0) > >>>+ return EFI_NOT_FOUND; > >>>+ > >>>+ if (attributes) > >>>+ *attributes = new->attributes; > >>>+ if (*data_size < new->data_size) { > >>>+ *data_size = new->data_size; > >>>+ return EFI_BUFFER_TOO_SMALL; > >>>+ } > >>>+ > >>>+ *data_size = new->data_size; > >>>+ memcpy_runtime(data, entry_data(new), new->data_size); > >>>+ > >>>+ return EFI_SUCCESS; > >>>+} > >>>+ > >>>+static int prev_idx __efi_runtime_data; > >>>+ > >>>+efi_status_t > >>>+__efi_runtime EFIAPI efi_get_next_variable_name_runtime( > >>>+ efi_uintn_t *variable_name_size, > >>>+ u16 *variable_name, > >>>+ const efi_guid_t *vendor) > >>>+{ > >>>+ _ENTRY *e; > >>>+ u16 *name; > >>>+ efi_uintn_t name_size; > >>>+ > >>>+ if (!variable_name_size || !variable_name || !vendor) > >>>+ return EFI_INVALID_PARAMETER; > >>>+ > >>>+ if (variable_name[0]) { > >>>+ /* sanity check for previous variable */ > >>>+ if (prev_idx < 0) > >>>+ return EFI_INVALID_PARAMETER; > >>>+ > >>>+ e = &efi_variable_table->table[prev_idx]; > >>>+ if (!e->used || efi_cmpkey(e, variable_name, vendor)) > >>>+ return EFI_INVALID_PARAMETER; > >>>+ } else { > >>>+ prev_idx = -1; > >>>+ } > >>>+ > >>>+ /* next variable */ > >>>+ while (++prev_idx <= efi_variable_table->size) { > >>>+ e = &efi_variable_table->table[prev_idx]; > >>>+ if (e->used) > >>>+ break; > >>>+ } > >>>+ if (prev_idx > efi_variable_table->size) > >>>+ return EFI_NOT_FOUND; > >>>+ > >>>+ name = entry_name(e); > >>>+ name_size = (u16_strlen_runtime(name) + 1) > >>>+ * sizeof(u16); > >>>+ if (*variable_name_size < name_size) { > >>>+ *variable_name_size = name_size; > >>>+ return EFI_BUFFER_TOO_SMALL; > >>>+ } > >>>+ > >>>+ memcpy_runtime(variable_name, name, name_size); > >>>+ memcpy_runtime((void *)&vendor->b, &e->vendor.b, sizeof(vendor)); > >>>+ > >>>+ return EFI_SUCCESS; > >>>+} > >>>+#endif /* CONFIG_EFI_RUNTIME_GET_VARIABLE_CACHING */ > >>> > >> > > > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot