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 Singh= al <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 fo= r 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 h= elper function (maybe called

'IoMmuAllocateAlignedBuffer') can be added in DmaMem.c file. The main differece

between IoMmuAllocateAlignedBuffer() and existin= g IoMmuAllocateBuffer() will be:

1. If IOMMU PPI ex= ists, mIoMmu->Al= locateBuffer() should be called with bigger

number of pages fo= r alignment and mIoMmu->Fr= eeBuffer() can be called right after

to free the unused= pages.

2. If IOMMU PPI do= es 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 rai= se if you observe any open for the above proposal, thanks.

Best Regards,

Hao Wu

From: d= evel@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 u= sing IoMMUPei for memory allocation now inste= ad 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 c= all so that freeing can be done correctly as well. This may need more refac= toring 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.co= m>; Ni, Ray <ray.ni@intel.com>
Subject: RE: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation

Thanks Ashish,

Please help to sen= d 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.g= roups.io; Ni, Ray
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation

Hello Hao,

I agree that for completeness we shoul= d 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 cha= nge meanwhile to fix the issue in DXE.

Thanks

Ashish

From:<= span style=3D"font-size:11.0pt; font-family:"Calibri","sans-= serif"; color:black"> Wu, Hao A <hao.a.wu@intel.com>
Sent: Wednesday, October 9, 2019 8:09 PM
To: Ashish Singhal <a= shishsingha@nvidia.com>; devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray &l= t;ray.ni@intel.com>
Subject: RE: [PATCH] MdeModulePkg/XhciDx= e: Fix Aligned Page Allocation

> -----Orig= inal Message-----
> From: Ashish Singhal [mail= to:ashishsingha@nvidia.com]
> Sent: Thursday, October 10, 2019 9:58 AM
> To: Wu, Hao A; devel@edk2.gro= ups.io; Ni, Ray
> Subject: RE: [PATCH] MdeModulePkg/XhciDxe<= /span>: Fix Aligned Page Allocation
>
> Hello Hao,
>
> I can see that the PEI also has the same issue and take a look at tha= t as well
> but I have no way to verify that as we are not using it. For the chan= ge 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<= br> 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@in= tel.com>
> Sent: Wednesday, October 9, 2019 7:33 PM
> To: Ashish Singhal <ash= ishsingha@nvidia.com>; devel@edk2.groups.io; Ni,
> Ray <ray.ni@intel.com><= br> > Subject: RE: [PATCH] MdeModulePkg/XhciDxe<= /span>: 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 f= or
> > that alignment. The calls to free pages takes care of this alrea= dy.
> >
> > 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/Pc= i/XhciDxe/UsbHcMem.c
> > @@ -656,7 +656,7 @@ UsbHcAllocate= AlignedPages (
> >           = ;             <= span class=3D"x_x_SpellE">PciIo,
> >           = ;             <= span class=3D"x_x_SpellE">AllocateAnyPages,
> >           = ;             <= span class=3D"x_x_SpellE">EfiBootServicesData,
> > -          &nb= sp;           Pages,
> > +          = ;            RealPages,
> >           = ;             &= amp;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 IoMmuAllocateB= uffer() 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 do= ne 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) a= nd may
> contain
> confidential information. Any unauthorized review, use, disclos= ure or
> distribution
> is prohibited. If you are not the intended recipient, please co= ntact the
> sender by
> reply email and destroy all copies of the original message.
> ---------------------------------------------------------------------= --------------