MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++--- MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-)
NVMe CQs are hardware queues mapped to EFI memory. In the NVMExpress Passthru implementations in PEI and DXE, it has been observed that NVMe CQs are not marked volatile, meaning the compiler has significant leeway to optimize accesses to these structures. This led to an issue where the passthru driver waited for a timeout period for an NVMe CQ to mark that it was finished with a transaction, but the compiler had optimized away the read to the actual HW mapped memory, so the transaction had completed but the timeout continued. Marking the CQs as volatile fixes this issue as the reads happen to the actual HW. Personal GitHub PR: https://github.com/tianocore/edk2/pull/4320 Github branch: https://github.com/os-d/edk2/tree/osde/volatile_cq_v2 Changes v1 => v2: ================= - Remove volatile keyword from SQ Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Michael Kubacki <mikuback@linux.microsoft.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Oliver Smith-Denny (2): Add the volatile keyword to NvmExpressDxe's Passthru CQ Add volatile keyword to NvmExpressPei's Passthru CQ MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++--- MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) -- 2.40.0 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103725): https://edk2.groups.io/g/devel/message/103725 Mute This Topic: https://groups.io/mt/98541927/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
For the series: Acked-by: Michael Kubacki <michael.kubacki@microsoft.com> On 4/27/2023 1:36 PM, Oliver Smith-Denny wrote: > NVMe CQs are hardware queues mapped to EFI memory. > In the NVMExpress Passthru implementations in PEI and DXE, > it has been observed that NVMe CQs are not marked > volatile, meaning the compiler has significant leeway > to optimize accesses to these structures. > > This led to an issue where the passthru driver waited > for a timeout period for an NVMe CQ to mark that it was > finished with a transaction, but the compiler had > optimized away the read to the actual HW mapped memory, > so the transaction had completed but the timeout continued. > > Marking the CQs as volatile fixes this issue as the reads > happen to the actual HW. > > Personal GitHub PR: https://github.com/tianocore/edk2/pull/4320 > Github branch: https://github.com/os-d/edk2/tree/osde/volatile_cq_v2 > > Changes v1 => v2: > ================= > - Remove volatile keyword from SQ > > Cc: Hao A Wu <hao.a.wu@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Michael Kubacki <mikuback@linux.microsoft.com> > Cc: Sean Brogan <sean.brogan@microsoft.com> > > > Oliver Smith-Denny (2): > Add the volatile keyword to NvmExpressDxe's Passthru CQ > Add volatile keyword to NvmExpressPei's Passthru CQ > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++--- > MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104068): https://edk2.groups.io/g/devel/message/104068 Mute This Topic: https://groups.io/mt/98541927/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
For the series: Reviewed-by: Hao A Wu <hao.a.wu@intel.com> Will wait a couple of days before merging to see if comments from other reviewers. Best Regards, Hao Wu > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oliver > Smith-Denny > Sent: Friday, April 28, 2023 1:36 AM > To: devel@edk2.groups.io > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, Jian > J <jian.j.wang@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Michael > Kubacki <mikuback@linux.microsoft.com>; Sean Brogan > <sean.brogan@microsoft.com> > Subject: [edk2-devel][PATCH v2 0/2] Add Volatile Keyword to NvmExpress > Passthru CQs > > NVMe CQs are hardware queues mapped to EFI memory. > In the NVMExpress Passthru implementations in PEI and DXE, > it has been observed that NVMe CQs are not marked > volatile, meaning the compiler has significant leeway > to optimize accesses to these structures. > > This led to an issue where the passthru driver waited > for a timeout period for an NVMe CQ to mark that it was > finished with a transaction, but the compiler had > optimized away the read to the actual HW mapped memory, > so the transaction had completed but the timeout continued. > > Marking the CQs as volatile fixes this issue as the reads > happen to the actual HW. > > Personal GitHub PR: https://github.com/tianocore/edk2/pull/4320 > Github branch: https://github.com/os-d/edk2/tree/osde/volatile_cq_v2 > > Changes v1 => v2: > ================= > - Remove volatile keyword from SQ > > Cc: Hao A Wu <hao.a.wu@intel.com> > Cc: Ray Ni <ray.ni@intel.com> > Cc: Jian J Wang <jian.j.wang@intel.com> > Cc: Liming Gao <gaoliming@byosoft.com.cn> > Cc: Michael Kubacki <mikuback@linux.microsoft.com> > Cc: Sean Brogan <sean.brogan@microsoft.com> > > > Oliver Smith-Denny (2): > Add the volatile keyword to NvmExpressDxe's Passthru CQ > Add volatile keyword to NvmExpressPei's Passthru CQ > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++--- > MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c | 6 +++--- > 2 files changed, 6 insertions(+), 6 deletions(-) > > -- > 2.40.0 > > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > View/Reply Online (#103725): https://edk2.groups.io/g/devel/message/103725 > Mute This Topic: https://groups.io/mt/98541927/1768737 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com] > -=-=-=-=-=-= > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103933): https://edk2.groups.io/g/devel/message/103933 Mute This Topic: https://groups.io/mt/98541927/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Pushed via: PR - https://github.com/tianocore/edk2/pull/4353 Commits: https://github.com/tianocore/edk2/commit/293b97d0c4624c13a4e934294d2c4b161a09a91b https://github.com/tianocore/edk2/commit/8dbf868e02c71b407e31f9b41b5266169c702812 Best Regards, Hao Wu > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Wu, Hao A > Sent: Thursday, May 4, 2023 10:20 AM > To: devel@edk2.groups.io; osde@linux.microsoft.com > Cc: Ni, Ray <ray.ni@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Gao, > Liming <gaoliming@byosoft.com.cn>; Michael Kubacki > <mikuback@linux.microsoft.com>; Sean Brogan <sean.brogan@microsoft.com> > Subject: Re: [edk2-devel][PATCH v2 0/2] Add Volatile Keyword to NvmExpress > Passthru CQs > > For the series: > Reviewed-by: Hao A Wu <hao.a.wu@intel.com> > > Will wait a couple of days before merging to see if comments from other > reviewers. > > Best Regards, > Hao Wu > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oliver > > Smith-Denny > > Sent: Friday, April 28, 2023 1:36 AM > > To: devel@edk2.groups.io > > Cc: Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Wang, > > Jian J <jian.j.wang@intel.com>; Gao, Liming > > <gaoliming@byosoft.com.cn>; Michael Kubacki > > <mikuback@linux.microsoft.com>; Sean Brogan > > <sean.brogan@microsoft.com> > > Subject: [edk2-devel][PATCH v2 0/2] Add Volatile Keyword to NvmExpress > > Passthru CQs > > > > NVMe CQs are hardware queues mapped to EFI memory. > > In the NVMExpress Passthru implementations in PEI and DXE, it has been > > observed that NVMe CQs are not marked volatile, meaning the compiler > > has significant leeway to optimize accesses to these structures. > > > > This led to an issue where the passthru driver waited for a timeout > > period for an NVMe CQ to mark that it was finished with a transaction, > > but the compiler had optimized away the read to the actual HW mapped > > memory, so the transaction had completed but the timeout continued. > > > > Marking the CQs as volatile fixes this issue as the reads happen to > > the actual HW. > > > > Personal GitHub PR: https://github.com/tianocore/edk2/pull/4320 > > Github branch: https://github.com/os-d/edk2/tree/osde/volatile_cq_v2 > > > > Changes v1 => v2: > > ================= > > - Remove volatile keyword from SQ > > > > Cc: Hao A Wu <hao.a.wu@intel.com> > > Cc: Ray Ni <ray.ni@intel.com> > > Cc: Jian J Wang <jian.j.wang@intel.com> > > Cc: Liming Gao <gaoliming@byosoft.com.cn> > > Cc: Michael Kubacki <mikuback@linux.microsoft.com> > > Cc: Sean Brogan <sean.brogan@microsoft.com> > > > > > > Oliver Smith-Denny (2): > > Add the volatile keyword to NvmExpressDxe's Passthru CQ > > Add volatile keyword to NvmExpressPei's Passthru CQ > > > > MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressPassthru.c | 6 +++--- > > MdeModulePkg/Bus/Pci/NvmExpressPei/NvmExpressPeiPassThru.c | 6 +++--- > > 2 files changed, 6 insertions(+), 6 deletions(-) > > > > -- > > 2.40.0 > > > > > > > > -=-=-=-=-=-= > > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#103725): > > https://edk2.groups.io/g/devel/message/103725 > > Mute This Topic: https://groups.io/mt/98541927/1768737 > > Group Owner: devel+owner@edk2.groups.io > > Unsubscribe: https://edk2.groups.io/g/devel/unsub [hao.a.wu@intel.com] > > -=-=-=-=-=-= > > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#104237): https://edk2.groups.io/g/devel/message/104237 Mute This Topic: https://groups.io/mt/98541927/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.