From: "Wu, Hao A" <hao.a.wu@intel.com>
To: 'Ashish Singhal' <ashishsingha@nvidia.com>,
"'devel@edk2.groups.io'" <devel@edk2.groups.io>,
"Ni, Ray" <ray.ni@intel.com>,
"'jbrasen@nvidia.com'" <jbrasen@nvidia.com>
Subject: Re: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation
Date: Thu, 17 Oct 2019 02:43:19 +0000 [thread overview]
Message-ID: <B80AF82E9BFB8E4FBD8C89DA810C6A093C9485F4@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <B80AF82E9BFB8E4FBD8C89DA810C6A093C947D84@SHSMSX104.ccr.corp.intel.com>
> -----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
>
> > -----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.
> >
>
>
> Hello Ashish,
>
> Please grant me some time to verify this patch.
> I will provide updates no later than early next week.
>
> 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() buffer
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 <hao.a.wu@intel.com>
I will wait a couple of days to see if there are other comments. I will then
push the series if there is no concern raised.
Best Regards,
Hao Wu
>
> Best Regards,
> Hao Wu
>
>
> > Change-Id: I32d36bfbff0eee93d14a9a1a86e5ce7635251b64
> > Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> > ---
> > 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 allocation.
> > Must be a power of two.
> > + @param HostAddress A pointer to store the base system memory
> > 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 allocated.
> > + @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal
> > 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 = NULL;
> > + *DeviceAddress = 0;
> > + AlignmentMask = Alignment - 1;
> > +
> > + //
> > + // Calculate the total number of pages since alignment is larger than page
> > size.
> > + //
> > + RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> > +
> > + //
> > + // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> > overflow.
> > + //
> > + ASSERT (RealPages > Pages);
> > +
> > + if (mIoMmu != NULL) {
> > + Status = mIoMmu->AllocateBuffer (
> > + mIoMmu,
> > + EfiBootServicesData,
> > + RealPages,
> > + HostAddress,
> > + 0
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > + Memory = *HostAddress;
> > + AlignedMemory = ((UINTN) Memory + AlignmentMask) &
> > ~AlignmentMask;
> > + UnalignedPages = EFI_SIZE_TO_PAGES (AlignedMemory - (UINTN)
> > Memory);
> > + if (UnalignedPages > 0) {
> > + //
> > + // Free first unaligned page(s).
> > + //
> > + Status = mIoMmu->FreeBuffer (
> > + mIoMmu,
> > + UnalignedPages,
> > + Memory);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > + Memory = (VOID *)(UINTN)(AlignedMemory + EFI_PAGES_TO_SIZE
> > (Pages));
> > + UnalignedPages = RealPages - Pages - UnalignedPages;
> > + if (UnalignedPages > 0) {
> > + //
> > + // Free last unaligned page(s).
> > + //
> > + Status = mIoMmu->FreeBuffer (
> > + mIoMmu,
> > + UnalignedPages,
> > + Memory);
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + }
> > + *HostAddress = (VOID *) AlignedMemory;
> > + NumberOfBytes = EFI_PAGES_TO_SIZE(Pages);
> > + Status = mIoMmu->Map (
> > + mIoMmu,
> > + EdkiiIoMmuOperationBusMasterCommonBuffer,
> > + *HostAddress,
> > + &NumberOfBytes,
> > + DeviceAddress,
> > + Mapping
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > + Status = mIoMmu->SetAttribute (
> > + mIoMmu,
> > + *Mapping,
> > + EDKII_IOMMU_ACCESS_READ |
> EDKII_IOMMU_ACCESS_WRITE
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return Status;
> > + }
> > + } else {
> > + Status = PeiServicesAllocatePages (
> > + EfiBootServicesData,
> > + RealPages,
> > + &HostPhyAddress
> > + );
> > + if (EFI_ERROR (Status)) {
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > + *HostAddress = (VOID *)(((UINTN) HostPhyAddress + AlignmentMask)
> &
> > ~AlignmentMask);
> > + *DeviceAddress = ((UINTN) HostPhyAddress + AlignmentMask) &
> > ~AlignmentMask;
> > + *Mapping = 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 than
> page
> > size.
> > - //
> > - AlignmentMask = Alignment - 1;
> > - RealPages = Pages + EFI_SIZE_TO_PAGES (Alignment);
> > - //
> > - // Make sure that Pages plus EFI_SIZE_TO_PAGES (Alignment) does not
> > overflow.
> > - //
> > - ASSERT (RealPages > Pages);
> > -
> > - Status = IoMmuAllocateBuffer (
> > + Status = IoMmuAllocateAlignedBuffer (
> > Pages,
> > + Alignment,
> > &Memory,
> > &DeviceMemory,
> > Mapping
> > @@ -601,8 +588,6 @@ UsbHcAllocateAlignedPages (
> > if (EFI_ERROR (Status)) {
> > return EFI_OUT_OF_RESOURCES;
> > }
> > - AlignedMemory = ((UINTN) Memory + AlignmentMask) &
> > ~AlignmentMask;
> > - AlignedDeviceMemory = ((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 = (UINTN) Memory;
> > - AlignedDeviceMemory = (UINTN) DeviceMemory;
> > }
> >
> > - *HostAddress = (VOID *) AlignedMemory;
> > - *DeviceAddress = (EFI_PHYSICAL_ADDRESS) AlignedDeviceMemory;
> > + *HostAddress = Memory;
> > + *DeviceAddress = 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 allocation.
> > Must be a power of two.
> > + @param HostAddress A pointer to store the base system memory
> > 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 allocated.
> > + @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal
> > 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
next prev parent reply other threads:[~2019-10-17 2:43 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-15 17:20 [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Ashish Singhal
2019-10-15 17:20 ` [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Ashish Singhal
2019-10-16 2:11 ` [edk2-devel] " Wu, Hao A
2019-10-15 17:20 ` [PATCH v4 2/2] MdeModulePkg/XhciPei: " Ashish Singhal
2019-10-16 2:13 ` Wu, Hao A
2019-10-17 2:43 ` Wu, Hao A [this message]
2019-10-21 1:02 ` [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Wu, Hao A
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=B80AF82E9BFB8E4FBD8C89DA810C6A093C9485F4@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox