Thank you for the confirmation, Heinrich. If no objection, I will commit it 36 hours later.
Reviewed-by: Eric Jin <eric....@intel.com> -----Original Message----- From: Heinrich Schuchardt <xypron.g...@gmx.de> Sent: Friday, September 27, 2019 2:00 PM To: Liu, XianhuiX <xianhuix....@intel.com>; devel@edk2.groups.io Cc: Supreeth Venkatesh <supreeth.venkat...@arm.com>; Jin, Eric <eric....@intel.com> Subject: Re: [edk2-devel] [edk2-test][Patch] uefi-sct/SctPkg: Add MediaPresentSupported check On 9/27/19 4:01 AM, Liu, XianhuiX wrote: > Hi Heinrich, > Any concern about it? Thanks. Your patch is correct. It is preceded by: SctPrint (L"\nPlease disconnect the machine from the LAN, press any key within 10 seconds\n"); If MediaPresentSupported is false you cannot detect if the user removed the LAN cable or not. Sorry for the noise. Best regards Heinrich > > Best Regards > Xianhui Liu > >> -----Original Message----- >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of >> xianhui liu >> Sent: Wednesday, September 25, 2019 10:18 AM >> To: devel@edk2.groups.io; xypron.g...@gmx.de >> Cc: Supreeth Venkatesh <supreeth.venkat...@arm.com>; Jin, Eric >> <eric....@intel.com> >> Subject: Re: [edk2-devel] [edk2-test][Patch] uefi-sct/SctPkg: Add >> MediaPresentSupported check >> >> Please refer to below comments. >> >>> -----Original Message----- >>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf >>> Of Heinrich Schuchardt >>> Sent: Wednesday, September 25, 2019 3:06 AM >>> To: Liu, XianhuiX <xianhuix....@intel.com>; devel@edk2.groups.io >>> Cc: Supreeth Venkatesh <supreeth.venkat...@arm.com>; Jin, Eric >>> <eric....@intel.com> >>> Subject: Re: [edk2-devel] [edk2-test][Patch] uefi-sct/SctPkg: Add >>> MediaPresentSupported check >>> >>> On 9/24/19 10:50 AM, xianhui liu wrote: >>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2061 >>>> check MediaPresent while MediaPresentSupported is TRUE sync change >>>> from EFI to IHV SimpleNetworkBBTestFunction >>> >>> Thanks for addressing this issue. >>> >>>> >>>> Cc: Heinrich Schuchardt <xypron.g...@gmx.de> >>> >>> %s/Cc:/Reported-by:/ >>> >>>> Cc: Supreeth Venkatesh <supreeth.venkat...@arm.com> >>>> Cc: Eric Jin <eric....@intel.com> >>>> >>>> Signed-off-by: xianhui liu <xianhuix....@intel.com> >>>> --- >>>> .../BlackBoxTest/SimpleNetworkBBTestFunction.c | 64 +++++++++++- >> -- >>> -------- >>>> 1 file changed, 33 insertions(+), 31 deletions(-) >>>> >>>> diff --git >>>> a/uefi- >>> sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTes >>>> t/SimpleNetworkBBTestFunction.c >>>> b/uefi- >>> sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTes >>>> t/SimpleNetworkBBTestFunction.c >>>> index b4c7b5ee..8559e894 100644 >>>> --- >>>> a/uefi- >>> sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTes >>>> t/SimpleNetworkBBTestFunction.c >>>> +++ b/uefi- >>> sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBo >>>> +++ xTest/SimpleNetworkBBTestFunction.c >>>> @@ -1888,38 +1888,40 @@ BBTestGetStatusFunctionTest ( >>>> Status = SnpInterface->GetStatus (SnpInterface, >>>> &InterruptStatus, >>> &TxBuf); >>>> Status1 = SnpInterface->GetStatus (SnpInterface, >>>> &InterruptStatus, &TxBuf); >>>> >>>> - if (SnpInterface->Mode->MediaPresent == FALSE) { >>>> - if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS) && >>> (InterruptStatus == 0)) { >>>> - AssertionType = EFI_TEST_ASSERTION_PASSED; >>>> - } else { >>>> - AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> - } >>>> - } else { >>>> - if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS)) { >>>> - AssertionType = EFI_TEST_ASSERTION_PASSED; >>>> - if (InterruptStatus & >>>> - ~( EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT | >>>> - EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT | >>>> - EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT | >>>> - EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT)) { >>>> - AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> - } >>>> - } else { >>>> - AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> - } >>>> + if (SnpInterface->Mode-> MediaPresentSupported == TRUE) { >>>> + if (SnpInterface->Mode->MediaPresent == FALSE) { >>> >>> >>> If MediaPresentSupported == FALSE shouldn't we assume that Media is >>> present. >>> >>> So isn't a single 'if' enough: >>> >>> if (SnpInterface->Mode-> MediaPresentSupported == TRUE && >>> SnpInterface->Mode->MediaPresent == FALSE) { >>> >>> Best regards >>> >>> Heinrich Schuchardt >> >> Hi Heinrich, >> We will skip the checkpoint if MediaPresentSupported == FALSE. Thanks. >> >> Hi Eric, >> Please help correct me if any mistake here. Thanks. >> >> Best Regards >> Xianhui Liu >> >> >>> >>>> + if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS) && >>> (InterruptStatus == 0)) { >>>> + AssertionType = EFI_TEST_ASSERTION_PASSED; >>>> + } else { >>>> + AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> + } >>>> + } else { >>>> + if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS)) { >>>> + AssertionType = EFI_TEST_ASSERTION_PASSED; >>>> + if (InterruptStatus & >>>> + ~( EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT | >>>> + EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT | >>>> + EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT | >>>> + EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT)) { >>>> + AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> + } >>>> + } else { >>>> + AssertionType = EFI_TEST_ASSERTION_FAILED; >>>> + } >>>> + } >>>> + StandardLib->RecordAssertion ( >>>> + StandardLib, >>>> + AssertionType, >>>> + gSimpleNetworkBBTestFunctionAssertionGuid022, >>>> + L"EFI_SIMPLE_NETWORK_PROTOCOL.GetStatus - >>>> + Invoke >>> GetStatus() and verify interface correctness within test case", >>>> + L"%a:%d:Status - %r, Status1 - %r, InterruptStatus - >>>> %d", >>>> + __FILE__, >>>> + (UINTN)__LINE__, >>>> + Status, >>>> + Status1, >>>> + InterruptStatus >>>> + ); >>>> } >>>> - StandardLib->RecordAssertion ( >>>> - StandardLib, >>>> - AssertionType, >>>> - gSimpleNetworkBBTestFunctionAssertionGuid022, >>>> - L"EFI_SIMPLE_NETWORK_PROTOCOL.GetStatus - Invoke >>> GetStatus() and verify interface correctness within test case", >>>> - L"%a:%d:Status - %r, Status1 - %r, InterruptStatus - %d", >>>> - __FILE__, >>>> - (UINTN)__LINE__, >>>> - Status, >>>> - Status1, >>>> - InterruptStatus >>>> - ); >>>> >>>> // >>>> // Restore SNP State >>>> >>> >>> >>> >> >> >> > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48189): https://edk2.groups.io/g/devel/message/48189 Mute This Topic: https://groups.io/mt/34258689/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
