From f81dc0a4e2e8ecd07766951cd3e4ee19d11fbfd1 Mon Sep 17 00:00:00 2001 From: Konstantin Aladyshev Date: Thu, 29 Sep 2022 15:32:02 +0300 Subject: Create a driver with 'Buffer Storage' - Part 4 Signed-off-by: Konstantin Aladyshev --- Lessons_uncategorized/Lesson_Varstore_4/README.md | 199 ++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 Lessons_uncategorized/Lesson_Varstore_4/README.md (limited to 'Lessons_uncategorized/Lesson_Varstore_4') diff --git a/Lessons_uncategorized/Lesson_Varstore_4/README.md b/Lessons_uncategorized/Lesson_Varstore_4/README.md new file mode 100644 index 0000000..081a706 --- /dev/null +++ b/Lessons_uncategorized/Lesson_Varstore_4/README.md @@ -0,0 +1,199 @@ +# Make `EFI_HII_CONFIG_ACCESS_PROTOCOL.ExtractConfig()` compatible with UEFI specification + +Here is some description from the UEFI specification regarding `EFI_HII_CONFIG_ACCESS_PROTOCOL.ExtractConfig()` function: +``` +If a NULL is passed in for the Request field, all of the settings being abstracted by this function will be returned in the Results field. In addition, if a ConfigHdr is +passed in with no request elements, all of the settings being abstracted for that particular ConfigHdr reference will be returned in the Results Field. +``` + +Currently our `ExtractConfig` implementation doesn't cover this corner cases. It simply uses `BlockToConfig` function, which needs `OFFSET=<...>&WIDTH=<...>` pairs for it to function correctly: +```cpp +STATIC +EFI_STATUS +EFIAPI +ExtractConfig ( + IN CONST EFI_HII_CONFIG_ACCESS_PROTOCOL *This, + IN CONST EFI_STRING Request, + OUT EFI_STRING *Progress, + OUT EFI_STRING *Results +) +{ + if (Progress == NULL || Results == NULL) { + return EFI_INVALID_PARAMETER; + } + + // Here can be additional custom operations to get you VARIABLE_STRUCTURE data + + EFI_STATUS Status = gHiiConfigRouting->BlockToConfig(gHiiConfigRouting, + Request, + (UINT8*)&FormStorage, + sizeof(VARIABLE_STRUCTURE), + Results, + Progress); + + return Status; +} +``` + +But we've just seen with our `HIIConfig.efi` application that every possible call works correctly. Why is that? Let's explore this. + +## No `OFFSET=<...>&WIDTH=<...>` pairs in the request + +I've tried to debug the code on the driver side while using `HIIConfig.efi` and the interesting fact is that even if our `HIIConfig` program sends +``` +GUID=927580373a731b4f9557f22af743e8c2&NAME=0046006f0072006d0044006100740061&PATH=01041400641982fbb4ac7b439fe666aa7ad7c5d87fff0400 +``` +The driver gets: +``` +GUID=927580373a731b4f9557f22af743e8c2&NAME=0046006f0072006d0044006100740061&PATH=01041400641982fbb4ac7b439fe666aa7ad7c5d87fff0400&OFFSET=0000&WIDTH=0001&OFFSET=0001&WIDTH=0002&OFFSET=0003&WIDTH=0014&OFFSET=0019&WIDTH=0004&OFFSET=001d&WIDTH=0003&OFFSET=0020&WIDTH=0001&OFFSET=0021&WIDTH=0003 +``` + +This transformation of the request is implicit and is happening inside the `EFI_HII_CONFIG_ROUTING_PROTOCOL.ExtartConfig()` function [https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c](https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/HiiDatabaseDxe/ConfigRouting.c) + +This is a reason why our driver handles requests without `OFFSET=<...>&WIDTH=<...>` pairs without any errors. + +## `Request == NULL` + +It is not possible to issue `EFI_HII_CONFIG_ROUTING_PROTOCOL.ExtractConfig()` with a `NULL` in place of a configuration request. Function will simply fail. + +But you've seen that our driver is called with a `NULL` request when `EFI_HII_CONFIG_ROUTING_PROTOCOL.ExportConfig()` function is executed. As we don't cover this case (`BlockToConfig` with `NULL` request simply returns error), the `ExportConfig()` issues the full request. And that request our driver can handle. So once again we don't see any visible errors if we don't implement `NULL` handling functionality. + +Currently `Request == NULL` handling can only be seen as an optimization for the `EFI_HII_CONFIG_ROUTING_PROTOCOL.ExportConfig()` call. + +## `ExtractConfig()` changes + +Even if we've seen that currently our driver is good with EDKII, we should respect UEFI specification requirements. In other case EDKII can change it's behaviour sometime in the future and our driver will just stop working. + +So let's add this functionality. It can be done like this: +```cpp +STATIC +EFI_STATUS +EFIAPI +ExtractConfig ( + IN CONST EFI_HII_CONFIG_ACCESS_PROTOCOL *This, + IN CONST EFI_STRING Request, + OUT EFI_STRING *Progress, + OUT EFI_STRING *Results +) +{ + if (Progress == NULL || Results == NULL) { + return EFI_INVALID_PARAMETER; + } + + BOOLEAN AllocatedRequest = FALSE; + EFI_STRING ConfigRequest = Request; <--- by default our ConfigRequest that we would use with BlockToConfig function is equal to Request argument + + if ((Request == NULL) || (StrStr (Request, L"OFFSET") == NULL)) { <--- check if Request argument is not enough for the direct BlockToConfig call + EFI_STRING ConfigRequestHdr = HiiConstructConfigHdr(&StorageGuid, StorageName, mDriverHandle); + UINTN Size = (StrLen(ConfigRequestHdr) + StrLen(L"&OFFSET=0&WIDTH=") + sizeof(UINT64)*2 + 1) * sizeof(CHAR16); <--- sizeof(VARIABLE_STRUCTURE) is UINTN=UINT64, therefore to encode it we need 'sizeof(UINT64)*2' symbols + ConfigRequest = AllocateZeroPool(Size); + AllocatedRequest = TRUE; + UnicodeSPrint(ConfigRequest, Size, L"%s&OFFSET=0&WIDTH=%016LX", ConfigRequestHdr, sizeof(VARIABLE_STRUCTURE)); <--- ConfigRequest = "Request + &OFFSET=0&WIDTH=XXXXXXXXXXXXXXXX" + FreePool(ConfigRequestHdr); + } + + EFI_STATUS Status = gHiiConfigRouting->BlockToConfig(gHiiConfigRouting, + ConfigRequest, <--- here we use ConfigRequest that can be "Request" or "Request + &OFFSET=0&WIDTH=XXXXXXXXXXXXXXXX" + (UINT8*)&FormStorage, + sizeof(VARIABLE_STRUCTURE), + Results, + Progress); + + if (AllocatedRequest) { <--- if we've generated ConfigRequest string we need to free it + FreePool(ConfigRequest); + if (Request == NULL) { <--- also in this case BlockToConfig sets the Progress pointer to the ConfigRequest, but we need to sets this pointer to original Request + *Progress = NULL; + } else if (StrStr(Request, L"OFFSET") == NULL) { + *Progress = Request + StrLen(Request); + } + } + + return Status; +} +``` +The pattern above is widely used in the EDKII. When you would investigate other drivers keep in mind that usually instead of writing `StrLen(L"&OFFSET=0&WIDTH=") + sizeof(UINT64)*2` the driver developers just write its calculated value, which is 32. + +# `HiiIsConfigHdrMatch` + +The last thing that is usually seen in the driver code is the check that `GUID` and `NAME` in the request inedeed match the current driver. The check is performed with the `HiiIsConfigHdrMatch` function from the `HiiLib` [https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiHiiLib/HiiLib.c](https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/UefiHiiLib/HiiLib.c) +```cpp +/** + Determines if the routing data specified by GUID and NAME match a . + If ConfigHdr is NULL, then ASSERT(). + @param[in] ConfigHdr Either or . + @param[in] Guid GUID of the storage. + @param[in] Name NAME of the storage. + @retval TRUE Routing information matches . + @retval FALSE Routing information does not match . +**/ +BOOLEAN +EFIAPI +HiiIsConfigHdrMatch ( + IN CONST EFI_STRING ConfigHdr, + IN CONST EFI_GUID *Guid OPTIONAL, + IN CONST CHAR16 *Name OPTIONAL + ) +``` +This function expects that incoming configuration string is not equal to `NULL`, therefore the it can be used like this: +```cpp +if ((Request != NULL) && !HiiIsConfigHdrMatch(Request, &StorageGuid, StorageName)) { + return EFI_NOT_FOUND; +} +``` +`HiiIsConfigHdrMatch` is more useful when you driver have several storages and you check which one currently is requested. + +Anyway with this check our final version for the `ExportConfig` function looks like this: +```cpp +STATIC +EFI_STATUS +EFIAPI +ExtractConfig ( + IN CONST EFI_HII_CONFIG_ACCESS_PROTOCOL *This, + IN CONST EFI_STRING Request, + OUT EFI_STRING *Progress, + OUT EFI_STRING *Results +) +{ + if (Progress == NULL || Results == NULL) { + return EFI_INVALID_PARAMETER; + } + + if ((Request != NULL) && !HiiIsConfigHdrMatch(Request, &StorageGuid, StorageName)) { + return EFI_NOT_FOUND; + } + + BOOLEAN AllocatedRequest = FALSE; + EFI_STRING ConfigRequest = Request; + + if ((Request == NULL) || (StrStr (Request, L"OFFSET") == NULL)) { + EFI_STRING ConfigRequestHdr = HiiConstructConfigHdr(&StorageGuid, StorageName, mDriverHandle); + UINTN Size = (StrLen(ConfigRequestHdr) + StrLen(L"&OFFSET=0&WIDTH=") + sizeof(UINTN)*2 + 1) * sizeof(CHAR16); + ConfigRequest = AllocateZeroPool(Size); + AllocatedRequest = TRUE; + UnicodeSPrint(ConfigRequest, Size, L"%s&OFFSET=0&WIDTH=%016LX", ConfigRequestHdr, sizeof(VARIABLE_STRUCTURE)); + FreePool(ConfigRequestHdr); + } + + EFI_STATUS Status = gHiiConfigRouting->BlockToConfig(gHiiConfigRouting, + ConfigRequest, + (UINT8*)&FormStorage, + sizeof(VARIABLE_STRUCTURE), + Results, + Progress); + + if (AllocatedRequest) { + FreePool(ConfigRequest); + if (Request == NULL) { + *Progress = NULL; + } else if (StrStr(Request, L"OFFSET") == NULL) { + *Progress = Request + StrLen(Request); + } + } + + return Status; +} +``` + +You can rebuild our driver and check that it still works correctly. As I've pointed earlier you wouldn't see any differences in the driver responses. +But if you'll try to debug the driver calls, you'll see that now `ExportConfig()` call from the `HIIConfig.efi dump` command is issued only once with a `NULL` request string. As now our driver handles such request successfully the `ExportConfig()` doesn't have any need to parse IFR and construct a full request. + -- cgit v1.2.3-18-g5258