Hi Feng,
I found that some "RemoveEntryList" calls that modifies SD/EMMC device's
queue are not guarded by raising the TPL.
I think most of them are related with EraseBlock feature functions in file
EmmcBlockIo.c & SdBlockIo.c.
Could you help to double confirm and add the missing guards?
Best Regards,
Hao Wu
> -----Original Message-----
> From: Tian, Feng
> Sent: Thursday, June 23, 2016 3:53 PM
> To: Wu, Hao A
> Cc: edk2-devel@lists.01.org
> Subject: [patch] MdeModulePkg/SdMmc: update TPL to notify to fix UEFI SCT
> hang
>
> We have to upgrade the TPL level used by SdMmc stack because the
> following flow:
>
> DiskIo2ReadWriteDisk() in logical partition -> PartitionReadBlocksEx()
> in logical partition at TPL callback level -> ProbeMediaStatusEx()
> with sync request -> DiskIo2ReadWriteDisk() in physical partition ->
> waiting for async task completion.
>
> if the low layer driver doesn't run at TPL_NOTIFY level, it will have
> no time to trigger async task and cause system hang.
>
> Cc: Hao Wu <hao.a...@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Feng Tian <feng.t...@intel.com>
> ---
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c | 8 ++--
> MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c | 2 +-
> MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c | 48 +++++++++++-----
> ------
> MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c | 22 +++++-----
> MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c | 2 +-
> 5 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> index ed6b557..8716fcd 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
> @@ -665,7 +665,7 @@ SdMmcPciHcDriverBindingStart (
> //
> Status = gBS->CreateEvent (
> EVT_TIMER | EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> ProcessAsyncTaskList,
> Private,
> &Private->TimerEvent
> @@ -684,7 +684,7 @@ SdMmcPciHcDriverBindingStart (
> //
> Status = gBS->CreateEvent (
> EVT_TIMER | EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> SdMmcPciHcEnumerateDevice,
> Private,
> &Private->ConnectEvent
> @@ -961,7 +961,7 @@ SdMmcPassThruPassThru (
> // Wait async I/O list is empty before execute sync I/O operation.
> //
> while (TRUE) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> if (IsListEmpty (&Private->Queue)) {
> gBS->RestoreTPL (OldTpl);
> break;
> @@ -1273,7 +1273,7 @@ SdMmcPassThruResetDevice (
> //
> // Free all async I/O requests in the queue
> //
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
>
> for (Link = GetFirstNode (&Private->Queue);
> !IsNull (&Private->Queue, Link);
> diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> index 8978182..b4ff2af 100644
> --- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> +++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
> @@ -1315,7 +1315,7 @@ SdMmcCreateTrb (
> }
>
> if (Event != NULL) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Private->Queue, &Trb->TrbList);
> gBS->RestoreTPL (OldTpl);
> }
> diff --git a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> index 5fe710d..a4c6053 100644
> --- a/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/EmmcDxe/EmmcBlockIo.c
> @@ -339,7 +339,7 @@ EmmcSetExtCsd (
> }
>
> SetExtCsdReq->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &SetExtCsdReq->Link);
> gBS->RestoreTPL (OldTpl);
> SetExtCsdReq->Packet.SdMmcCmdBlk = &SetExtCsdReq->SdMmcCmdBlk;
> @@ -361,7 +361,7 @@ EmmcSetExtCsd (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> SetExtCsdReq,
> &SetExtCsdReq->Event
> @@ -382,7 +382,7 @@ Error:
> // The request and event will be freed in asynchronous callback for
> success
> case.
> //
> if (EFI_ERROR (Status) && (SetExtCsdReq != NULL)) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&SetExtCsdReq->Link);
> gBS->RestoreTPL (OldTpl);
> if (SetExtCsdReq->Event != NULL) {
> @@ -395,7 +395,7 @@ Error:
> // For synchronous operation, free request whatever the execution result
> is.
> //
> if (SetExtCsdReq != NULL) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&SetExtCsdReq->Link);
> gBS->RestoreTPL (OldTpl);
> FreePool (SetExtCsdReq);
> @@ -445,7 +445,7 @@ EmmcSetBlkCount (
> }
>
> SetBlkCntReq->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &SetBlkCntReq->Link);
> gBS->RestoreTPL (OldTpl);
> SetBlkCntReq->Packet.SdMmcCmdBlk = &SetBlkCntReq->SdMmcCmdBlk;
> @@ -463,7 +463,7 @@ EmmcSetBlkCount (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> SetBlkCntReq,
> &SetBlkCntReq->Event
> @@ -484,7 +484,7 @@ Error:
> // The request and event will be freed in asynchronous callback for
> success
> case.
> //
> if (EFI_ERROR (Status) && (SetBlkCntReq != NULL)) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&SetBlkCntReq->Link);
> gBS->RestoreTPL (OldTpl);
> if (SetBlkCntReq->Event != NULL) {
> @@ -497,7 +497,7 @@ Error:
> // For synchronous operation, free request whatever the execution result
> is.
> //
> if (SetBlkCntReq != NULL) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&SetBlkCntReq->Link);
> gBS->RestoreTPL (OldTpl);
> FreePool (SetBlkCntReq);
> @@ -562,7 +562,7 @@ EmmcProtocolInOut (
> }
>
> ProtocolReq->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &ProtocolReq->Link);
> gBS->RestoreTPL (OldTpl);
> ProtocolReq->Packet.SdMmcCmdBlk = &ProtocolReq->SdMmcCmdBlk;
> @@ -596,7 +596,7 @@ EmmcProtocolInOut (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> ProtocolReq,
> &ProtocolReq->Event
> @@ -617,7 +617,7 @@ Error:
> // The request and event will be freed in asynchronous callback for
> success
> case.
> //
> if (EFI_ERROR (Status) && (ProtocolReq != NULL)) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&ProtocolReq->Link);
> gBS->RestoreTPL (OldTpl);
> if (ProtocolReq->Event != NULL) {
> @@ -630,7 +630,7 @@ Error:
> // For synchronous operation, free request whatever the execution result
> is.
> //
> if (ProtocolReq != NULL) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&ProtocolReq->Link);
> gBS->RestoreTPL (OldTpl);
> FreePool (ProtocolReq);
> @@ -688,7 +688,7 @@ EmmcRwMultiBlocks (
> }
>
> RwMultiBlkReq->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &RwMultiBlkReq->Link);
> gBS->RestoreTPL (OldTpl);
> RwMultiBlkReq->Packet.SdMmcCmdBlk = &RwMultiBlkReq-
> >SdMmcCmdBlk;
> @@ -730,7 +730,7 @@ EmmcRwMultiBlocks (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> RwMultiBlkReq,
> &RwMultiBlkReq->Event
> @@ -751,7 +751,7 @@ Error:
> // The request and event will be freed in asynchronous callback for
> success
> case.
> //
> if (EFI_ERROR (Status) && (RwMultiBlkReq != NULL)) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&RwMultiBlkReq->Link);
> gBS->RestoreTPL (OldTpl);
> if (RwMultiBlkReq->Event != NULL) {
> @@ -764,7 +764,7 @@ Error:
> // For synchronous operation, free request whatever the execution result
> is.
> //
> if (RwMultiBlkReq != NULL) {
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> RemoveEntryList (&RwMultiBlkReq->Link);
> gBS->RestoreTPL (OldTpl);
> FreePool (RwMultiBlkReq);
> @@ -901,7 +901,7 @@ EmmcReadWrite (
> if (EFI_ERROR (Status)) {
> return Status;
> }
> - DEBUG ((EFI_D_INFO, "Emmc%a(): Lba 0x%x BlkNo 0x%x Event %p
> with %r\n", IsRead ? "Read" : "Write", Lba, BlockNum, Token->Event, Status));
> + DEBUG ((EFI_D_INFO, "Emmc%a(): Part %d Lba 0x%x BlkNo 0x%x Event %p
> with %r\n", IsRead ? "Read " : "Write", Partition->PartitionType, Lba,
> BlockNum,
> Token ? Token->Event : NULL, Status));
>
> Lba += BlockNum;
> Buffer = (UINT8*)Buffer + BufferSize;
> @@ -1069,7 +1069,7 @@ EmmcResetEx (
>
> Partition = EMMC_PARTITION_DATA_FROM_BLKIO2 (This);
>
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> for (Link = GetFirstNode (&Partition->Queue);
> !IsNull (&Partition->Queue, Link);
> Link = NextLink) {
> @@ -1629,7 +1629,7 @@ EmmcEraseBlockStart (
> }
>
> EraseBlockStart->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &EraseBlockStart->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlockStart->Packet.SdMmcCmdBlk = &EraseBlockStart-
> >SdMmcCmdBlk;
> @@ -1652,7 +1652,7 @@ EmmcEraseBlockStart (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlockStart,
> &EraseBlockStart->Event
> @@ -1732,7 +1732,7 @@ EmmcEraseBlockEnd (
> }
>
> EraseBlockEnd->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &EraseBlockEnd->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlockEnd->Packet.SdMmcCmdBlk = &EraseBlockEnd->SdMmcCmdBlk;
> @@ -1755,7 +1755,7 @@ EmmcEraseBlockEnd (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlockEnd,
> &EraseBlockEnd->Event
> @@ -1833,7 +1833,7 @@ EmmcEraseBlock (
> }
>
> EraseBlock->Signature = EMMC_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Partition->Queue, &EraseBlock->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlock->Packet.SdMmcCmdBlk = &EraseBlock->SdMmcCmdBlk;
> @@ -1850,7 +1850,7 @@ EmmcEraseBlock (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlock,
> &EraseBlock->Event
> diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> index d8b9459..ee16a2b 100644
> --- a/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> +++ b/MdeModulePkg/Bus/Sd/SdDxe/SdBlockIo.c
> @@ -340,7 +340,7 @@ SdRwSingleBlock (
> }
>
> RwSingleBlkReq->Signature = SD_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Device->Queue, &RwSingleBlkReq->Link);
> gBS->RestoreTPL (OldTpl);
> RwSingleBlkReq->Packet.SdMmcCmdBlk = &RwSingleBlkReq-
> >SdMmcCmdBlk;
> @@ -382,7 +382,7 @@ SdRwSingleBlock (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> RwSingleBlkReq,
> &RwSingleBlkReq->Event
> @@ -468,7 +468,7 @@ SdRwMultiBlocks (
> }
>
> RwMultiBlkReq->Signature = SD_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Device->Queue, &RwMultiBlkReq->Link);
> gBS->RestoreTPL (OldTpl);
> RwMultiBlkReq->Packet.SdMmcCmdBlk = &RwMultiBlkReq-
> >SdMmcCmdBlk;
> @@ -510,7 +510,7 @@ SdRwMultiBlocks (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> RwMultiBlkReq,
> &RwMultiBlkReq->Event
> @@ -830,7 +830,7 @@ SdResetEx (
>
> Device = SD_DEVICE_DATA_FROM_BLKIO2 (This);
>
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> for (Link = GetFirstNode (&Device->Queue);
> !IsNull (&Device->Queue, Link);
> Link = NextLink) {
> @@ -1007,7 +1007,7 @@ SdEraseBlockStart (
> }
>
> EraseBlockStart->Signature = SD_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Device->Queue, &EraseBlockStart->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlockStart->Packet.SdMmcCmdBlk = &EraseBlockStart-
> >SdMmcCmdBlk;
> @@ -1030,7 +1030,7 @@ SdEraseBlockStart (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlockStart,
> &EraseBlockStart->Event
> @@ -1107,7 +1107,7 @@ SdEraseBlockEnd (
> }
>
> EraseBlockEnd->Signature = SD_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Device->Queue, &EraseBlockEnd->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlockEnd->Packet.SdMmcCmdBlk = &EraseBlockEnd->SdMmcCmdBlk;
> @@ -1130,7 +1130,7 @@ SdEraseBlockEnd (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlockEnd,
> &EraseBlockEnd->Event
> @@ -1205,7 +1205,7 @@ SdEraseBlock (
> }
>
> EraseBlock->Signature = SD_REQUEST_SIGNATURE;
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> InsertTailList (&Device->Queue, &EraseBlock->Link);
> gBS->RestoreTPL (OldTpl);
> EraseBlock->Packet.SdMmcCmdBlk = &EraseBlock->SdMmcCmdBlk;
> @@ -1222,7 +1222,7 @@ SdEraseBlock (
> if ((Token != NULL) && (Token->Event != NULL)) {
> Status = gBS->CreateEvent (
> EVT_NOTIFY_SIGNAL,
> - TPL_CALLBACK,
> + TPL_NOTIFY,
> AsyncIoCallback,
> EraseBlock,
> &EraseBlock->Event
> diff --git a/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c
> b/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c
> index 7c70c60..0cf9067 100644
> --- a/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c
> +++ b/MdeModulePkg/Bus/Sd/SdDxe/SdDxe.c
> @@ -800,7 +800,7 @@ SdDxeDriverBindingStop (
> //
> // Free all on-going async tasks.
> //
> - OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> + OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> for (Link = GetFirstNode (&Device->Queue);
> !IsNull (&Device->Queue, Link);
> Link = NextLink) {
> --
> 2.7.1.windows.2
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
