* [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
@ 2019-10-09 17:01 Ashish Singhal
2019-10-10 1:32 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-09 17:01 UTC (permalink / raw)
To: devel, hao.a.wu, ray.ni; +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] 12+ messages in thread
* Re: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-09 17:01 [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Ashish Singhal
@ 2019-10-10 1:32 ` Wu, Hao A
2019-10-10 1:58 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-10-10 1:32 UTC (permalink / raw)
To: Ashish Singhal, devel@edk2.groups.io, Ni, Ray
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 1:02 AM
> To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray
> Cc: Ashish Singhal
> Subject: [PATCH] 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
> );
Hello,
The change looks good to me.
Just a couple of things to confirm:
1. I think there is a similar case within the XhciPei driver
Could you help to resolve it as well?
I think for the PEI counterpart you may need to update both
UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the IOMMU helper
functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer() might not be
suitable now. Instead, I think services in the IoMmu PPI can be used.
2. Could you help to provide the information on what test has been done for the
proposed patch?
Thanks in advance.
Best Regards,
Hao Wu
> --
> 2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-10 1:32 ` Wu, Hao A
@ 2019-10-10 1:58 ` Ashish Singhal
2019-10-10 2:09 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-10 1:58 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
Hello Hao,
I can see that the PEI also has the same issue and take a look at that as well but I have no way to verify that as we are not using it. For the change I have made in DXE, I have verified it with an alignment of 4K and 64K.
Thanks
Ashish
-----Original Message-----
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Wednesday, October 9, 2019 7:33 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 1:02 AM
> To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray
> Cc: Ashish Singhal
> Subject: [PATCH] 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
> );
Hello,
The change looks good to me.
Just a couple of things to confirm:
1. I think there is a similar case within the XhciPei driver Could you help to resolve it as well?
I think for the PEI counterpart you may need to update both
UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer() might not be suitable now. Instead, I think services in the IoMmu PPI can be used.
2. Could you help to provide the information on what test has been done for the proposed patch?
Thanks in advance.
Best Regards,
Hao Wu
> --
> 2.7.4
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-10 1:58 ` Ashish Singhal
@ 2019-10-10 2:09 ` Wu, Hao A
2019-10-10 17:51 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-10-10 2:09 UTC (permalink / raw)
To: Ashish Singhal, devel@edk2.groups.io, Ni, Ray
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-10 2:09 ` Wu, Hao A
@ 2019-10-10 17:51 ` Ashish Singhal
2019-10-11 2:59 ` [edk2-devel] " Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-10 17:51 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 4272 bytes --]
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 7416 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-10 17:51 ` Ashish Singhal
@ 2019-10-11 2:59 ` Wu, Hao A
2019-10-11 3:30 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-10-11 2:59 UTC (permalink / raw)
To: devel@edk2.groups.io, ashishsingha@nvidia.com, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 4711 bytes --]
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 32754 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-11 2:59 ` [edk2-devel] " Wu, Hao A
@ 2019-10-11 3:30 ` Ashish Singhal
2019-10-11 5:53 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-11 3:30 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 5912 bytes --]
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 14634 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-11 3:30 ` Ashish Singhal
@ 2019-10-11 5:53 ` Wu, Hao A
2019-10-11 18:07 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-10-11 5:53 UTC (permalink / raw)
To: devel@edk2.groups.io, ashishsingha@nvidia.com, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 7177 bytes --]
Hello Ashish,
I think the IOMMU feature is added for the protection of FW integrity against
HW devices, which is the motivation to use IOMMU PPI for memory allocation.
The fix is easy for the DXE part since the IOMMU part has been integrated to the
implementation of the PciIo protocol.
As for the XhciPei case, my thought is that a new helper function (maybe called
'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece
between IoMmuAllocateAlignedBuffer() and existing IoMmuAllocateBuffer() will be:
1. If IOMMU PPI exists, mIoMmu->AllocateBuffer() should be called with bigger
number of pages for alignment and mIoMmu->FreeBuffer() can be called right after
to free the unused pages.
2. If IOMMU PPI does not exist, this case will fall back to a similar fix for
the DXE case.
Ideally, IoMmuFreeBuffer() can be reused to free the aligned buffer.
Please help to raise if you observe any open for the above proposal, thanks.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 11:30 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 42573 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-11 5:53 ` Wu, Hao A
@ 2019-10-11 18:07 ` Ashish Singhal
2019-10-14 18:17 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-11 18:07 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 7678 bytes --]
Patch set 2 has been submitted for review.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 11:53 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Ashish,
I think the IOMMU feature is added for the protection of FW integrity against
HW devices, which is the motivation to use IOMMU PPI for memory allocation.
The fix is easy for the DXE part since the IOMMU part has been integrated to the
implementation of the PciIo protocol.
As for the XhciPei case, my thought is that a new helper function (maybe called
'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece
between IoMmuAllocateAlignedBuffer() and existing IoMmuAllocateBuffer() will be:
1. If IOMMU PPI exists, mIoMmu->AllocateBuffer() should be called with bigger
number of pages for alignment and mIoMmu->FreeBuffer() can be called right after
to free the unused pages.
2. If IOMMU PPI does not exist, this case will fall back to a similar fix for
the DXE case.
Ideally, IoMmuFreeBuffer() can be reused to free the aligned buffer.
Please help to raise if you observe any open for the above proposal, thanks.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 11:30 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 21632 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-11 18:07 ` Ashish Singhal
@ 2019-10-14 18:17 ` Ashish Singhal
2019-10-15 1:21 ` Wu, Hao A
0 siblings, 1 reply; 12+ messages in thread
From: Ashish Singhal @ 2019-10-14 18:17 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 8089 bytes --]
Hello Hao,
Were you able to validate the PEI change I sent last week?
Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Friday, October 11, 2019 12:07 PM
To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Patch set 2 has been submitted for review.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 11:53 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Ashish,
I think the IOMMU feature is added for the protection of FW integrity against
HW devices, which is the motivation to use IOMMU PPI for memory allocation.
The fix is easy for the DXE part since the IOMMU part has been integrated to the
implementation of the PciIo protocol.
As for the XhciPei case, my thought is that a new helper function (maybe called
'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece
between IoMmuAllocateAlignedBuffer() and existing IoMmuAllocateBuffer() will be:
1. If IOMMU PPI exists, mIoMmu->AllocateBuffer() should be called with bigger
number of pages for alignment and mIoMmu->FreeBuffer() can be called right after
to free the unused pages.
2. If IOMMU PPI does not exist, this case will fall back to a similar fix for
the DXE case.
Ideally, IoMmuFreeBuffer() can be reused to free the aligned buffer.
Please help to raise if you observe any open for the above proposal, thanks.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 11:30 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 23253 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-14 18:17 ` Ashish Singhal
@ 2019-10-15 1:21 ` Wu, Hao A
2019-10-15 17:19 ` Ashish Singhal
0 siblings, 1 reply; 12+ messages in thread
From: Wu, Hao A @ 2019-10-15 1:21 UTC (permalink / raw)
To: Ashish Singhal, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 9550 bytes --]
Hello,
Sorry for the delayed response.
For the PEI changes, I found that when using the function IoMmuFreeBuffer() to
free the first/last unaligned page(s), the below operations related to 'Mapping':
>| Status = mIoMmu->SetAttribute (mIoMmu, Mapping, 0);
>| Status = mIoMmu->Unmap (mIoMmu, Mapping);
will be done multiple times. I think they should only be done once and should
happen when freeing the aligned buffer.
Hence, I would suggest to add a new helper function called
IoMmuAllocateAlignedBuffer(), which will be similar to the existing
IoMmuAllocateBuffer() but with a couple of additional things to handle:
1. If the IOMMU PPI exists:
a. Allocate the buffer according to the real number of pages needed;
b. (New) Free the first & last unaligned pages;
c. Map the allocated aligned buffer;
d. Set the IOMMU attribute for the buffer.
2. If the IOMMU PPI does not exist:
a. Allocate the buffer according to the real number of pages needed;
b. (New) Free the first & last unaligned pages.
Also, could you help to separate the change for XhciPei into a separate patch?
It will be helpful to locate the exact commit made to XhciPei and XhciDxe in
the future.
Best Regards,
Hao Wu
From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
Sent: Tuesday, October 15, 2019 2:17 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
Were you able to validate the PEI change I sent last week?
Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Friday, October 11, 2019 12:07 PM
To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Patch set 2 has been submitted for review.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 11:53 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Ashish,
I think the IOMMU feature is added for the protection of FW integrity against
HW devices, which is the motivation to use IOMMU PPI for memory allocation.
The fix is easy for the DXE part since the IOMMU part has been integrated to the
implementation of the PciIo protocol.
As for the XhciPei case, my thought is that a new helper function (maybe called
'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece
between IoMmuAllocateAlignedBuffer() and existing IoMmuAllocateBuffer() will be:
1. If IOMMU PPI exists, mIoMmu->AllocateBuffer() should be called with bigger
number of pages for alignment and mIoMmu->FreeBuffer() can be called right after
to free the unused pages.
2. If IOMMU PPI does not exist, this case will fall back to a similar fix for
the DXE case.
Ideally, IoMmuFreeBuffer() can be reused to free the aligned buffer.
Please help to raise if you observe any open for the above proposal, thanks.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 11:30 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 57981 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
2019-10-15 1:21 ` Wu, Hao A
@ 2019-10-15 17:19 ` Ashish Singhal
0 siblings, 0 replies; 12+ messages in thread
From: Ashish Singhal @ 2019-10-15 17:19 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io, Ni, Ray
[-- Attachment #1: Type: text/plain, Size: 10307 bytes --]
Hello Hao,
Thanks for the clarification. I have made the changes and submitted a new patch. Also, in case the IOMMU PPI does not exist, we are now allocating the memory and adjust return pointers to be aligned. There is no freeing of unaligned pages here as PEI framework does not provide a mechanism for that.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Monday, October 14, 2019 7:21 PM
To: Ashish Singhal <ashishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello,
Sorry for the delayed response.
For the PEI changes, I found that when using the function IoMmuFreeBuffer() to
free the first/last unaligned page(s), the below operations related to 'Mapping':
>| Status = mIoMmu->SetAttribute (mIoMmu, Mapping, 0);
>| Status = mIoMmu->Unmap (mIoMmu, Mapping);
will be done multiple times. I think they should only be done once and should
happen when freeing the aligned buffer.
Hence, I would suggest to add a new helper function called
IoMmuAllocateAlignedBuffer(), which will be similar to the existing
IoMmuAllocateBuffer() but with a couple of additional things to handle:
1. If the IOMMU PPI exists:
a. Allocate the buffer according to the real number of pages needed;
b. (New) Free the first & last unaligned pages;
c. Map the allocated aligned buffer;
d. Set the IOMMU attribute for the buffer.
2. If the IOMMU PPI does not exist:
a. Allocate the buffer according to the real number of pages needed;
b. (New) Free the first & last unaligned pages.
Also, could you help to separate the change for XhciPei into a separate patch?
It will be helpful to locate the exact commit made to XhciPei and XhciDxe in
the future.
Best Regards,
Hao Wu
From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
Sent: Tuesday, October 15, 2019 2:17 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
Were you able to validate the PEI change I sent last week?
Thanks
Ashish
________________________________
From: Ashish Singhal <ashishsingha@nvidia.com>
Sent: Friday, October 11, 2019 12:07 PM
To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Patch set 2 has been submitted for review.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 11:53 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Ashish,
I think the IOMMU feature is added for the protection of FW integrity against
HW devices, which is the motivation to use IOMMU PPI for memory allocation.
The fix is easy for the DXE part since the IOMMU part has been integrated to the
implementation of the PciIo protocol.
As for the XhciPei case, my thought is that a new helper function (maybe called
'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece
between IoMmuAllocateAlignedBuffer() and existing IoMmuAllocateBuffer() will be:
1. If IOMMU PPI exists, mIoMmu->AllocateBuffer() should be called with bigger
number of pages for alignment and mIoMmu->FreeBuffer() can be called right after
to free the unused pages.
2. If IOMMU PPI does not exist, this case will fall back to a similar fix for
the DXE case.
Ideally, IoMmuFreeBuffer() can be reused to free the aligned buffer.
Please help to raise if you observe any open for the above proposal, thanks.
Best Regards,
Hao Wu
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 11:30 AM
To: Wu, Hao A; devel@edk2.groups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
What is your motivation towards using IoMMUPei for memory allocation now instead of Pei Services? The fix would be simple if we just change the number of pages needed to accommodate for the alignment. The call to free the pages would not do anything.
If we use IoMMUPei, we may need to change function definition for allocate to return how many pages were allocated for each call so that freeing can be done correctly as well. This may need more refactoring of the code that initially thought of.
Thanks
Ashish
From: Wu, Hao A <hao.a.wu@intel.com>
Sent: Thursday, October 10, 2019 9:00 PM
To: devel@edk2.groups.io; Ashish Singhal <ashishsingha@nvidia.com>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Thanks Ashish,
Please help to send out the fix for XhciPei in a separate patch as well.
I can help to see if verification can be done on my side.
Best Regards,
Hao Wu
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> [mailto:devel@edk2.groups.io] On Behalf Of Ashish Singhal
Sent: Friday, October 11, 2019 1:51 AM
To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
Hello Hao,
I agree that for completeness we should fix the issue in both DXE as well as PEI but on my target, we do not have any PEI phase for me to be able to verify any change that I will be making. If you still want, I can still make the change so that someone else can verify or we can have someone else fix PEI and push my change meanwhile to fix the issue in DXE.
Thanks
Ashish
________________________________
From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
> -----Original Message-----
> From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at that as well
> but I have no way to verify that as we are not using it. For the change I have
> made in DXE, I have verified it with an alignment of 4K and 64K.
Is it possible for you to verify the PEI case with a test PEI module that
performs read operation to a USB storage device? I think this will trigger
the affecting codes.
In my opinion, it would be better for the fix to be complete.
Best Regards,
Hao Wu
>
> Thanks
> Ashish
>
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com<mailto:hao.a.wu@intel.com>>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ashishsingha@nvidia.com<mailto:ashishsingha@nvidia.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Ni,
> Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation
>
> > -----Original Message-----
> > From: Ashish Singhal [mailto:ashishsingha@nvidia.com]
> > Sent: Thursday, October 10, 2019 1:02 AM
> > To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wu, Hao A; Ni, Ray
> > Cc: Ashish Singhal
> > Subject: [PATCH] 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<mailto: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
> > );
>
>
> Hello,
>
> The change looks good to me.
>
> Just a couple of things to confirm:
>
> 1. I think there is a similar case within the XhciPei driver Could you help to
> resolve it as well?
>
> I think for the PEI counterpart you may need to update both
> UsbHcAllocateAlignedPages() and UsbHcFreeAlignedPages(), since the
> IOMMU helper functions like IoMmuAllocateBuffer() and IoMmuFreeBuffer()
> might not be suitable now. Instead, I think services in the IoMmu PPI can be
> used.
>
> 2. Could you help to provide the information on what test has been done for
> the proposed patch?
>
> Thanks in advance.
>
> Best Regards,
> Hao Wu
>
>
> > --
> > 2.7.4
>
> -----------------------------------------------------------------------------------
> This email message is for the sole use of the intended recipient(s) and may
> contain
> confidential information. Any unauthorized review, use, disclosure or
> distribution
> is prohibited. If you are not the intended recipient, please contact the
> sender by
> reply email and destroy all copies of the original message.
> -----------------------------------------------------------------------------------
[-- Attachment #2: Type: text/html, Size: 35189 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-10-15 17:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-09 17:01 [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Ashish Singhal
2019-10-10 1:32 ` Wu, Hao A
2019-10-10 1:58 ` Ashish Singhal
2019-10-10 2:09 ` Wu, Hao A
2019-10-10 17:51 ` Ashish Singhal
2019-10-11 2:59 ` [edk2-devel] " Wu, Hao A
2019-10-11 3:30 ` Ashish Singhal
2019-10-11 5:53 ` Wu, Hao A
2019-10-11 18:07 ` Ashish Singhal
2019-10-14 18:17 ` Ashish Singhal
2019-10-15 1:21 ` Wu, Hao A
2019-10-15 17:19 ` Ashish Singhal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox