From 09abc636756b5d1f29224402181f7fd34f736c5f Mon Sep 17 00:00:00 2001 From: Hao Wu Date: Wed, 20 Apr 2016 13:23:59 +0800 Subject: [PATCH] MdeModulePkg RamDiskDxe: Fix wrong HII behavior for more than 8 RAM disks The RamDiskDxe driver originally uses a variable-length HII varstore to retrieve the HII checkbox status of each registered RAM disk. However, HII does not support the variable-length varstore feature. Therefore, only the checkbox status for the first 8 RAM disks are tracked for the following definition of HII varstore structure considering the alignment: typedef struct { UINT64 Size; UINT8 RamDiskList[0]; } RAM_DISK_CONFIGURATION; This commit uses the private data of each registered RAM disks to track the HII checkbox status instead to resolve the issue. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Hao Wu Reviewed-by: Feng Tian --- .../Universal/Disk/RamDiskDxe/RamDiskDriver.c | 1 - .../Universal/Disk/RamDiskDxe/RamDiskImpl.c | 47 ++++++++++++------- .../Universal/Disk/RamDiskDxe/RamDiskImpl.h | 5 +- .../Universal/Disk/RamDiskDxe/RamDiskNVData.h | 13 +++-- .../Disk/RamDiskDxe/RamDiskProtocol.c | 2 - 5 files changed, 38 insertions(+), 30 deletions(-) diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c index e65aee8021..d1dd13a819 100644 --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDriver.c @@ -32,7 +32,6 @@ EFI_RAM_DISK_PROTOCOL mRamDiskProtocol = { // RamDiskDxe driver maintains a list of registered RAM disks. // LIST_ENTRY RegisteredRamDisks; -UINTN ListEntryNum; // // Pointers to the EFI_ACPI_TABLE_PROTOCOL and EFI_ACPI_SDT_PROTOCOL. diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c index 29dcbf72f7..f402440e45 100644 --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c @@ -193,7 +193,6 @@ UnregisterAllRamDisks ( FreePool (PrivateData->DevicePath); FreePool (PrivateData); - ListEntryNum--; } } } @@ -263,7 +262,7 @@ RamDiskExtractConfig ( // Convert buffer data to by helper function BlockToConfig() // ConfigPrivate = RAM_DISK_CONFIG_PRIVATE_FROM_THIS (This); - BufferSize = sizeof (RAM_DISK_CONFIGURATION) + ListEntryNum; + BufferSize = sizeof (RAM_DISK_CONFIGURATION); Configuration = AllocateZeroPool (BufferSize); if (Configuration == NULL) { return EFI_OUT_OF_RESOURCES; @@ -557,8 +556,14 @@ UpdateMainForm ( Index = 0; EFI_LIST_FOR_EACH (Entry, &RegisteredRamDisks) { - PrivateData = RAM_DISK_PRIVATE_FROM_THIS (Entry); - String = RamDiskStr; + PrivateData = RAM_DISK_PRIVATE_FROM_THIS (Entry); + PrivateData->CheckBoxId = (EFI_QUESTION_ID) + (MAIN_CHECKBOX_QUESTION_ID_START + Index); + // + // CheckBox is unchecked by default. + // + PrivateData->CheckBoxChecked = FALSE; + String = RamDiskStr; UnicodeSPrint ( String, @@ -574,12 +579,12 @@ UpdateMainForm ( HiiCreateCheckBoxOpCode ( StartOpCodeHandle, - (EFI_QUESTION_ID) (MAIN_CHECKBOX_QUESTION_ID_START + Index), - RAM_DISK_CONFIGURATION_VARSTORE_ID, - (UINT16) (RAM_DISK_LIST_VAR_OFFSET + Index), + PrivateData->CheckBoxId, + 0, + 0, StringId, STRING_TOKEN (STR_RAM_DISK_LIST_HELP), - 0, + EFI_IFR_FLAG_CALLBACK, 0, NULL ); @@ -634,7 +639,6 @@ RamDiskCallback ( ) { EFI_STATUS Status; - UINTN Index; RAM_DISK_PRIVATE_DATA *PrivateData; RAM_DISK_CONFIG_PRIVATE_DATA *ConfigPrivate; RAM_DISK_CONFIGURATION *Configuration; @@ -679,7 +683,7 @@ RamDiskCallback ( // // Get Browser data // - Configuration = AllocateZeroPool (sizeof (RAM_DISK_CONFIGURATION) + ListEntryNum); + Configuration = AllocateZeroPool (sizeof (RAM_DISK_CONFIGURATION)); if (Configuration == NULL) { return EFI_OUT_OF_RESOURCES; } @@ -689,7 +693,7 @@ RamDiskCallback ( HiiGetBrowserData ( &gRamDiskFormSetGuid, mRamDiskStorageName, - sizeof (RAM_DISK_CONFIGURATION) + ListEntryNum, + sizeof (RAM_DISK_CONFIGURATION), (UINT8 *) Configuration ); @@ -742,11 +746,9 @@ RamDiskCallback ( // // Remove the selected RAM disks // - Index = 0; EFI_LIST_FOR_EACH_SAFE (Entry, NextEntry, &RegisteredRamDisks) { - if (Configuration->RamDiskList[Index++] != 0) { - PrivateData = RAM_DISK_PRIVATE_FROM_THIS (Entry); - + PrivateData = RAM_DISK_PRIVATE_FROM_THIS (Entry); + if (PrivateData->CheckBoxChecked) { RamDiskUnregister ( (EFI_DEVICE_PATH_PROTOCOL *) PrivateData->DevicePath ); @@ -756,7 +758,6 @@ RamDiskCallback ( UpdateMainForm (ConfigPrivate); *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_APPLY; - ZeroMem (Configuration->RamDiskList, ListEntryNum); break; case CREATE_RAW_SUBMIT_QUESTION_ID: @@ -781,6 +782,18 @@ RamDiskCallback ( break; default: + // + // QuestionIds for checkboxes + // + if ((QuestionId >= MAIN_CHECKBOX_QUESTION_ID_START) && + (QuestionId < CREATE_RAW_RAM_DISK_FORM_ID)) { + EFI_LIST_FOR_EACH (Entry, &RegisteredRamDisks) { + PrivateData = RAM_DISK_PRIVATE_FROM_THIS (Entry); + if (PrivateData->CheckBoxId == QuestionId) { + PrivateData->CheckBoxChecked = (BOOLEAN) (Value->u8 != 0); + } + } + } break; } } @@ -789,7 +802,7 @@ RamDiskCallback ( HiiSetBrowserData ( &gRamDiskFormSetGuid, mRamDiskStorageName, - sizeof (RAM_DISK_CONFIGURATION) + ListEntryNum, + sizeof (RAM_DISK_CONFIGURATION), (UINT8 *) Configuration, NULL ); diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h index 2fcc89f046..d660ab9366 100644 --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h @@ -71,7 +71,6 @@ // RamDiskDxe driver maintains a list of registered RAM disks. // extern LIST_ENTRY RegisteredRamDisks; -extern UINTN ListEntryNum; // // Pointers to the EFI_ACPI_TABLE_PROTOCOL and EFI_ACPI_SDT_PROTOCOL. @@ -108,6 +107,8 @@ typedef struct { UINT16 InstanceNumber; RAM_DISK_CREATE_METHOD CreateMethod; BOOLEAN InNfit; + EFI_QUESTION_ID CheckBoxId; + BOOLEAN CheckBoxChecked; LIST_ENTRY ThisInstance; } RAM_DISK_PRIVATE_DATA; @@ -145,8 +146,6 @@ extern RAM_DISK_CONFIG_PRIVATE_DATA mRamDiskConfigPrivateDataTemplate; #define RAM_DISK_CONFIG_PRIVATE_DATA_SIGNATURE SIGNATURE_32 ('R', 'C', 'F', 'G') #define RAM_DISK_CONFIG_PRIVATE_FROM_THIS(a) CR (a, RAM_DISK_CONFIG_PRIVATE_DATA, ConfigAccess, RAM_DISK_CONFIG_PRIVATE_DATA_SIGNATURE) -#define RAM_DISK_LIST_VAR_OFFSET ((UINT16) OFFSET_OF (RAM_DISK_CONFIGURATION, RamDiskList)) - /** Register a RAM disk with specified address, size and type. diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskNVData.h b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskNVData.h index 2b5d045198..1426320121 100644 --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskNVData.h +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskNVData.h @@ -23,9 +23,9 @@ #define MAIN_FORM_ID 0x1000 #define MAIN_GOTO_FILE_EXPLORER_ID 0x1001 #define MAIN_REMOVE_RD_QUESTION_ID 0x1002 -#define MAIN_CHECKBOX_QUESTION_ID_START 0x1003 -#define MAIN_LABEL_LIST_START 0x1004 -#define MAIN_LABEL_LIST_END 0x1005 +#define MAIN_LABEL_LIST_START 0x1003 +#define MAIN_LABEL_LIST_END 0x1004 +#define MAIN_CHECKBOX_QUESTION_ID_START 0x1100 #define CREATE_RAW_RAM_DISK_FORM_ID 0x2000 #define CREATE_RAW_SIZE_QUESTION_ID 0x2001 @@ -33,11 +33,10 @@ #define CREATE_RAW_DISCARD_QUESTION_ID 0x2003 typedef struct { + // + // The size of the RAM disk to be created. + // UINT64 Size; - // - // CheckBox status for created RAM disks - // - UINT8 RamDiskList[0]; } RAM_DISK_CONFIGURATION; #endif diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c index 93b1b83877..ed71849fb4 100644 --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c @@ -694,7 +694,6 @@ RamDiskRegister ( // Insert the newly created one to the registered RAM disk list // InsertTailList (&RegisteredRamDisks, &PrivateData->ThisInstance); - ListEntryNum++; gBS->ConnectController (PrivateData->Handle, NULL, NULL, TRUE); @@ -829,7 +828,6 @@ RamDiskUnregister ( FreePool (PrivateData->DevicePath); FreePool (PrivateData); - ListEntryNum--; Found = TRUE; break;