* [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI
@ 2019-10-15 17:20 Ashish Singhal
2019-10-15 17:20 ` [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Ashish Singhal
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ashish Singhal @ 2019-10-15 17:20 UTC (permalink / raw)
To: devel, hao.a.wu, ray.ni, jbrasen; +Cc: Ashish Singhal
This patch set is an attempt to fix the error where we allocate incorrectly
aligned memory for XHCI PEI and DXE. The change for DXE phase has been verified
already but change for PEI needs to be verified by Hao as I do not have a
setup to be able to verify that.
The change in DXE just updates a parameter passed in to allocate aligned memory.
The change in PEI adds a new function to allocate aligned memory. There was no
need to add separate function to free aligned pages as unaligned pages have been
already freed during allocation function and the aligned one can be freed using
the existing function.
Ashish Singhal (2):
MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
MdeModulePkg/XhciPei: Fix Aligned Page Allocation
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 2 +-
MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c | 128 ++++++++++++++++++++++++++++++++
MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c | 25 +------
MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h | 28 +++++++
4 files changed, 161 insertions(+), 22 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-15 17:20 [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Ashish Singhal
@ 2019-10-15 17:20 ` 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-21 1:02 ` [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Wu, Hao A
2 siblings, 1 reply; 7+ messages in thread
From: Ashish Singhal @ 2019-10-15 17:20 UTC (permalink / raw)
To: devel, hao.a.wu, ray.ni, jbrasen; +Cc: Ashish Singhal
While allocating pages aligned at an alignment higher than
4K, allocate memory taking into consideration the padding
required for that alignment. The calls to free pages takes
care of this already.
Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
index fd79988..aa69c47 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
@@ -656,7 +656,7 @@ UsbHcAllocateAlignedPages (
PciIo,
AllocateAnyPages,
EfiBootServicesData,
- Pages,
+ RealPages,
&Memory,
0
);
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation
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-15 17:20 ` Ashish Singhal
2019-10-16 2:13 ` Wu, Hao A
2019-10-21 1:02 ` [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Wu, Hao A
2 siblings, 1 reply; 7+ messages in thread
From: Ashish Singhal @ 2019-10-15 17:20 UTC (permalink / raw)
To: devel, hao.a.wu, ray.ni, jbrasen; +Cc: Ashish Singhal
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.
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-15 17:20 ` [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Ashish Singhal
@ 2019-10-16 2:11 ` Wu, Hao A
0 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2019-10-16 2:11 UTC (permalink / raw)
To: devel@edk2.groups.io, ashishsingha@nvidia.com, Ni, Ray,
jbrasen@nvidia.com
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ashish Singhal
> 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: [edk2-devel] [PATCH v4 1/2] MdeModulePkg/XhciDxe: Fix Aligned
> Page Allocation
>
> While allocating pages aligned at an alignment higher than
> 4K, allocate memory taking into consideration the padding
> required for that alignment. The calls to free pages takes
> care of this already.
>
> Signed-off-by: Ashish Singhal <ashishsingha@nvidia.com>
> ---
> MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
> b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
> index fd79988..aa69c47 100644
> --- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
> @@ -656,7 +656,7 @@ UsbHcAllocateAlignedPages (
> PciIo,
> AllocateAnyPages,
> EfiBootServicesData,
> - Pages,
> + RealPages,
> &Memory,
> 0
> );
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
Best Regards,
Hao Wu
> --
> 2.7.4
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation
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
0 siblings, 1 reply; 7+ messages in thread
From: Wu, Hao A @ 2019-10-16 2:13 UTC (permalink / raw)
To: Ashish Singhal, devel@edk2.groups.io, Ni, Ray, jbrasen@nvidia.com
> -----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.
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v4 2/2] MdeModulePkg/XhciPei: Fix Aligned Page Allocation
2019-10-16 2:13 ` Wu, Hao A
@ 2019-10-17 2:43 ` Wu, Hao A
0 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2019-10-17 2:43 UTC (permalink / raw)
To: 'Ashish Singhal', 'devel@edk2.groups.io', Ni, Ray,
'jbrasen@nvidia.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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI
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-15 17:20 ` [PATCH v4 2/2] MdeModulePkg/XhciPei: " Ashish Singhal
@ 2019-10-21 1:02 ` Wu, Hao A
2 siblings, 0 replies; 7+ messages in thread
From: Wu, Hao A @ 2019-10-21 1:02 UTC (permalink / raw)
To: devel@edk2.groups.io, ashishsingha@nvidia.com, Ni, Ray,
jbrasen@nvidia.com
> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Ashish Singhal
> 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: [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI
>
> This patch set is an attempt to fix the error where we allocate incorrectly
> aligned memory for XHCI PEI and DXE. The change for DXE phase has been
> verified
> already but change for PEI needs to be verified by Hao as I do not have a
> setup to be able to verify that.
>
> The change in DXE just updates a parameter passed in to allocate aligned
> memory.
> The change in PEI adds a new function to allocate aligned memory. There was
> no
> need to add separate function to free aligned pages as unaligned pages have
> been
> already freed during allocation function and the aligned one can be freed
> using
> the existing function.
>
> Ashish Singhal (2):
> MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> MdeModulePkg/XhciPei: Fix Aligned Page Allocation
>
> MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c | 2 +-
> MdeModulePkg/Bus/Pci/XhciPei/DmaMem.c | 128
> ++++++++++++++++++++++++++++++++
> MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c | 25 +------
> MdeModulePkg/Bus/Pci/XhciPei/XhcPeim.h | 28 +++++++
> 4 files changed, 161 insertions(+), 22 deletions(-)
Thanks for resolving the issues.
Series pushed via commits 0f28c513d3..2bbbdeeea2.
Best Regards,
Hao Wu
>
> --
> 2.7.4
>
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-21 1:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2019-10-21 1:02 ` [edk2-devel] [PATCH v4 0/2] Fix Aligned Page Allocation For XHCI Wu, Hao A
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox