[edk2-devel] [edk2-test][Patch] uefi-sct/SctPkg: Add MediaPresentSupported check

[xianhui liu]

Hi Heinrich,
Any concern about it? Thanks.

Best Regards
Xianhui Liu

> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> xianhui liu
> Sent: Wednesday, September 25, 2019 10:18 AM
> To: devel at edk2.groups.io; xypron.glpk at gmx.de
> Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>; Jin, Eric
> <eric.jin at intel.com>
> Subject: Re: [edk2-devel] [edk2-test][Patch] uefi-sct/SctPkg: Add
> MediaPresentSupported check
> 
> Please refer to below comments.
> 
> > -----Original Message-----
> > From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> > Heinrich Schuchardt
> > Sent: Wednesday, September 25, 2019 3:06 AM
> > To: Liu, XianhuiX <xianhuix.liu at intel.com>; devel at edk2.groups.io
> > Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>; Jin, Eric
> > <eric.jin at 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.glpk at gmx.de>
> >
> > %s/Cc:/Reported-by:/
> >
> > > Cc: Supreeth Venkatesh <supreeth.venkatesh at arm.com>
> > > Cc: Eric Jin <eric.jin at intel.com>
> > >
> > > Signed-off-by: xianhui liu <xianhuix.liu at 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 (#48169): https://edk2.groups.io/g/devel/message/48169
Mute This Topic: https://groups.io/mt/34258689/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-