Hi Heinrich,
2021年6月30日(水) 23:55 Heinrich Schuchardt xypron.glpk@gmx.de:
This does not catch attributes == EFI_VARIABLE_NON_VOLATILE where test case 5.2.1.4.5 requires EFI_INVALID_PARAMETER.
Hmm, but this could pass the SCT runtime test. So attributes == EFI_VARIABLES_NON_VOLATILE should fail? Actually, I referred to the VariableServiceQueryVariableInfo()@EDK2 and UEFI spec, and I couldn't see such conditions.
The SCT specification requires in 5.2.1.4.5.:
"1. Call QueryVariableInfoservice 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."
Ah, I see. so either NON_VOLATILE and RUNTIME_ACCESS must be used with BOOTSERVICE_ACCESS.
See patch
[PATCH edk2-test 1/1] uefi-sct/SctPkg: uefi-sct: QueryVariableInfo(EFI_VARIABLE_NON_VOLATILE) https://edk2.groups.io/g/devel/message/77367
Shouldn't we check
!(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS)instead?
Ah, right, because this function is only used for the bootservice. (I found that runtime service uses another function)
A variable without EFI_VARIABLE_BOOTSERVICE_ACCESS could neither be accessed before nor after ExitBootServices(). So this has to be invalid.
OK, so BOOTSERVICE_ACCESS is required. Hmm, but in that case, if we confirm BOOTSERVICE_ACCESS bit is set, should we still need to check NON_VOLATILE and RUNTIME_ACCESS?
I mean
if (!(attr & BOOTSERVICE_ACCESS)) return INVALID_PARAM; else if (...) /* at this point, attr must have the BOOTSERVICE_ACCESS */
Thus, attributes shouldn't be equal to any of;
- EFI_VARIABLE_NON_VOLATILE
- EFI_VARIABLE_RUNTIME_ACCESS
- EFI_VARIABLE_NON_VOLATILE|EFI_VARIABLE_RUNTIME_ACCESS
So, I think we only need " !(attributes & EFI_VARIABLE_BOOTSERVICE_ACCESS) " this check.
Best regards,
Best regards
Heinrich
ret = EFI_INVALID_PARAMETER;} else if ((attributes & (EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_HARDWARE_ERROR_RECORD)) == EFI_VARIABLE_HARDWARE_ERROR_RECORD) {/* HW error occurs only on non-volatile variables */We don't support EFI_VARIABLE_HARDWARE_ERROR_RECORD at all. So shouldn't flag EFI_VARIABLE_HARDWARE_ERROR_RECORD result in EFI_UNSUPPORTED?
OK, I'll do.
BTW, from the UEFI spec, EFI_VARIABLE_APPEND_WRITE should be ignored in the QueryVariableInfo because that flag is used for SetVariables (as overwrite flag). Thus, if attributes == EFI_VARIABLE_APPEND_WRITE, should we return EFI_INVALID_PARAMETER?
ret = EFI_INVALID_PARAMETER;} else {ret = efi_query_variable_info_int(attributes, maximum_variable_storage_size, remaining_variable_storage_size, maximum_variable_size);CHECK: Alignment should match open parenthesis #44: FILE: lib/efi_loader/efi_var_common.c:184:
ret = efi_query_variable_info_int(attributes, maximum_variable_storage_size,OK, let me fix that.
Thank you,
Best regards
Heinrich
} return EFI_EXIT(ret);}
-- Masami Hiramatsu