From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by mx.groups.io with SMTP id smtpd.web10.5489.1570762799904690022 for ; Thu, 10 Oct 2019 20:00:00 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 134.134.136.65, mailfrom: hao.a.wu@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Oct 2019 19:59:59 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,282,1566889200"; d="scan'208,217";a="197449671" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga003.jf.intel.com with ESMTP; 10 Oct 2019 19:59:58 -0700 Received: from fmsmsx607.amr.corp.intel.com (10.18.126.87) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 10 Oct 2019 19:59:57 -0700 Received: from fmsmsx607.amr.corp.intel.com (10.18.126.87) by fmsmsx607.amr.corp.intel.com (10.18.126.87) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 10 Oct 2019 19:59:57 -0700 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by fmsmsx607.amr.corp.intel.com (10.18.126.87) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 10 Oct 2019 19:59:57 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.166]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.225]) with mapi id 14.03.0439.000; Fri, 11 Oct 2019 10:59:55 +0800 From: "Wu, Hao A" To: "devel@edk2.groups.io" , "ashishsingha@nvidia.com" , "Ni, Ray" Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Thread-Topic: [edk2-devel] [PATCH] MdeModulePkg/XhciDxe: Fix Aligned Page Allocation Thread-Index: AQHVfsNZ4DcOEA/aqEOUTbXlESnb2adTE2xQ//+FUgCAAIfmQIAAgk8AgAEfKnA= Date: Fri, 11 Oct 2019 02:59:55 +0000 Message-ID: References: <9ce268553db91fbe7fb13e2205d0e1611e1d0212.1570640221.git.ashishsingha@nvidia.com> , In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: hao.a.wu@intel.com Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_B80AF82E9BFB8E4FBD8C89DA810C6A093C942719SHSMSX104ccrcor_" --_000_B80AF82E9BFB8E4FBD8C89DA810C6A093C942719SHSMSX104ccrcor_ Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 Ashi= sh 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 A= llocation 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 mak= e the change so that someone else can verify or we can have someone else fi= x PEI and push my change meanwhile to fix the issue in DXE. Thanks Ashish ________________________________ From: Wu, Hao A Sent: Wednesday, October 9, 2019 8:09 PM To: Ashish Singhal ; devel@edk2.groups.io ; Ni, Ray 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 a= s 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 > Sent: Wednesday, October 9, 2019 7:33 PM > To: Ashish Singhal ; devel@edk2.groups.io; Ni, > Ray > 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 > > --- > > 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 h= elp 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 ca= n 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 th= e > sender by > reply email and destroy all copies of the original message. > ------------------------------------------------------------------------= ----------- --_000_B80AF82E9BFB8E4FBD8C89DA810C6A093C942719SHSMSX104ccrcor_ Content-Type: text/html; charset="us-ascii" Content-Transfer-Encoding: quoted-printable

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 m= y 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.grou= ps.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 [mail= to: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 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@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 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/Pci/XhciDxe/UsbHcMem.= c
> > @@ -656,7 +656,7 @@ UsbHcAllocateAlig= nedPages (
> >           = ;             <= span class=3D"SpellE">PciIo,
> >           = ;             <= span class=3D"SpellE">AllocateAnyPages,
> >           = ;             <= span class=3D"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 IoMmuAllocateBuffe= r() 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.
> ---------------------------------------------------------------------= --------------

--_000_B80AF82E9BFB8E4FBD8C89DA810C6A093C942719SHSMSX104ccrcor_--