public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Wu, Hao A" <hao.a.wu@intel.com>
To: Jeff Brasen <jbrasen@nvidia.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Ni, Ray" <ray.ni@intel.com>
Subject: Re: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Date: Wed, 15 Dec 2021 02:59:30 +0000	[thread overview]
Message-ID: <DM6PR11MB4025212CDB24D44A5A6DBD07CA769@DM6PR11MB4025.namprd11.prod.outlook.com> (raw)
In-Reply-To: <cabfee4150c1adc404c62e442301ca4b1b43b66f.1639504483.git.jbrasen@nvidia.com>

> -----Original Message-----
> From: Jeff Brasen <jbrasen@nvidia.com>
> Sent: Wednesday, December 15, 2021 1:59 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Jeff
> Brasen <jbrasen@nvidia.com>
> Subject: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
> 
> Increase TPL to TPL_NOTIFY to allow for use if caller is > TPL_CALLBACK.
> This allows services like variable services that run at TPL_NOTIFY to be hosted
> on ScsiDisks (i.e. UFS)
> 
> Aligns with the eMMC driver that also uses a higher TPL.
> This change was made in 3b1d8241d0dac25c5e678c364fa2754ac1731060


Sorry, my take is that this change is not equivalent to the one made in the SD/MMC stack.

For the SD/MMC change you mentioned (commit 3b1d8241d0dac25c5e678c364fa2754ac1731060), the TPL is raised to TPL_NOTIFY only when:
  a) Operation to the linked lists that manage the asynchronous IO tasks
  b) Callback functions that process the asynchronous IO tasks
The TPL remains TPL_CALLBACK during the BlockIO services and the majority of the BlockIO2 services (operations to asynchronous tasks linked list are the exceptions).

But the proposed change in ScsiDisk modifies the TPL level of the entire BlockIO/BlockIO2 (and other protocols) services to TPL_NOTIFY.
For me, this is not aligned with the "TPL Restrictions" documented in the UEFI specification.

Best Regards,
Hao Wu


> 
> Signed-off-by: Jeff Brasen <jbrasen@nvidia.com>
> ---
>  MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c | 22 ++++++++++----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> index 98e84b4ea8..b6e5848e77 100644
> --- a/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> +++ b/MdeModulePkg/Bus/Scsi/ScsiDiskDxe/ScsiDisk.c
> @@ -514,7 +514,7 @@ ScsiDiskReset (
>    SCSI_DISK_DEV  *ScsiDiskDevice;
>    EFI_STATUS     Status;
> 
> -  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> 
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO (This);
> 
> @@ -581,7 +581,7 @@ ScsiDiskReadBlocks (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO (This);
>    Media          = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -733,7 +733,7 @@ ScsiDiskWriteBlocks (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO (This);
>    Media          = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -898,7 +898,7 @@ ScsiDiskResetEx (
>    SCSI_DISK_DEV  *ScsiDiskDevice;
>    EFI_STATUS     Status;
> 
> -  OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl = gBS->RaiseTPL (TPL_NOTIFY);
> 
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
> 
> @@ -975,7 +975,7 @@ ScsiDiskReadBlocksEx (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
>    Media          = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -1154,7 +1154,7 @@ ScsiDiskWriteBlocksEx (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
>    Media          = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -1323,7 +1323,7 @@ ScsiDiskFlushBlocksEx (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_BLKIO2 (This);
>    Media          = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -1717,7 +1717,7 @@ ScsiDiskEraseBlocks (
>    EFI_TPL             OldTpl;
> 
>    MediaChange    = FALSE;
> -  OldTpl         = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl         = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice = SCSI_DISK_DEV_FROM_ERASEBLK (This);
> 
>    if (!IS_DEVICE_FIXED (ScsiDiskDevice)) { @@ -1907,7 +1907,7 @@
> ScsiDiskReceiveData (
>    AlignedBuffer          = NULL;
>    MediaChange            = FALSE;
>    AlignedBufferAllocated = FALSE;
> -  OldTpl                 = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl                 = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice         = SCSI_DISK_DEV_FROM_STORSEC (This);
>    Media                  = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -2122,7 +2122,7 @@ ScsiDiskSendData (
>    AlignedBuffer          = NULL;
>    MediaChange            = FALSE;
>    AlignedBufferAllocated = FALSE;
> -  OldTpl                 = gBS->RaiseTPL (TPL_CALLBACK);
> +  OldTpl                 = gBS->RaiseTPL (TPL_NOTIFY);
>    ScsiDiskDevice         = SCSI_DISK_DEV_FROM_STORSEC (This);
>    Media                  = ScsiDiskDevice->BlkIo.Media;
> 
> @@ -2294,7 +2294,7 @@ ScsiDiskDetectMedia (
> 
>    Status = gBS->CreateEvent (
>                    EVT_TIMER,
> -                  TPL_CALLBACK,
> +                  TPL_NOTIFY,
>                    NULL,
>                    NULL,
>                    &TimeoutEvt
> --
> 2.17.1


  reply	other threads:[~2021-12-15  2:59 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-14 17:59 [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY Jeff Brasen
2021-12-15  2:59 ` Wu, Hao A [this message]
2021-12-15  4:48   ` Jeff Brasen
2021-12-15  5:43     ` Wu, Hao A
2021-12-17  4:07       ` Zeng, Star
2022-03-10 16:38         ` [edk2-devel] " Jeff Brasen
2022-06-17 15:37     ` Jeff Brasen
2022-06-23  2:46       ` [edk2-devel] " Wu, Hao A
2022-06-23  2:53         ` Wu, Hao A

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR11MB4025212CDB24D44A5A6DBD07CA769@DM6PR11MB4025.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox