From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by mx.groups.io with SMTP id smtpd.web11.1667.1571280205566447673 for ; Wed, 16 Oct 2019 19:43:25 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.120, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Oct 2019 19:43:25 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,306,1566889200"; d="scan'208";a="208539944" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga001.fm.intel.com with ESMTP; 16 Oct 2019 19:43:25 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 16 Oct 2019 19:43:25 -0700 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.439.0; Wed, 16 Oct 2019 19:43:23 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.33]) with mapi id 14.03.0439.000; Thu, 17 Oct 2019 10:43:19 +0800 From: "Wu, Hao A" To: 'Ashish Singhal' , "'devel@edk2.groups.io'" , "Ni, Ray" , "'jbrasen@nvidia.com'" Subject: Re: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation Thread-Topic: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation Thread-Index: AQHVg3zu0KkTk4OYSkWluuy09bOKKKdchzrQgAGZSJA= Date: Thu, 17 Oct 2019 02:43:19 +0000 Message-ID: References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Wu, Hao A > Sent: Wednesday, October 16, 2019 10:14 AM > To: Ashish Singhal; devel@edk2.groups.io; Ni, Ray; jbrasen@nvidia.com > Subject: RE: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page > Allocation >=20 > > -----Original Message----- > > From: Ashish Singhal [mailto:ashishsingha@nvidia.com] > > Sent: Wednesday, October 16, 2019 1:21 AM > > To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray; jbrasen@nvidia.com > > Cc: Ashish Singhal > > Subject: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page > > Allocation > > > > Add support for allocating aligned pages at an alignment higher > > than 4K. The new function allocated memory taking into account > > the padding required and then frees up unused pages before mapping > > it. > > >=20 >=20 > Hello Ashish, >=20 > Please grant me some time to verify this patch. > I will provide updates no later than early next week. >=20 > Sorry for the possible delay. Hello, Though I cannot directly verify the patch on USB devices at this moment, I = have verified this patch by the following approach: * Duplicate the newly introduced function IoMmuAllocateAlignedBuffer() buff= er to the ATA PEI driver stack; * Use the IoMmuAllocateAlignedBuffer() API instead of IoMmuAllocateBuffer()= to allocate the IOMMU buffer; * Verified that the size and alignment of the allocated buffer are expected= . I think the above test should be enough for the patch, so: Reviewed-by: Hao A Wu I will wait a couple of days to see if there are other comments. I will the= n push the series if there is no concern raised. Best Regards, Hao Wu >=20 > Best Regards, > Hao Wu >=20 >=20 > > Change-Id: I32d36bfbff0eee93d14a9a1a86e5ce7635251b64 > > Signed-off-by: Ashish Singhal > > --- > > MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c | 128 > > ++++++++++++++++++++++++++++++++ > > MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c | 25 +------ > > MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h | 28 +++++++ > > 3 files changed, 160 insertions(+), 21 deletions(-) > > > > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c > > b/MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c > > index 71d6113..c4d93ae 100644 > > --- a/MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c > > +++ b/MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c > > @@ -225,6 +225,134 @@ IoMmuFreeBuffer ( > > } > > > > /** > > + Allocates aligned pages that are suitable for an > > OperationBusMasterCommonBuffer or > > + OperationBusMasterCommonBuffer64 mapping. > > + > > + @param Pages The number of pages to allocate. > > + @param Alignment The requested alignment of the allocat= ion. > > Must be a power of two. > > + @param HostAddress A pointer to store the base system mem= ory > > address of the > > + allocated range. > > + @param DeviceAddress The resulting map address for the bus > master > > PCI controller to use to > > + access the hosts HostAddress. > > + @param Mapping A resulting value to pass to Unmap(). > > + > > + @retval EFI_SUCCESS The requested memory pages were alloca= ted. > > + @retval EFI_UNSUPPORTED Attributes is unsupported. The only le= gal > > attribute bits are > > + MEMORY_WRITE_COMBINE and MEMORY_CACHED= . > > + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. > > + @retval EFI_OUT_OF_RESOURCES The memory pages could not be > > allocated. > > + > > +**/ > > +EFI_STATUS > > +IoMmuAllocateAlignedBuffer ( > > + IN UINTN Pages, > > + IN UINTN Alignment, > > + OUT VOID **HostAddress, > > + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, > > + OUT VOID **Mapping > > + ) > > +{ > > + EFI_STATUS Status; > > + VOID *Memory; > > + UINTN AlignedMemory; > > + UINTN AlignmentMask; > > + UINTN UnalignedPages; > > + UINTN RealPages; > > + UINTN NumberOfBytes; > > + EFI_PHYSICAL_ADDRESS HostPhyAddress; > > + > > + *HostAddress =3D NULL; > > + *DeviceAddress =3D 0; > > + AlignmentMask =3D Alignment - 1; > > + > > + // > > + // Calculate the total number of pages since alignment is larger tha= n page > > size. > > + // > > + RealPages =3D Pages + EFI_SIZE_TO_PAGES (Alignment); > > + > > + // > > + // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not > > overflow. > > + // > > + ASSERT (RealPages > Pages); > > + > > + if (mIoMmu !=3D NULL) { > > + Status =3D mIoMmu->AllocateBuffer ( > > + mIoMmu, > > + EfiBootServicesData, > > + RealPages, > > + HostAddress, > > + 0 > > + ); > > + if (EFI_ERROR (Status)) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + Memory =3D *HostAddress; > > + AlignedMemory =3D ((UINTN) Memory + AlignmentMask) & > > ~AlignmentMask; > > + UnalignedPages =3D EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN) > > Memory); > > + if (UnalignedPages > 0) { > > + // > > + // Free first unaligned page(s). > > + // > > + Status =3D mIoMmu->FreeBuffer ( > > + mIoMmu, > > + UnalignedPages, > > + Memory); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + } > > + Memory =3D (VOID *)(UINTN)(AlignedMemory + EFI_PAGES_TO_SI= ZE > > (Pages)); > > + UnalignedPages =3D RealPages - Pages - UnalignedPages; > > + if (UnalignedPages > 0) { > > + // > > + // Free last unaligned page(s). > > + // > > + Status =3D mIoMmu->FreeBuffer ( > > + mIoMmu, > > + UnalignedPages, > > + Memory); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + } > > + *HostAddress =3D (VOID *) AlignedMemory; > > + NumberOfBytes =3D EFI_PAGES_TO_SIZE(Pages); > > + Status =3D mIoMmu->Map ( > > + mIoMmu, > > + EdkiiIoMmuOperationBusMasterCommonBuffer, > > + *HostAddress, > > + &NumberOfBytes, > > + DeviceAddress, > > + Mapping > > + ); > > + if (EFI_ERROR (Status)) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + Status =3D mIoMmu->SetAttribute ( > > + mIoMmu, > > + *Mapping, > > + EDKII_IOMMU_ACCESS_READ | > EDKII_IOMMU_ACCESS_WRITE > > + ); > > + if (EFI_ERROR (Status)) { > > + return Status; > > + } > > + } else { > > + Status =3D PeiServicesAllocatePages ( > > + EfiBootServicesData, > > + RealPages, > > + &HostPhyAddress > > + ); > > + if (EFI_ERROR (Status)) { > > + return EFI_OUT_OF_RESOURCES; > > + } > > + *HostAddress =3D (VOID *)(((UINTN) HostPhyAddress + AlignmentMask) > & > > ~AlignmentMask); > > + *DeviceAddress =3D ((UINTN) HostPhyAddress + AlignmentMask) & > > ~AlignmentMask; > > + *Mapping =3D NULL; > > + } > > + return Status; > > +} > > + > > +/** > > Initialize IOMMU. > > **/ > > VOID > > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > b/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > index 56c0b90..01f42285 100644 > > --- a/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > +++ b/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > @@ -562,11 +562,7 @@ UsbHcAllocateAlignedPages ( > > { > > EFI_STATUS Status; > > VOID *Memory; > > - UINTN AlignedMemory; > > - UINTN AlignmentMask; > > EFI_PHYSICAL_ADDRESS DeviceMemory; > > - UINTN AlignedDeviceMemory; > > - UINTN RealPages; > > > > // > > // Alignment must be a power of two or zero. > > @@ -582,18 +578,9 @@ UsbHcAllocateAlignedPages ( > > } > > > > if (Alignment > EFI_PAGE_SIZE) { > > - // > > - // Calculate the total number of pages since alignment is larger t= han > page > > size. > > - // > > - AlignmentMask =3D Alignment - 1; > > - RealPages =3D Pages + EFI_SIZE_TO_PAGES (Alignment); > > - // > > - // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does no= t > > overflow. > > - // > > - ASSERT (RealPages > Pages); > > - > > - Status =3D IoMmuAllocateBuffer ( > > + Status =3D IoMmuAllocateAlignedBuffer ( > > Pages, > > + Alignment, > > &Memory, > > &DeviceMemory, > > Mapping > > @@ -601,8 +588,6 @@ UsbHcAllocateAlignedPages ( > > if (EFI_ERROR (Status)) { > > return EFI_OUT_OF_RESOURCES; > > } > > - AlignedMemory =3D ((UINTN) Memory + AlignmentMask) & > > ~AlignmentMask; > > - AlignedDeviceMemory =3D ((UINTN) DeviceMemory + AlignmentMask) & > > ~AlignmentMask; > > } else { > > // > > // Do not over-allocate pages in this case. > > @@ -616,12 +601,10 @@ UsbHcAllocateAlignedPages ( > > if (EFI_ERROR (Status)) { > > return EFI_OUT_OF_RESOURCES; > > } > > - AlignedMemory =3D (UINTN) Memory; > > - AlignedDeviceMemory =3D (UINTN) DeviceMemory; > > } > > > > - *HostAddress =3D (VOID *) AlignedMemory; > > - *DeviceAddress =3D (EFI_PHYSICAL_ADDRESS) AlignedDeviceMemory; > > + *HostAddress =3D Memory; > > + *DeviceAddress =3D DeviceMemory; > > > > return EFI_SUCCESS; > > } > > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h > > b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h > > index 480e13b..03a55f3 100644 > > --- a/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h > > +++ b/MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h > > @@ -342,4 +342,32 @@ IoMmuFreeBuffer ( > > IN VOID *Mapping > > ); > > > > +/** > > + Allocates aligned pages that are suitable for an > > OperationBusMasterCommonBuffer or > > + OperationBusMasterCommonBuffer64 mapping. > > + > > + @param Pages The number of pages to allocate. > > + @param Alignment The requested alignment of the allocat= ion. > > Must be a power of two. > > + @param HostAddress A pointer to store the base system mem= ory > > address of the > > + allocated range. > > + @param DeviceAddress The resulting map address for the bus > master > > PCI controller to use to > > + access the hosts HostAddress. > > + @param Mapping A resulting value to pass to Unmap(). > > + > > + @retval EFI_SUCCESS The requested memory pages were alloca= ted. > > + @retval EFI_UNSUPPORTED Attributes is unsupported. The only le= gal > > attribute bits are > > + MEMORY_WRITE_COMBINE and MEMORY_CACHED= . > > + @retval EFI_INVALID_PARAMETER One or more parameters are invalid. > > + @retval EFI_OUT_OF_RESOURCES The memory pages could not be > > allocated. > > + > > +**/ > > +EFI_STATUS > > +IoMmuAllocateAlignedBuffer ( > > + IN UINTN Pages, > > + IN UINTN Alignment, > > + OUT VOID **HostAddress, > > + OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, > > + OUT VOID **Mapping > > + ); > > + > > #endif > > -- > > 2.7.4