Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Fix a typo below: blocking I/O request will wait for all the (synchronous -> asynchronous) I/O requests to complete first before sending down the synchronous request Best Regards, Hao Wu > -Original Message- > From: Wu, Hao A > Sent: Thursday, June 23, 2022 10:47 AM > To: devel@edk2.groups.io; jbra...@nvidia.com > Cc: Ni, Ray ; Kinney, Michael D > ; Gao, Liming ; > Zeng, Star > Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to > NOTIFY > > Hello, > > Several concerns: > 1. Raising the TPL to NOTIFY level for blocking/synchronous BlockIO service > will break the using case when the co-existence of synchronous and non- > blocking/asynchronous IO tasks > As far as I know, in DiskIoDxe, blocking I/O request will wait for all the > synchronous I/O requests to complete first before sending down the > synchronous request. > Please consider the scenario where a previous asynchronous IO task, it will > take a long time to complete due to big data transfer size. > When a subsequent synchronous IO task is submitted before the > asynchronous task completes, it will wait for the previous asynchronous task > to complete. > However, if the TPL level for this synchronous task is NOTIFY, it will get > stuck > since the completion callback for the asynchronous task is also at NOTIFY TPL > level, which will never be called. > More details can be referred to commit: > SHA-1: a717086c5f973821b9b49646a4ec725f6b898bdb > * MdeModulePkg ScsiDiskDxe: Raise the Tpl of async IO callback to > TPL_NOTIFY > > 2. My personal take is that BlockIO is not a proper base service for variable > service > To my knowledge, the variable service will still exist during runtime, but the > BlockIO services (as far as I know) do not. > My understanding is that the main purpose of the BlockIO services are to > provide BIOS boot codes with the ability to locate partitions and file systems > on media storage devices and search boot options on them. > I would recommend to implement a dedicated platform driver to produce > the FVB services for variable driver consumption. > > If you think using the BlockIO service as the base for variable service is the > right direction, I have added people in the CC list that might help. > Could you please reach out to them for feedbacks on the TPL restriction on > BlockIO services and Variable service? > > Best Regards, > Hao Wu > > > -Original Message- > > From: devel@edk2.groups.io On Behalf Of Jeff > > Brasen via groups.io > > Sent: Friday, June 17, 2022 11:37 PM > > To: Wu, Hao A ; devel@edk2.groups.io > > Cc: Ni, Ray > > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to > > NOTIFY > > > > Resending this as I replying via edk2.groups.io doesn't seem to copy > > maintainers. > > > > Resuming this patch to see if there are any additional thoughts on this. > > > > In response to the query about DXE/BDS services we have some internal > > connection logic that runs in DXE to connect the devices that are needed > for > > arch services that have to be connected prior the end of dxe. > > > > Thanks, > > Jeff > > > > > -Original Message- > > > From: Jeff Brasen > > > Sent: Tuesday, December 14, 2021 9:48 PM > > > To: Wu, Hao A ; devel@edk2.groups.io > > > Cc: Ni, Ray > > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > > > > > > > > > -Original Message- > > > > From: Wu, Hao A > > > > Sent: Tuesday, December 14, 2021 8:00 PM > > > > To: Jeff Brasen ; devel@edk2.groups.io > > > > Cc: Ni, Ray > > > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > > -Original Message- > > > > > From: Jeff Brasen > > > > > Sent: Wednesday, December 15, 2021 1:59 AM > > > > > To: devel@edk2.groups.io > > > > > Cc: Wu, Hao A ; Ni, Ray ; > > > > > Jeff Brasen > > > > > 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 T
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Hello, Several concerns: 1. Raising the TPL to NOTIFY level for blocking/synchronous BlockIO service will break the using case when the co-existence of synchronous and non-blocking/asynchronous IO tasks As far as I know, in DiskIoDxe, blocking I/O request will wait for all the synchronous I/O requests to complete first before sending down the synchronous request. Please consider the scenario where a previous asynchronous IO task, it will take a long time to complete due to big data transfer size. When a subsequent synchronous IO task is submitted before the asynchronous task completes, it will wait for the previous asynchronous task to complete. However, if the TPL level for this synchronous task is NOTIFY, it will get stuck since the completion callback for the asynchronous task is also at NOTIFY TPL level, which will never be called. More details can be referred to commit: SHA-1: a717086c5f973821b9b49646a4ec725f6b898bdb * MdeModulePkg ScsiDiskDxe: Raise the Tpl of async IO callback to TPL_NOTIFY 2. My personal take is that BlockIO is not a proper base service for variable service To my knowledge, the variable service will still exist during runtime, but the BlockIO services (as far as I know) do not. My understanding is that the main purpose of the BlockIO services are to provide BIOS boot codes with the ability to locate partitions and file systems on media storage devices and search boot options on them. I would recommend to implement a dedicated platform driver to produce the FVB services for variable driver consumption. If you think using the BlockIO service as the base for variable service is the right direction, I have added people in the CC list that might help. Could you please reach out to them for feedbacks on the TPL restriction on BlockIO services and Variable service? Best Regards, Hao Wu > -Original Message- > From: devel@edk2.groups.io On Behalf Of Jeff > Brasen via groups.io > Sent: Friday, June 17, 2022 11:37 PM > To: Wu, Hao A ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to > NOTIFY > > Resending this as I replying via edk2.groups.io doesn't seem to copy > maintainers. > > Resuming this patch to see if there are any additional thoughts on this. > > In response to the query about DXE/BDS services we have some internal > connection logic that runs in DXE to connect the devices that are needed for > arch services that have to be connected prior the end of dxe. > > Thanks, > Jeff > > > -Original Message- > > From: Jeff Brasen > > Sent: Tuesday, December 14, 2021 9:48 PM > > To: Wu, Hao A ; devel@edk2.groups.io > > Cc: Ni, Ray > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > > > > > -Original Message- > > > From: Wu, Hao A > > > Sent: Tuesday, December 14, 2021 8:00 PM > > > To: Jeff Brasen ; devel@edk2.groups.io > > > Cc: Ni, Ray > > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > > External email: Use caution opening links or attachments > > > > > > > > > > -Original Message- > > > > From: Jeff Brasen > > > > Sent: Wednesday, December 15, 2021 1:59 AM > > > > To: devel@edk2.groups.io > > > > Cc: Wu, Hao A ; Ni, Ray ; > > > > Jeff Brasen > > > > 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 oth
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Resending this as I replying via edk2.groups.io doesn't seem to copy maintainers. Resuming this patch to see if there are any additional thoughts on this. In response to the query about DXE/BDS services we have some internal connection logic that runs in DXE to connect the devices that are needed for arch services that have to be connected prior the end of dxe. Thanks, Jeff > -Original Message- > From: Jeff Brasen > Sent: Tuesday, December 14, 2021 9:48 PM > To: Wu, Hao A ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > -Original Message- > > From: Wu, Hao A > > Sent: Tuesday, December 14, 2021 8:00 PM > > To: Jeff Brasen ; devel@edk2.groups.io > > Cc: Ni, Ray > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > External email: Use caution opening links or attachments > > > > > > > -Original Message- > > > From: Jeff Brasen > > > Sent: Wednesday, December 15, 2021 1:59 AM > > > To: devel@edk2.groups.io > > > Cc: Wu, Hao A ; Ni, Ray ; Jeff > > > Brasen > > > 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 > > > > > > I had sent out a query on this before and didn't see any response. The core > of the issue I am trying to solve it support variable services on a UFS > device. > When the UFS blockIO is invoked from variable services it is not allowed > (which does align from the UEFI spec perspective but does not allow me to > implement variables services on UFS) > > The other way that worked was lowering the lock TPL level in the PCD driver > and Variable down to callback. The PCD one seems like it should be done as > variable services is supposed to only be called from <= TPL_CALLBACK. > However, I was worried about that having a larger system impact on that > change. > > > > > > > Signed-off-by: Jeff Brasen > > > --- > > > 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
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Resuming this PATCH to see if there are any additional thoughts on this. In response to the query about DXE/BDS services we have some internal connection logic that runs in DXE to connect the devices that are needed for arch services that have to be connected prior the end of dxe. Thanks, Jeff -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#87420): https://edk2.groups.io/g/devel/message/87420 Mute This Topic: https://groups.io/mt/87726872/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
Jeff's statement about the TPLs in variable and PCD drivers seems correct, not sure the impact to change them. No good idea from me about the TPL change in ScsiDisk. Side question: Variable service is an arch service which should be ready at DXE phase, ScsiDisk is a driver model driver which will be only ready/connected at BDS phase, right? Thanks, Star -Original Message- From: Wu, Hao A Sent: 2021年12月15日 13:43 To: Jeff Brasen ; devel@edk2.groups.io; Kinney, Michael D ; Gao, Liming ; Zeng, Star Cc: Ni, Ray Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY (Add more people) Hello Mike, Liming and Star, Do you have suggestions for the below question raised from Jeff Brasen: " The core of the issue I am trying to solve it support variable services on a UFS device. When the UFS blockIO is invoked from variable services it is not allowed (which does align from the UEFI spec perspective but does not allow me to implement variables services on UFS) The other way that worked was lowering the lock TPL level in the PCD driver and Variable down to callback. The PCD one seems like it should be done as variable services is supposed to only be called from <= TPL_CALLBACK. However, I was worried about that having a larger system impact on that change." Thanks in advance. Best Regards, Hao Wu > -Original Message- > From: Jeff Brasen > Sent: Wednesday, December 15, 2021 12:48 PM > To: Wu, Hao A ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > -Original Message- > > From: Wu, Hao A > > Sent: Tuesday, December 14, 2021 8:00 PM > > To: Jeff Brasen ; devel@edk2.groups.io > > Cc: Ni, Ray > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > External email: Use caution opening links or attachments > > > > > > > -Original Message- > > > From: Jeff Brasen > > > Sent: Wednesday, December 15, 2021 1:59 AM > > > To: devel@edk2.groups.io > > > Cc: Wu, Hao A ; Ni, Ray ; > > > Jeff Brasen > > > 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 > > > > > > I had sent out a query on this before and didn't see any response. The > core of the issue I am trying to solve it support variable services on a UFS > device. > When the UFS blockIO is invoked from variable services it is not > allowed (which does align from the UEFI spec perspective but does not > allow me to implement variables services on UFS) > > The other way that worked was lowering the lock TPL level in the PCD > driver and Variable down to callback. The PCD one seems like it should > be done as variable services is supposed to only be called from <= > TPL_CALLBACK. > However, I was worried about that having a larger system impact on > that change. > > > > > > > Signed-off-by: Jeff Brasen > > > --- > > > 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
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
(Add more people) Hello Mike, Liming and Star, Do you have suggestions for the below question raised from Jeff Brasen: " The core of the issue I am trying to solve it support variable services on a UFS device. When the UFS blockIO is invoked from variable services it is not allowed (which does align from the UEFI spec perspective but does not allow me to implement variables services on UFS) The other way that worked was lowering the lock TPL level in the PCD driver and Variable down to callback. The PCD one seems like it should be done as variable services is supposed to only be called from <= TPL_CALLBACK. However, I was worried about that having a larger system impact on that change." Thanks in advance. Best Regards, Hao Wu > -Original Message- > From: Jeff Brasen > Sent: Wednesday, December 15, 2021 12:48 PM > To: Wu, Hao A ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > > -Original Message- > > From: Wu, Hao A > > Sent: Tuesday, December 14, 2021 8:00 PM > > To: Jeff Brasen ; devel@edk2.groups.io > > Cc: Ni, Ray > > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > > > External email: Use caution opening links or attachments > > > > > > > -Original Message- > > > From: Jeff Brasen > > > Sent: Wednesday, December 15, 2021 1:59 AM > > > To: devel@edk2.groups.io > > > Cc: Wu, Hao A ; Ni, Ray ; Jeff > > > Brasen > > > 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 > > > > > > I had sent out a query on this before and didn't see any response. The core > of the issue I am trying to solve it support variable services on a UFS > device. > When the UFS blockIO is invoked from variable services it is not allowed > (which does align from the UEFI spec perspective but does not allow me to > implement variables services on UFS) > > The other way that worked was lowering the lock TPL level in the PCD driver > and Variable down to callback. The PCD one seems like it should be done as > variable services is supposed to only be called from <= TPL_CALLBACK. > However, I was worried about that having a larger system impact on that > change. > > > > > > > Signed-off-by: Jeff Brasen > > > --- > > > 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;
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
> -Original Message- > From: Wu, Hao A > Sent: Tuesday, December 14, 2021 8:00 PM > To: Jeff Brasen ; devel@edk2.groups.io > Cc: Ni, Ray > Subject: RE: [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY > > External email: Use caution opening links or attachments > > > > -Original Message- > > From: Jeff Brasen > > Sent: Wednesday, December 15, 2021 1:59 AM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A ; Ni, Ray ; Jeff > > Brasen > > 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 > > I had sent out a query on this before and didn't see any response. The core of the issue I am trying to solve it support variable services on a UFS device. When the UFS blockIO is invoked from variable services it is not allowed (which does align from the UEFI spec perspective but does not allow me to implement variables services on UFS) The other way that worked was lowering the lock TPL level in the PCD driver and Variable down to callback. The PCD one seems like it should be done as variable services is supposed to only be called from <= TPL_CALLBACK. However, I was worried about that having a larger system impact on that change. > > > > Signed-off-by: Jeff Brasen > > --- > > 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 =
Re: [edk2-devel] [PATCH] MdeModulePkg/ScsiDisk: Change TPL to NOTIFY
> -Original Message-
> From: Jeff Brasen
> Sent: Wednesday, December 15, 2021 1:59 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A ; Ni, Ray ; Jeff
> Brasen
> 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
> ---
> 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;
>
> @@
[edk2-devel] [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
Signed-off-by: Jeff Brasen
---
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,
--
2.17.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#84826): https://edk2.groups.io/g/devel/message/84826
Mute This Topic: https://groups.io/mt/87726872/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
