Hello Ashish,=

I think the IOMMU fe= ature 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 th= e PciIo protocol.

As for the XhciPei case, my thought is that a new hel= per 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 exis= ts, mIoMmu->Alloca= teBuffer() should be called with bigger

number of pages for = alignment and mIoMmu->FreeBu= ffer() can be called right after

to free the unused p= ages.

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 th= e aligned buffer.

Please help to raise= if you observe any open for the above proposal, thanks.

Best Regards,=

Hao Wu

From: dev= el@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 usi= ng IoMMUPei for memory allocation now instead= of Pei Services? The fix would be simple if we just change the number of p= ages needed to accommodate for the alignment. The call to free the pages wo= uld not do anything.

If we use IoMMUPei, we may need to change function d= efinition for allocate to return how many pages were allocated for each cal= l so that freeing can be done correctly as well. This may need more refacto= ring of the code that initially thought of.

Thanks

Ashish

From: W= u, 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 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.g= roups.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 a= ny 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: 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/XhciDxe<= /span>: Fix Aligned Page Allocation

> -----Origin= al 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: 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: 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/<= span class=3D"x_SpellE">UsbHcMem.c
> > b/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c > > index fd79988..aa69c47 100644
> > --- a/MdeModulePkg/Bus/Pci/XhciDxe/UsbHcMem.c
> > +++ b/MdeModulePkg/Bus/Pci<= /span>/XhciDxe/Usb= HcMem.c
> > @@ -656,7 +656,7 @@ UsbHcAllocateAl= ignedPages (
> >           = ;             <= span class=3D"x_SpellE">PciIo,
> >           = ;             <= span class=3D"x_SpellE">AllocateAnyPages,
> >           = ;             <= span class=3D"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 IoMmuAllocateBuf= fer() 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.
> ---------------------------------------------------------------------= --------------