From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by mx.groups.io with SMTP id smtpd.web11.9369.1645703938693404201 for ; Thu, 24 Feb 2022 03:58:59 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=BICu+nJx; spf=pass (domain: kernel.org, ip: 145.40.68.75, mailfrom: ardb@kernel.org) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 43DCEB8256D for ; Thu, 24 Feb 2022 11:58:56 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E5B48C340EF for ; Thu, 24 Feb 2022 11:58:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1645703934; bh=EnsDdntiaHgVpyve2GWLqdGNvK0BZn1yqbc0vHdQyZY=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=BICu+nJxwtNyBEpQw74vZ0HhwBAzLYZeTjaMCAoEoHgZh0fo9aKrX/8LsfOfvT5n4 l/RvXHisqH5doqQp56c83ibZ9lUAVMFOjLlC4aqZi5rkKQ1vlZ1mjDJicAYd1nEMLJ dz9L0Gg0S3kREu/YmWn4GoM/dvu98CpAQmxfDBgtviLvFkcLSvPsIdxeJA70VqVfQL SJqCrjT5GQkVvSIgtCw3MytFwjbacoHo/1q90MT5nh2sW3tjFVSm8VSJItibnrM3oB irqwB9BJ6tXAtqNKC835QcfipT5xrXobKdQyaTVUozelZfCD/8eO/8CF7Z3hcCc8FU RqEy7lL7/szaw== Received: by mail-yb1-f171.google.com with SMTP id v186so3329408ybg.1 for ; Thu, 24 Feb 2022 03:58:54 -0800 (PST) X-Gm-Message-State: AOAM533583FzBAPoxP6eSQhMd+Vfy39g6b3SUv7VJ4Jh0Zxk+y1gs3SG 0Md9bUCjOMBECAr6zfipIrHmouCLmUhhkpYlyDI= X-Google-Smtp-Source: ABdhPJyOG77Dw/7RRuWHMy4WQ2vmKOX3wGYafqUuMBr3zMsUdxH1xwnbl2DqX08isDhLTG4jrCciE5LhO0oPDtOdGpU= X-Received: by 2002:a25:af8e:0:b0:622:c778:c0a2 with SMTP id g14-20020a25af8e000000b00622c778c0a2mr2018458ybh.50.1645703933936; Thu, 24 Feb 2022 03:58:53 -0800 (PST) MIME-Version: 1.0 References: <20220224114744.1966974-1-quic_tpilar@quicinc.com> In-Reply-To: <20220224114744.1966974-1-quic_tpilar@quicinc.com> From: "Ard Biesheuvel" Date: Thu, 24 Feb 2022 12:58:42 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] MdeModulePkg: Correct high-memory use in NvmExpressDxe To: Tomas Pilar Cc: edk2-devel-groups-io , Ray Ni , Ard Biesheuvel , Leif Lindholm Content-Type: text/plain; charset="UTF-8" On Thu, 24 Feb 2022 at 12:48, Tomas Pilar wrote: > > Delay and move the allocation and mapping of memory that backs the DMA > engine in NvmExpress devices to NvmeInit() to ensure that > the allocation only happens after the > EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE attribute is set > on the PciIo controller. > > 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. > > Cc: Ray Ni > Cc: Ard Biesheuvel > Cc: Leif Lindholm > Signed-off-by: Tomas Pilar NvmeControllerInit () can be called multiple times, no? So you should probably make sure that the buffer is not allocated and mapped again if one already exists. > --- > .../Bus/Pci/NvmExpressDxe/NvmExpress.c | 41 ------------- > .../Bus/Pci/NvmExpressDxe/NvmExpressHci.c | 57 ++++++++++++++++--- > 2 files changed, 49 insertions(+), 49 deletions(-) > > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c > index 9d40f67e8e..cc921756f5 100644 > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpress.c > @@ -912,8 +912,6 @@ NvmExpressDriverBindingStart ( > NVME_CONTROLLER_PRIVATE_DATA *Private; > EFI_DEVICE_PATH_PROTOCOL *ParentDevicePath; > UINT32 NamespaceId; > - EFI_PHYSICAL_ADDRESS MappedAddr; > - UINTN Bytes; > EFI_NVM_EXPRESS_PASS_THRU_PROTOCOL *Passthru; > > DEBUG ((DEBUG_INFO, "NvmExpressDriverBindingStart: start\n")); > @@ -959,45 +957,6 @@ NvmExpressDriverBindingStart ( > goto Exit; > } > > - // > - // 6 x 4kB aligned buffers will be carved out of this buffer. > - // 1st 4kB boundary is the start of the admin submission queue. > - // 2nd 4kB boundary is the start of the admin completion queue. > - // 3rd 4kB boundary is the start of I/O submission queue #1. > - // 4th 4kB boundary is the start of I/O completion queue #1. > - // 5th 4kB boundary is the start of I/O submission queue #2. > - // 6th 4kB boundary is the start of I/O completion queue #2. > - // > - // Allocate 6 pages of memory, then map it for bus master read and write. > - // > - Status = PciIo->AllocateBuffer ( > - PciIo, > - AllocateAnyPages, > - EfiBootServicesData, > - 6, > - (VOID **)&Private->Buffer, > - 0 > - ); > - if (EFI_ERROR (Status)) { > - goto Exit; > - } > - > - Bytes = EFI_PAGES_TO_SIZE (6); > - Status = PciIo->Map ( > - PciIo, > - EfiPciIoOperationBusMasterCommonBuffer, > - Private->Buffer, > - &Bytes, > - &MappedAddr, > - &Private->Mapping > - ); > - > - if (EFI_ERROR (Status) || (Bytes != EFI_PAGES_TO_SIZE (6))) { > - goto Exit; > - } > - > - Private->BufferPciAddr = (UINT8 *)(UINTN)MappedAddr; > - > Private->Signature = NVME_CONTROLLER_PRIVATE_DATA_SIGNATURE; > Private->ControllerHandle = Controller; > Private->ImageHandle = This->DriverBindingHandle; > diff --git a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c > index ac77afe113..359373300e 100644 > --- a/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c > +++ b/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c > @@ -718,14 +718,16 @@ NvmeControllerInit ( > IN NVME_CONTROLLER_PRIVATE_DATA *Private > ) > { > - EFI_STATUS Status; > - EFI_PCI_IO_PROTOCOL *PciIo; > - UINT64 Supports; > - NVME_AQA Aqa; > - NVME_ASQ Asq; > - NVME_ACQ Acq; > - UINT8 Sn[21]; > - UINT8 Mn[41]; > + EFI_STATUS Status; > + EFI_PCI_IO_PROTOCOL *PciIo; > + UINT64 Supports; > + NVME_AQA Aqa; > + NVME_ASQ Asq; > + NVME_ACQ Acq; > + UINT8 Sn[21]; > + UINT8 Mn[41]; > + UINTN Bytes; > + EFI_PHYSICAL_ADDRESS MappedAddr; > > // > // Save original PCI attributes and enable this controller. > @@ -777,6 +779,45 @@ NvmeControllerInit ( > DEBUG ((DEBUG_WARN, "NvmeControllerInit: 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. > + // 2nd 4kB boundary is the start of the admin completion queue. > + // 3rd 4kB boundary is the start of I/O submission queue #1. > + // 4th 4kB boundary is the start of I/O completion queue #1. > + // 5th 4kB boundary is the start of I/O submission queue #2. > + // 6th 4kB boundary is the start of I/O completion queue #2. > + // > + // Allocate 6 pages of memory, then map it for bus master read and write. > + // > + Status = PciIo->AllocateBuffer ( > + PciIo, > + AllocateAnyPages, > + EfiBootServicesData, > + 6, > + (VOID **)&Private->Buffer, > + 0 > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Bytes = EFI_PAGES_TO_SIZE (6); > + Status = PciIo->Map ( > + PciIo, > + EfiPciIoOperationBusMasterCommonBuffer, > + Private->Buffer, > + &Bytes, > + &MappedAddr, > + &Private->Mapping > + ); > + > + if (EFI_ERROR (Status) || (Bytes != EFI_PAGES_TO_SIZE (6))) { > + return Status; > + } > + > + Private->BufferPciAddr = (UINT8 *)(UINTN)MappedAddr; > + > // > // Read the Controller Capabilities register and verify that the NVM command set is supported > // > -- > 2.30.2 >