Re: [Patch] MdeModulePkg/Mtftp4Dxe: Restore the TPL before the poll function.

["Wu, Jiaxin" <jiaxin.wu@intel.com>]

Yes, I agree with you. But we still can't raise time event to TPL_NOTIFY, because in the notify callback function, Udp protocol will be called to retransmit the packet while it's not allowed to call UDP in the TPL_NOTIFY.



> -----Original Message-----
> From: Fu, Siyuan
> Sent: Friday, March 2, 2018 9:24 AM
> To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> Subject: RE: [Patch] MdeModulePkg/Mtftp4Dxe: Restore the TPL before the
> poll function.
> 
> Jiaxin,
> 
> There will be problem if someone calls Mtftp.Start() at TPL Callback to
> send/receive a file with your patch. In such case the RestoreTpl() won't be
> able to restore the TPL to application level so it will still while loop in the Poll.
> 
> BestRegards
> Fu Siyuan
> 
> > -----Original Message-----
> > From: Wu, Jiaxin
> > Sent: Friday, March 2, 2018 9:18 AM
> > To: Fu, Siyuan <siyuan.fu@intel.com>; edk2-devel@lists.01.org
> > Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> > Subject: RE: [Patch] MdeModulePkg/Mtftp4Dxe: Restore the TPL before
> the
> > poll function.
> >
> > It's not actual hang but always running at while-poll function in the TPL
> > call back level , meanwhile, the while condition depends on another time
> > event that running on the same TPL. If so, the time event might have no
> > chance to be triggered. So, the code will never run out of while () {}:
> >
> >   while (Token->Status == EFI_NOT_READY) {
> >     This->Poll (This);
> >   }
> >
> >
> > Thanks,
> > Jiaxin
> >
> > > -----Original Message-----
> > > From: Fu, Siyuan
> > > Sent: Thursday, March 1, 2018 7:03 PM
> > > To: Wu, Jiaxin <jiaxin.wu@intel.com>; edk2-devel@lists.01.org
> > > Cc: Wang, Fan <fan.wang@intel.com>; Ye, Ting <ting.ye@intel.com>
> > > Subject: RE: [Patch] MdeModulePkg/Mtftp4Dxe: Restore the TPL before
> the
> > > poll function.
> > >
> > > Hi, Jiaxin
> > >
> > > Do you mean the code which calls MTFTP4->Poll() at TPL_CALLBACK will
> > > cause system hang? This should not happen because all network protocol
> > > should be able to run at TPL_CALLBACK.
> > >
> > > BestRegards
> > > Fu Siyuan
> > >
> > >
> > > > -----Original Message-----
> > > > From: Wu, Jiaxin
> > > > Sent: Thursday, March 1, 2018 5:38 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Wang, Fan <fan.wang@intel.com>; Fu, Siyuan
> <siyuan.fu@intel.com>;
> > > Ye,
> > > > Ting <ting.ye@intel.com>
> > > > Subject: [Patch] MdeModulePkg/Mtftp4Dxe: Restore the TPL before
> the
> > > poll
> > > > function.
> > > >
> > > > This patch is to fix the hang issue, which was enrolled by the commit
> > of
> > > > 39b0867d.
> > > > The TPL should be restored before calling poll function at
> > TPL_CALLBACK.
> > > >
> > > > Cc: Wang Fan <fan.wang@intel.com>
> > > > Cc: Fu Siyuan <siyuan.fu@intel.com>
> > > > Cc: Ye Ting <ting.ye@intel.com>
> > > > Contributed-under: TianoCore Contribution Agreement 1.0
> > > > Signed-off-by: Jiaxin Wu <jiaxin.wu@intel.com>
> > > > ---
> > > >  MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c | 9
> > > ++++++---
> > > >  1 file changed, 6 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git
> a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > index f5f9e6d8f7..64e0463dd9 100644
> > > > --- a/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > +++ b/MdeModulePkg/Universal/Network/Mtftp4Dxe/Mtftp4Impl.c
> > > > @@ -507,24 +507,27 @@ Mtftp4Start (
> > > >    if (EFI_ERROR (Status)) {
> > > >      Status = EFI_DEVICE_ERROR;
> > > >      goto ON_ERROR;
> > > >    }
> > > >
> > > > +  //
> > > > +  // Restore the TPL now, don't call poll function at TPL_CALLBACK.
> > > > +  //
> > > > +  gBS->RestoreTPL (OldTpl);
> > > > +
> > > >    if (Token->Event != NULL) {
> > > > -    gBS->RestoreTPL (OldTpl);
> > > >      return EFI_SUCCESS;
> > > >    }
> > > >
> > > >    //
> > > >    // Return immediately for asynchronous operation or poll the
> > > >    // instance for synchronous operation.
> > > >    //
> > > >    while (Token->Status == EFI_NOT_READY) {
> > > >      This->Poll (This);
> > > >    }
> > > > -
> > > > -  gBS->RestoreTPL (OldTpl);
> > > > +
> > > >    return Token->Status;
> > > >
> > > >  ON_ERROR:
> > > >    Mtftp4CleanOperation (Instance, Status);
> > > >    gBS->RestoreTPL (OldTpl);
> > > > --
> > > > 2.16.2.windows.1