Heinrich, On Tue, Jun 18, 2019 at 10:19:06AM +0900, AKASHI Takahiro wrote: > 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.
Let me elaborate more: My tentative "secure boot" patch is self-contained on non-secure side and provides the functionality that UEFI specification describes, but there is one missing and essential aspect: Tamper-resistant. Standalone MM services is expected to provides this semantics with the same interfaces as [Get|Set]Variable for both volatile and non-volatile variables alike. There can be another implementation (with yet another interfaces to secure side), but it is currently the only implementation available as far as I know. -Takahiro Akashi > > 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