Re: Return EFI_INVALID_PARAMETER if attribute only has EFI_VARIABLE_NON_VOLATILE set

["Heinrich Schuchardt" <xypron.glpk@gmx.de>]

On 10/21/21 12:18, Sunny Wang wrote:
> Hi Liming, Hao, and all
>
> Now we’re checking the SCT runtime variable service test case.
> https://github.com/tianocore/edk2-test/blob/92a0343c1553342c53fae9d9d646b763add232c0/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestConformance.c#L3401
> <https://github.com/tianocore/edk2-test/blob/92a0343c1553342c53fae9d9d646b763add232c0/uefi-sct/SctPkg/TestCase/UEFI/EFI/RuntimeServices/VariableServices/BlackBoxTest/VariableServicesBBTestConformance.c#L3401>
> and have a question below.
>
> Is there any use case to call the runtime variable service functions
> with the Attributes that only has EFI_VARIABLE_NON_VOLATILE set?
>
> We checked UEFI spec, documents, and current EDK2 implementation. There
> is no specific description or any implementation for this. However,
> there seems an implication that EFI_VARIABLE_NON_VOLATILE must be set
> with at least EFI_VARIABLE_BOOTSERVICE_ACCESS.  Actually, it looks like
> making NO sense to have a variable attribute combination that doesn’t
> have any XXXXX_ACCESS attribute (BS, RT, or AT) set.
>
> Therefore, we think only having EFI_VARIABLE_NON_VOLATILE set may be an
> invalid case and would like to add a check into the EDK2 variable driver
> to return EFI_INVALID_PARAMETER.  What do you guys think?

The Self Certification Test (SCT) II Case Specification, June 2017
explicitly forbids this value for QueryVariableInfo():

<cite>
5.2.1.4.5

  Call QueryVariableInfo service with the Attributes:

* EFI_VARIABLE_NON_VOLATILE
* EFI_VARIABLE_RUNTIME_ACCESS
* EFI_VARIABLE_NON_VOLATILE|EFI_VARIABLE_RUNTIME_ACCESS

The returned code must be EFI_INVALID_PARAMETER.
</cite>

This corresponds to the UEFI specification saying:
<cite>
QueryVariableInfo()
Status Codes returned
EFI_INVALID_PARAMETER:
An invalid combination of attribute bits was supplied.
</cite>

A variable being not accessible at BootTime seems not to be foreseen by
the specification:

<cite>
SetVariable()
...
If software uses a nonvolatile variable, it should use a variable that
is only accessible at boot services time if possible.
...
Attributes that have EFI_VARIABLE_RUNTIME_ACCESS set must also have
EFI_VARIABLE_BOOTSERVICE_ACCESS set.
</cite>

This sounds like a nonvolatile variable should always be accessible at
boot services time. But an explicit rule forbidding the creation of
inaccessible variables, i.e. without EFI_VARIABLE_BOOTSERVICE_ACCESS, is
missing.

It would be good to have a paragraph in the specification that
unambiguously defines which combination of attribute bits are valid.

How about:

"All variables must be created with attribute bit
EFI_VARIABLE_BOOTSERVICE_ACCESS."

Best regards

Heinrich

>
> Best Regards,
>
> Sunny