Skip to content

Commit

Permalink
Allow arbitrary SVD values to be set through ConfApp (microsoft#77)
Browse files Browse the repository at this point in the history 
## Description

Currently, mu_feature_config is in a transition to an XML based backend.
The current YAML based ConfApp expects SVDs with the IDs
`"Device.ConfigData.*"`. However, the XML has no such restriction. In
the case of the XML, we want to allow arbitrary SVD provided variables
to be set, as this option is only provided in the manufacturing mode
state (used often for bringup and debug).

This change leaves the YAML processing in place until more XML pieces
are set to remove the remaining YAML pieces.

For each item, place an "x" in between `[` and `]` if true. Example:
`[x]`.
_(you can also check items in the GitHub UI)_

- [x] Impacts functionality?
- [ ] Impacts security?
- [ ] Breaking change?
- [x] Includes tests?
- [ ] Includes documentation?

## How This Was Tested

Tested with unit tests and in QemuQ35 by booting to the ConfApp in
manufacturing mode, applying an SVD with arbitrary variables, rebooting
to the EFI shell, using dmpstore to dump the variables to a file,
applying that file in Config Editor and seeing that the current variable
state matches what was provided in the SVD.

## Integration Instructions

N/A
  • Loading branch information
@os-d
os-d authored Dec 15, 2022
1 parent 81cec89 commit 4a08dbe
Show file tree
Hide file tree
Showing 4 changed files with 335 additions and 43 deletions.
202 changes: 161 additions & 41 deletions SetupDataPkg/ConfApp/SetupConf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,123 @@ PrintOptions (
return Status;
}

/**
Set arbitrary SVD values to variable storage
@param Value a pointer to the variable list
@param ValueSize Size of the data for this setting.
@retval EFI_SUCCESS If setting could be set. Check flags for other info (reset required, etc)
@retval Error Setting not set.
**/
STATIC
EFI_STATUS
WriteSVDSetting (
IN CONST UINT8 *Value,
IN UINTN ValueSize
)
{
EFI_STATUS Status = EFI_SUCCESS;
CHAR16 *AlignedVarName = NULL;
CHAR16 *UnalignedVarName;
EFI_GUID *Guid;
UINT32 Attributes;
CHAR8 *Data;
UINT32 CRC32;
UINT32 LenToCRC32;
UINT32 CalcCRC32 = 0;
CONFIG_VAR_LIST_HDR *VarList;
UINT32 ListIndex = 0;

if ((Value == NULL) || (ValueSize == 0)) {
return EFI_INVALID_PARAMETER;
}

while (ListIndex < ValueSize) {
// index into variable list
VarList = (CONFIG_VAR_LIST_HDR *)(Value + ListIndex);

if (ListIndex + sizeof (*VarList) + VarList->NameSize + VarList->DataSize + sizeof (*Guid) +
sizeof (Attributes) + sizeof (CRC32) > ValueSize)
{
// the NameSize and DataSize have bad values and are pushing us past the end of the binary
DEBUG ((DEBUG_ERROR, "SVD settings had bad NameSize or DataSize, unable to process all settings\n"));
Status = EFI_COMPROMISED_DATA;
break;
}

/*
* Var List is in DmpStore format:
*
* struct {
* CONFIG_VAR_LIST_HDR VarList;
* CHAR16 Name[VarList->NameSize/2];
* EFI_GUID Guid;
* UINT32 Attributes;
* CHAR8 Data[VarList->DataSize];
* UINT32 CRC32; // CRC32 of all bytes from VarList to end of Data
* }
*/
UnalignedVarName = (CHAR16 *)(VarList + 1);
Guid = (EFI_GUID *)((CHAR8 *)UnalignedVarName + VarList->NameSize);
Attributes = *(UINT32 *)(Guid + 1);
Data = (CHAR8 *)Guid + sizeof (*Guid) + sizeof (Attributes);
CRC32 = *(UINT32 *)(Data + VarList->DataSize);
LenToCRC32 = sizeof (*VarList) + VarList->NameSize + VarList->DataSize + sizeof (*Guid) + sizeof (Attributes);

// on next iteration, skip past this variable
ListIndex += LenToCRC32 + sizeof (CRC32);

// validate CRC32
CalcCRC32 = CalculateCrc32 (VarList, LenToCRC32);

if (CalcCRC32 != CRC32) {
// CRC didn't match, drop this variable, continue to the next
DEBUG ((DEBUG_ERROR, "SVD setting failed CRC check, skipping applying setting: %a Status: %r \
Received CRC32: %u Calculated CRC32: %u\n", UnalignedVarName, Status, CRC32, CalcCRC32));
continue;
}

AlignedVarName = AllocatePool (VarList->NameSize);
if (AlignedVarName == NULL) {
Status = EFI_OUT_OF_RESOURCES;
return Status;
}

// Copy non-16 bit aligned UnalignedVarName to AlignedVarName, not using StrCpyS functions as they assert on non-16 bit alignment
CopyMem (AlignedVarName, UnalignedVarName, VarList->NameSize);

// first delete the variable in case of size or attributes change, not validated here as this is only allowed
// in manufacturing mode. Don't retrieve the status, if we fail to delete, try to write it anyway. If we fail
// there, just log it and move on
gRT->SetVariable (
AlignedVarName,
Guid,
0,
0,
NULL
);

// write variable directly to var storage
Status = gRT->SetVariable (
AlignedVarName,
Guid,
Attributes,
VarList->DataSize,
Data
);

if (EFI_ERROR (Status)) {
// failed to set variable, continue to try with other variables
DEBUG ((DEBUG_ERROR, "Failed to set SVD Setting %s, continuing to try next variables\n", AlignedVarName));
}

FreePool (AlignedVarName);
}

return Status;
}

/**
Apply all settings from XML to their associated setting providers.
Expand Down Expand Up @@ -357,13 +474,6 @@ ApplySettings (
goto EXIT;
}

// only care about our target
ZeroMem (&VarListEntry, sizeof (VarListEntry));
Status = QuerySingleActiveConfigAsciiVarList (Id, &VarListEntry);
if (EFI_ERROR (Status)) {
continue;
}

// Now we have an Id and Value
b64Size = AsciiStrnLenS (Value, MAX_ALLOWABLE_DFCI_APPLY_VAR_SIZE);
ValueSize = 0;
Expand All @@ -387,45 +497,55 @@ ApplySettings (
DEBUG ((DEBUG_INFO, "Setting BINARY data\n"));
DUMP_HEX (DEBUG_VERBOSE, 0, SetValue, ValueSize, "");

if (mSettingAccess == NULL) {
// Should not be here
Status = EFI_NOT_STARTED;
goto EXIT;
}
if ((0 == AsciiStrnCmp (Id, DFCI_OEM_SETTING_ID__CONF, AsciiStrLen (DFCI_OEM_SETTING_ID__CONF))) ||
(0 == AsciiStrnCmp (Id, SINGLE_SETTING_PROVIDER_START, AsciiStrLen (SINGLE_SETTING_PROVIDER_START))))
{
// only care about our target
ZeroMem (&VarListEntry, sizeof (VarListEntry));
Status = QuerySingleActiveConfigAsciiVarList (Id, &VarListEntry);
if (EFI_ERROR (Status)) {
FreePool (ByteArray);
continue;
}

// Now set the settings
Status = mSettingAccess->Set (
mSettingAccess,
Id,
&mAuthToken,
DFCI_SETTING_TYPE_BINARY,
ValueSize,
SetValue,
&Flags
);
DEBUG ((DEBUG_INFO, "%a - Set %a = %a. Result = %r\n", __FUNCTION__, Id, Value, Status));
// Now set the settings via DFCI
Status = mSettingAccess->Set (
mSettingAccess,
Id,
&mAuthToken,
DFCI_SETTING_TYPE_BINARY,
ValueSize,
SetValue,
&Flags
);

// Record Status result
ZeroMem (StatusString, sizeof (StatusString));
ZeroMem (FlagString, sizeof (FlagString));
StatusString[0] = '0';
StatusString[1] = 'x';
FlagString[0] = '0';
FlagString[1] = 'x';

AsciiValueToStringS (&(StatusString[2]), sizeof (StatusString)-2, RADIX_HEX, (INT64)Status, 18);
AsciiValueToStringS (&(FlagString[2]), sizeof (FlagString)-2, RADIX_HEX, (INT64)Flags, 18);
Status = SetOutputSettingsStatus (ResultSettingsNode, Id, &(StatusString[0]), &(FlagString[0]));
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Failed to SetOutputSettingStatus. %r\n", Status));
Status = EFI_DEVICE_ERROR;
goto EXIT;
}
// Record Status result
ZeroMem (StatusString, sizeof (StatusString));
ZeroMem (FlagString, sizeof (FlagString));
StatusString[0] = '0';
StatusString[1] = 'x';
FlagString[0] = '0';
FlagString[1] = 'x';

AsciiValueToStringS (&(StatusString[2]), sizeof (StatusString)-2, RADIX_HEX, (INT64)Status, 18);
AsciiValueToStringS (&(FlagString[2]), sizeof (FlagString)-2, RADIX_HEX, (INT64)Flags, 18);
Status = SetOutputSettingsStatus (ResultSettingsNode, Id, &(StatusString[0]), &(FlagString[0]));
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Failed to SetOutputSettingStatus. %r\n", Status));
Status = EFI_DEVICE_ERROR;
goto EXIT;
}

if (Flags & DFCI_SETTING_FLAGS_OUT_REBOOT_REQUIRED) {
ResetRequired = TRUE;
if (Flags & DFCI_SETTING_FLAGS_OUT_REBOOT_REQUIRED) {
ResetRequired = TRUE;
}
} else {
// this is not a DFCI variable, just write the variable
Status = WriteSVDSetting (SetValue, ValueSize);
}

DEBUG ((DEBUG_INFO, "%a - Set %a = %a. Result = %r\n", __FUNCTION__, Id, Value, Status));

// all done.
} // end for loop

Expand Down
102 changes: 102 additions & 0 deletions SetupDataPkg/ConfApp/UnitTest/ConfAppSetupConfUnitTest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,107 @@ ConfAppSetupConfSelectSerial (
return UNIT_TEST_PASSED;
}

/**
Unit test for SetupConf page when selecting configure from serial and passing in arbitrary SVD variables.
@param[in] Context [Optional] An optional parameter that enables:
1) test-case reuse with varied parameters and
2) test-case re-entry for Target tests that need a
reboot. This parameter is a VOID* and it is the
responsibility of the test author to ensure that the
contents are well understood by all test cases that may
consume it.
@retval UNIT_TEST_PASSED The Unit test has completed and the test
case was successful.
@retval UNIT_TEST_ERROR_TEST_FAILED A test case assertion has failed.
**/
UNIT_TEST_STATUS
EFIAPI
ConfAppSetupConfSelectSerialWithArbitrarySVD (
IN UNIT_TEST_CONTEXT Context
)
{
EFI_STATUS Status;
EFI_KEY_DATA KeyData1;
UINTN Index;
CHAR8 *KnowGoodXml;
BASE_LIBRARY_JUMP_BUFFER JumpBuf;

will_return (MockClearScreen, EFI_SUCCESS);
will_return_always (MockSetAttribute, EFI_SUCCESS);

will_return (MockGetVariable, 0);
will_return (MockGetVariable, NULL);
will_return (MockGetVariable, EFI_NOT_FOUND);

// Expect the prints twice
expect_any (MockSetCursorPosition, Column);
expect_any (MockSetCursorPosition, Row);
will_return (MockSetCursorPosition, EFI_SUCCESS);

// Initial run
Status = SetupConfMgr ();
UT_ASSERT_NOT_EFI_ERROR (Status);
UT_ASSERT_EQUAL (mSetupConfState, SetupConfWait);

mSimpleTextInEx = &MockSimpleInput;

KeyData1.Key.UnicodeChar = '3';
KeyData1.Key.ScanCode = SCAN_NULL;
will_return (MockReadKey, &KeyData1);

Status = SetupConfMgr ();
UT_ASSERT_NOT_EFI_ERROR (Status);
UT_ASSERT_EQUAL (mSetupConfState, SetupConfUpdateSerialHint);

Index = 0;
KnowGoodXml = KNOWN_GOOD_VARLIST_XML;
while (KnowGoodXml[Index] != 0) {
KeyData1.Key.UnicodeChar = KnowGoodXml[Index];
KeyData1.Key.ScanCode = SCAN_NULL;
will_return (MockReadKey, &KeyData1);
Status = SetupConfMgr ();
Index++;
}

KeyData1.Key.UnicodeChar = CHAR_CARRIAGE_RETURN;
KeyData1.Key.ScanCode = SCAN_NULL;
will_return (MockReadKey, &KeyData1);

will_return_always (MockSetVariable, EFI_SUCCESS);

// first time through is delete
expect_memory (MockSetVariable, VariableName, L"COMPLEX_KNOB1a", StrSize (L"COMPLEX_KNOB1a"));
expect_memory (MockSetVariable, VendorGuid, &mKnown_Good_Xml_Guid_LE, sizeof (mKnown_Good_Xml_Guid_LE));
expect_value (MockSetVariable, DataSize, 0x00);
expect_value (MockSetVariable, Data, NULL);

expect_memory (MockSetVariable, VariableName, L"COMPLEX_KNOB1a", StrSize (L"COMPLEX_KNOB1a"));
expect_memory (MockSetVariable, VendorGuid, &mKnown_Good_Xml_Guid_LE, sizeof (mKnown_Good_Xml_Guid_LE));
expect_value (MockSetVariable, DataSize, 0x09);
expect_memory (MockSetVariable, Data, mKnown_Good_Xml_VarList_Entries[0], 0x09);

// first time through is delete
expect_memory (MockSetVariable, VariableName, L"INTEGER_KNOB", StrSize (L"INTEGER_KNOB"));
expect_memory (MockSetVariable, VendorGuid, &mKnown_Good_Xml_Guid_LE, sizeof (mKnown_Good_Xml_Guid_LE));
expect_value (MockSetVariable, DataSize, 0x00);
expect_value (MockSetVariable, Data, NULL);

expect_memory (MockSetVariable, VariableName, L"INTEGER_KNOB", StrSize (L"INTEGER_KNOB"));
expect_memory (MockSetVariable, VendorGuid, &mKnown_Good_Xml_Guid_LE, sizeof (mKnown_Good_Xml_Guid_LE));
expect_value (MockSetVariable, DataSize, 0x04);
expect_memory (MockSetVariable, Data, mKnown_Good_Xml_VarList_Entries[1], 0x04);

will_return (ResetCold, &JumpBuf);

if (!SetJump (&JumpBuf)) {
SetupConfMgr ();
}

return UNIT_TEST_PASSED;
}

/**
Unit test for SetupConf page when selecting configure from serial and return in the middle.
Expand Down Expand Up @@ -1086,6 +1187,7 @@ UnitTestingEntry (
AddTestCase (MiscTests, "Setup Configuration page select others should do nothing", "SelectOther", ConfAppSetupConfSelectOther, NULL, SetupConfCleanup, NULL);
AddTestCase (MiscTests, "Setup Configuration page should setup configuration from USB", "SelectUsb", ConfAppSetupConfSelectUsb, SetupConfPrerequisite, SetupConfCleanup, &Context);
AddTestCase (MiscTests, "Setup Configuration page should setup configuration from serial", "SelectSerial", ConfAppSetupConfSelectSerial, SetupConfPrerequisite, SetupConfCleanup, &Context);
AddTestCase (MiscTests, "Setup Configuration page should setup configuration from serial", "SelectSerialWithArbitrarySVD", ConfAppSetupConfSelectSerialWithArbitrarySVD, SetupConfPrerequisite, SetupConfCleanup, &Context);
AddTestCase (MiscTests, "Setup Configuration page should return with ESC key during serial transport", "SelectSerial", ConfAppSetupConfSelectSerialEsc, NULL, SetupConfCleanup, NULL);
AddTestCase (MiscTests, "Setup Configuration page should dump all configurations from serial", "ConfDump", ConfAppSetupConfDumpSerial, SetupConfPrerequisite, SetupConfCleanup, &Context);
AddTestCase (MiscTests, "Setup Configuration page should ignore updating configurations when in non-mfg mode", "ConfNonMfg", ConfAppSetupConfNonMfg, NULL, SetupConfCleanup, NULL);
Expand Down
Loading

0 comments on commit 4a08dbe

Please sign in to comment.