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.
> -----------------------------------------------------------------------------------