Re: [PATCH v5] MdeModulePkg: Correct high-memory use in NvmExpressDxe

["Ard Biesheuvel" <ardb@kernel.org>]

On Fri, 25 Feb 2022 at 09:17, Wu, Hao A <hao.a.wu@intel.com> wrote:
>
> Acked-by: Hao A Wu <hao.a.wu@intel.com>
>

Merged as #2561

Thanks all


> > -----Original Message-----
> > From: Tomas Pilar <quic_tpilar@quicinc.com>
> > Sent: Friday, February 25, 2022 12:22 AM
> > To: devel@edk2.groups.io
> > Cc: Ni, Ray <ray.ni@intel.com>; Ard Biesheuvel
> > <ardb+tianocore@kernel.org>; Leif Lindholm <quic_llindhol@quicinc.com>;
> > Wu, Hao A <hao.a.wu@intel.com>; Ard Biesheuvel <ardb@kernel.org>
> > Subject: [PATCH v5] MdeModulePkg: Correct high-memory use in
> > NvmExpressDxe
> >
> > Move the logic that stores starting PCI attributes and sets the
> > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute to
> > DriverBindingStart() before the memory that backs the
> > DMA engine is allocated.
> >
> > This ensures that the DMA-backing memory is not forcibly allocated
> > below 4G in system address map. Otherwise the allocation fails on
> > platforms that do not have any memory below the 4G mark and the drive
> > initialisation fails.
> >
> > Leave the PCI device enabling attribute logic in NvmeControllerInit()
> > to ensure that the device is re-enabled on reset in case it was
> > disabled via PCI attributes.
> >
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Leif Lindholm <quic_llindhol@quicinc.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Reviewed-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Tomas Pilar <quic_tpilar@quicinc.com>
> > ---
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c    | 27
> > ++++++++++++++++++++
> >  MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 26 +------------
> > ------
> >  2 files changed, 28 insertions(+), 25 deletions(-)
> >
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> > index 9d40f67e8e..5a1eda8e8d 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c
> > @@ -959,6 +959,33 @@ NvmExpressDriverBindingStart (
> >        goto Exit;
> >      }
> >
> > +    //
> > +    // Save original PCI attributes
> > +    //
> > +    Status = PciIo->Attributes (
> > +                      PciIo,
> > +                      EfiPciIoAttributeOperationGet,
> > +                      0,
> > +                      &Private->PciAttributes
> > +                      );
> > +
> > +    if (EFI_ERROR (Status)) {
> > +      return Status;
> > +    }
> > +
> > +    //
> > +    // Enable 64-bit DMA support in the PCI layer.
> > +    //
> > +    Status = PciIo->Attributes (
> > +                      PciIo,
> > +                      EfiPciIoAttributeOperationEnable,
> > +                      EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
> > +                      NULL
> > +                      );
> > +    if (EFI_ERROR (Status)) {
> > +      DEBUG ((DEBUG_WARN, "NvmExpressDriverBindingStart: failed to
> > enable 64-bit DMA (%r)\n", Status));
> > +    }
> > +
> >      //
> >      // 6 x 4kB aligned buffers will be carved out of this buffer.
> >      // 1st 4kB boundary is the start of the admin submission queue.
> > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> > b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> > index ac77afe113..d87212ffb2 100644
> > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c
> > @@ -728,20 +728,9 @@ NvmeControllerInit (
> >    UINT8                Mn[41];
> >
> >    //
> > -  // Save original PCI attributes and enable this controller.
> > +  // Enable this controller.
> >    //
> >    PciIo  = Private->PciIo;
> > -  Status = PciIo->Attributes (
> > -                    PciIo,
> > -                    EfiPciIoAttributeOperationGet,
> > -                    0,
> > -                    &Private->PciAttributes
> > -                    );
> > -
> > -  if (EFI_ERROR (Status)) {
> > -    return Status;
> > -  }
> > -
> >    Status = PciIo->Attributes (
> >                      PciIo,
> >                      EfiPciIoAttributeOperationSupported,
> > @@ -764,19 +753,6 @@ NvmeControllerInit (
> >      return Status;
> >    }
> >
> > -  //
> > -  // Enable 64-bit DMA support in the PCI layer.
> > -  //
> > -  Status = PciIo->Attributes (
> > -                    PciIo,
> > -                    EfiPciIoAttributeOperationEnable,
> > -                    EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
> > -                    NULL
> > -                    );
> > -  if (EFI_ERROR (Status)) {
> > -    DEBUG ((DEBUG_WARN, "NvmeControllerInit: failed to enable 64-bit
> > DMA (%r)\n", Status));
> > -  }
> > -
> >    //
> >    // Read the Controller Capabilities register and verify that the NVM
> > command set is supported
> >    //
> > --
> > 2.30.2