[PATCH] uefirtvariable: remove the invalid attribute tests after ExitBootServices() is performed

[IvanHu]

On 12/19/2012 07:00 PM, Colin Ian King wrote:
> On 18/12/12 06:15, IvanHu wrote:
>> On 12/14/2012 06:52 PM, Colin Ian King wrote:
>>> On 14/12/12 09:48, Ivan Hu wrote:
>>>> In the UEFI spec Version 2.3.1, Errata C, section 7.2, it mentions
>>>> that attributes have some usage rules:
>>>> * Attributes that have EFI_VARIABLE_RUNTIME_ACCESS set must also have
>>>>    EFI_VARIABLE_BOOTSERVICE_ACCESS set.
>>>>
>>>> * Once ExitBootServices() is performed, only variables that have
>>>>    EFI_VARIABLE_RUNTIME_ACCESS and EFI_VARIABLE_NON_VOLATILE set can be
>>>>    set with SetVariable().
>>>
>>> I'm still a bit uncertain about this.
>>>
>>> So what happens if we try to set variables with the wrong attributes?
>>> Should the firmware return an error?  In which case, isn't it valid to
>>> test with invalid attributes to make sure the firmware rejects this?
>>>
>>> Colin
>>>
>>
>> Hi Colin,
>>
>> the UEFI spec Version 2.3.1, Errata C p.223, related to the setvariable
>> are below:
>> • Runtime access to a data variable implies boot service access.
>> Attributes that have
>>     EFI_VARIABLE_RUNTIME_ACCESS set must also have
>>    EFI_VARIABLE_BOOTSERVICE_ACCESS set. The caller is responsible for
>> following this rule.
>> • Once ExitBootServices() is performed, data variables that did not have
>>     EFI_VARIABLE_RUNTIME_ACCESS set are no longer visible to
>> GetVariable().
>> • Once ExitBootServices() is performed, only variables that have
>>     EFI_VARIABLE_RUNTIME_ACCESS and EFI_VARIABLE_NON_VOLATILE set can be
>>    set with SetVariable(). Variables that have runtime access but that
>> are not nonvolatile are read-only data variables once ExitBootServices()
>> is performed.
>>
>> When ExitBootServices is performed(power on to Ubuntu), the POST
>> resource of firmware will be released, only runtime resource reserved.
>> But, actually UEFI spec doesn't tell what should be done for firmware
>> when the invalid attribute is set under this situation. So the return
>> status depends on firmware implementation. Checking with the returen
>> status on spec, *EFI_INVALID_PARAMETER - An invalid combination of
>> attribute bits was supplied, or the DataSize exceeds the maximum
>> allowed.* should be the most likely be returned by most firmware. I
>> built the EDK2, and powered on and tested the Bios with Qemu. It did
>> return EFI_INVALID_PARAMETER. So I think it's worth to add another patch
>> to test the invalid attribute in SetVariable function test, subtest3, to
>> check and give warnings or failure if firmware returns EFI_SUCCESS under
>> this situation. Make sense?
>
> Sounds sensible to me. Let's see if any firmware fails this new test ;-)

Thanks!. I've sent another patch to the setvariable with invalid 
attributes. I think, this patch is also needed for removing the 
setvariable fail within getvariable, getnextvaribal and setvariable 
function tests.