On 10/14/15 04:16, Zeng, Star wrote:
> On 2015/10/13 19:55, Laszlo Ersek wrote:
>> The IsValidWorkSpace() function checks if the working block header of the
>> workspace is valid. A mismatch detected by this function is not
>> necessarily an error; it can happen with an as-yet unwritten flash chip,
>> which is e.g. common and normal when a new ArmVirtQemu virtual machine is
>> booted. Therefore downgrade the message emitted by IsValidWorkSpace()
>> from
>> EFI_D_ERROR to EFI_D_INFO, and change the wording from "error" to
>> "mismatch".
>>
>> The only caller of IsValidWorkSpace(), InitFtwProtocol(), handles all of
>> the following cases:
>>
>> (1) IsValidWorkSpace() succeeds for the working block -- this is normal
>> operation,
>>
>> (2) IsValidWorkSpace() fails for the working block, but succeeds for the
>> spare block -- InitFtwProtocol() then restores the working block
>> from
>> the spare block,
>>
>> (3) IsValidWorkSpace() fails for both the working and spare blocks --
>> InitFtwProtocol() reinitializes the full workspace.
>>
>> In cases (2) and (3), InitFtwProtocol() logs additional messages about
>> the
>> branch taken. Their current level is EFI_D_ERROR, but the messages are
>> arguably informative, not necessarily error reports.
>>
>> Downgrade these messages from EFI_D_ERROR to EFI_D_INFO, so that they
>> don't clutter the debug output when the PcdDebugPrintErrorLevel mask only
>> enables EFI_D_ERROR (i.e., in a "silent" build).
>>
>> These messages have annoyed / confused users; see for example:
>> - https://bugzilla.redhat.com/show_bug.cgi?id=1270279
>>
>> Cc: Star Zeng <[email protected]>
>> Cc: Liming Gao <[email protected]>
>> Cc: Drew Jones <[email protected]>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Laszlo Ersek <[email protected]>
>> ---
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c |
>> 6 ++++--
>> MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c |
>> 2 +-
>> 2 files changed, 5 insertions(+), 3 deletions(-)
>
> Basically, I am ok with the patch.
>
> Reviewed-by: Star Zeng <[email protected]>
>
> It will be nicer if it can also cover below lines.
> FaultTolerantWrite.c:
> FtwRestart():
> DEBUG ((EFI_D_ERROR, "Ftw: Restart() success \n")); -> EFI_D_INFO
> FtwAbort():
> DEBUG ((EFI_D_ERROR, "Ftw: Abort() success \n")); -> EFI_D_INFO
> FtwGetLastWrite():
> DEBUG ((EFI_D_ERROR, "Ftw: GetLasetWrite() success\n")); -> EFI_D_INFO
I will send an updated version for review.
Thanks!
Laszlo
>
>
> Thanks,
> Star
>>
>> diff --git a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>> index 0922321..9604469 100644
>> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/FtwMisc.c
>> @@ -1273,29 +1273,31 @@ InitFtwProtocol (
>> FtwDevice->FtwWorkSpaceSize,
>> FtwDevice->FtwWorkSpace
>> );
>> ASSERT_EFI_ERROR (Status);
>>
>> //
>> // If spare block is valid, then replace working block content.
>> //
>> if (IsValidWorkSpace (FtwDevice->FtwWorkSpaceHeader)) {
>> Status = FlushSpareBlockToWorkingBlock (FtwDevice);
>> - DEBUG ((EFI_D_ERROR, "Ftw: Restart working block update in
>> InitFtwProtocol() - %r\n", Status));
>> + DEBUG ((EFI_D_INFO, "Ftw: Restart working block update in %a()
>> - %r\n",
>> + __FUNCTION__, Status));
>> FtwAbort (&FtwDevice->FtwInstance);
>> //
>> // Refresh work space.
>> //
>> Status = WorkSpaceRefresh (FtwDevice);
>> ASSERT_EFI_ERROR (Status);
>> } else {
>> - DEBUG ((EFI_D_ERROR, "Ftw: Both are invalid, init workspace\n"));
>> + DEBUG ((EFI_D_INFO,
>> + "Ftw: Both working and spare blocks are invalid, init
>> workspace\n"));
>> //
>> // If both are invalid, then initialize work space.
>> //
>> SetMem (
>> FtwDevice->FtwWorkSpace,
>> FtwDevice->FtwWorkSpaceSize,
>> FTW_ERASED_BYTE
>> );
>> InitWorkSpaceHeader (FtwDevice->FtwWorkSpaceHeader);
>> //
>> diff --git
>> a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> index 31f1e0b..d46a37f 100644
>> --- a/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> +++ b/MdeModulePkg/Universal/FaultTolerantWriteDxe/UpdateWorkingBlock.c
>> @@ -91,21 +91,21 @@ IsValidWorkSpace (
>> )
>> {
>> if (WorkingHeader == NULL) {
>> return FALSE;
>> }
>>
>> if (CompareMem (WorkingHeader, &mWorkingBlockHeader, sizeof
>> (EFI_FAULT_TOLERANT_WORKING_BLOCK_HEADER)) == 0) {
>> return TRUE;
>> }
>>
>> - DEBUG ((EFI_D_ERROR, "Ftw: Work block header check error\n"));
>> + DEBUG ((EFI_D_INFO, "Ftw: Work block header check mismatch\n"));
>> return FALSE;
>> }
>>
>> /**
>> Initialize a work space when there is no work space.
>>
>> @param WorkingHeader Pointer of working block header
>>
>> @retval EFI_SUCCESS The function completed successfully
>> @retval EFI_ABORTED The function could not complete successfully.
>>
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
