[AMD Official Use Only - General] I will send the update according to the discussion.
Thanks Abner > -----Original Message----- > From: Wu, Hao A <hao.a...@intel.com> > Sent: Tuesday, December 20, 2022 8:27 AM > To: He, Jiangang <jiangang...@amd.com>; Chang, Abner > <abner.ch...@amd.com>; devel@edk2.groups.io > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > <zhikai....@intel.com>; Kirkendall, Garrett <garrett.kirkend...@amd.com> > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory > block > > [AMD Official Use Only - General] > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Thanks. > You are right. I agree with you that aligning with XhciDxe is a better > resolution. > > Best Regards, > Hao Wu > > > -----Original Message----- > > From: He, Jiangang <jiangang...@amd.com> > > Sent: Tuesday, December 20, 2022 6:54 AM > > To: Wu, Hao A <hao.a...@intel.com>; Chang, Abner > > <abner.ch...@amd.com>; devel@edk2.groups.io > > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > > <zhikai....@intel.com>; Kirkendall, Garrett > > <garrett.kirkend...@amd.com> > > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > memory > > block > > > > [AMD Official Use Only - General] > > > > UsbHcFreeMemBlock()->IoMmuFreeBuffer()->mIoMmu->FreeBuffer(), > which > > may end up calling PeiFreePages() depending on gEdkiiIoMmuPpiGuid > > implementation. Surely both will work since UsbHcFreeMemPool() can't > > be interrupted by any service call to use the memory just freed. Just > > for good coding practice reason, I pick the one aligning with XhciDxe. > > > > Thanks, > > Jiangang > > -----Original Message----- > > From: Wu, Hao A <hao.a...@intel.com> > > Sent: Monday, December 19, 2022 12:40 AM > > To: He, Jiangang <jiangang...@amd.com>; Chang, Abner > > <abner.ch...@amd.com>; devel@edk2.groups.io > > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > > <zhikai....@intel.com>; Kirkendall, Garrett > > <garrett.kirkend...@amd.com> > > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > memory > > block > > > > [AMD Official Use Only - General] > > > > Caution: This message originated from an External Source. Use proper > > caution when opening attachments, clicking links, or responding. > > > > > > Hello, > > > > My take is that unlike in DXE, the UsbHcFreeMemBlock() implementation > > in PEI phase does not perform freeing the memory. > > > > So I think both the solution: > > * Provided at > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk > 2 > > .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01% > 7 > > CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3d > > d8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126779782%7C > > > Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > T > > > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=uJMNyYXMU22 > > UZNjrKtDetyD2BqrvkualuorPZHAV3wg%3D&reserved=0, which aligns with > > EhciPei > > * Mentioned at the end of your previous reply, which aligns with > > XhciDxe should work fine. > > > > I will leave it to you for the final decision. > > > > Best Regards, > > Hao Wu > > > > > -----Original Message----- > > > From: He, Jiangang <jiangang...@amd.com> > > > Sent: Friday, December 16, 2022 12:48 AM > > > To: Chang, Abner <abner.ch...@amd.com>; Wu, Hao A > > > <hao.a...@intel.com>; devel@edk2.groups.io > > > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > > > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > > > <zhikai....@intel.com>; Kirkendall, Garrett > > > <garrett.kirkend...@amd.com> > > > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > > > memory block > > > > > > [AMD Official Use Only - General] > > > > > > Yes, it is the same issue discussed in > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk > 2 > > > .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01 > > %7CJiang > > > > > ang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3dd8961f > > e4884e60 > > > > > 8e11a82d994e183d%7C0%7C0%7C638070288126936018%7CUnknown%7CT > > WFpbGZsb3d8 > > > > > > eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3 > > D%7C3 > > > > > > 000%7C%7C%7C&sdata=58j41QJxKbrQenhyZKYO4dxYj3Sat2kJejQGioZhtu4% > 3 > > D&rese > > > rved=0 > > > > > > MdeModulePkg\Bus\Pci\XhciPei\UsbHcMem.c > > > > > > for (Block = Pool->Head->Next; Block != NULL; Block = Pool->Head- > >Next) { > > > // UsbHcUnlinkMemBlock (Pool->Head, Block); > > > UsbHcFreeMemBlock (Pool, Block); > > > } > > > Block = Pool->Head->Next never change without calling > > > UsbHcUnlinkMemBlock (Pool->Head, Block), therefore dead loop. > > > > > > Our proposed fix came from dxe version of the equivalent file > > > MdeModulePkg\Bus\Pci\XhciDxe\UsbHcMem.c but swapped two > routine > > call > > > order (Now I think it is incorrect as clarified below). > > > for (Block = Pool->Head->Next; Block != NULL; Block = Pool->Head- > >Next) { > > > UsbHcFreeMemBlock (Pool, Block); > > > UsbHcUnlinkMemBlock (Pool->Head, Block); > > > } > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk > 2 > > .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01% > 7 > > CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3d > > d8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126936018%7C > > > Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJB > T > > > iI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=58j41QJxKbrQen > > hyZKYO4dxYj3Sat2kJejQGioZhtu4%3D&reserved=0 proposed fix: > > > > > > for (Block = Pool->Head->Next; Block != NULL; Block = Block ->Next) { > > > // UsbHcUnlinkMemBlock (Pool->Head, Block); > > > UsbHcFreeMemBlock (Pool, Block); > > > } > > > > > > I think it again, both proposals have problem of reading memory > > > content in the buffer that has just been freed. > > > > > > for (Block = Pool->Head->Next; Block != NULL; Block = Pool->Head- > >Next) { > > > UsbHcUnlinkMemBlock (Pool->Head, Block); > > > UsbHcFreeMemBlock (Pool, Block); > > > } > > > is right solution and matches dxe version of UsbHcMem.c. > > > > > > Thanks, > > > Jiangang > > > > > > -----Original Message----- > > > From: Chang, Abner <abner.ch...@amd.com> > > > Sent: Wednesday, December 14, 2022 8:12 PM > > > To: Wu, Hao A <hao.a...@intel.com>; devel@edk2.groups.io > > > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > > > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > > > <zhikai....@intel.com>; Kirkendall, Garrett > > > <garrett.kirkend...@amd.com>; He, Jiangang <jiangang...@amd.com> > > > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > > > memory block > > > > > > [AMD Official Use Only - General] > > > > > > Hi Jiangang, > > > Could you please provide the context of this patch? > > > > > > Thanks > > > Abner > > > > > > > -----Original Message----- > > > > From: Wu, Hao A <hao.a...@intel.com> > > > > Sent: Monday, December 12, 2022 11:27 AM > > > > To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io > > > > Cc: Lin, Kuei-Hung (Timothy) <kuei-hung....@amd.com>; Ni, Ray > > > > <ray...@intel.com>; Zeng, Star <star.z...@intel.com>; Sun, Zhikai > > > > <zhikai....@intel.com>; Kirkendall, Garrett > > > > <garrett.kirkend...@amd.com> > > > > Subject: RE: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > > > > memory block > > > > > > > > Caution: This message originated from an External Source. Use > > > > proper caution when opening attachments, clicking links, or responding. > > > > > > > > > > > > Sorry for a question, may I know what issue was met that leads to > > > > the proposed patch? > > > > Could you help to check if it is related with the topic discussed > > > > in > > > > > > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk > 2 > > > > .gr > > > > > > > > oups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C0 > > > 1%7 > > > > > > > > > Cabner.chang%40amd.com%7Ccac49c2820e741b8c48a08dadbf0cc24%7C3d > > > d8 > > > > > > > 961fe4884e608e11a82d994e183d%7C0%7C0%7C638064124512265992%7C > > > Unk > > > > > > > > > > nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6I > > > k1h > > > > > > > > > > aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=z1Q7NRxN4GMA > > > %2 > > > > FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&reserved=0? Thanks > > in > > > advance. > > > > > > > > Best Regards, > > > > Hao Wu > > > > > > > > > -----Original Message----- > > > > > From: abner.ch...@amd.com <abner.ch...@amd.com> > > > > > Sent: Saturday, December 10, 2022 11:13 PM > > > > > To: devel@edk2.groups.io > > > > > Cc: kuei-hung....@amd.com; Wu, Hao A <hao.a...@intel.com>; Ni, > > Ray > > > > > <ray...@intel.com>; Garrett Kirkendall > > > > > <garrett.kirkend...@amd.com>; Abner Chang > <abner.ch...@amd.com> > > > > > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei > > > > > memory block > > > > > > > > > > From: Abner Chang <abner.ch...@amd.com> > > > > > > > > > > In V2: Add AMD copyright. > > > > > > > > > > Unlink the XhciPei memory block when it has been freed. > > > > > > > > > > Signed-off-by: Kuei-Hung Lin <kuei-hung....@amd.com> > > > > > Cc: Hao A Wu <hao.a...@intel.com> > > > > > Cc: Ray Ni <ray...@intel.com> > > > > > Cc: Garrett Kirkendall <garrett.kirkend...@amd.com> > > > > > Cc: Abner Chang <abner.ch...@amd.com> > > > > > --- > > > > > MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c | 29 > > > > > ++++++++++++++++++++++++- > > > > > 1 file changed, 28 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > > > > b/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > > > > index c64b38fcfc8..39ba31b0913 100644 > > > > > --- a/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > > > > +++ b/MdeModulePkg/Bus/Pci/XhciPei/UsbHcMem.c > > > > > @@ -3,6 +3,7 @@ PEIM to produce gPeiUsb2HostControllerPpiGuid > > > based > > > > > on gPeiUsbControllerPpiGuid which is used to enable recovery > > > > > function from USB Drivers. > > > > > > > > > > Copyright (c) 2014 - 2016, Intel Corporation. All rights > > > > > reserved.<BR> > > > > > +Copyright (C) 2022 Advanced Micro Devices, Inc. All rights > > > > > +reserved.<BR> > > > > > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > > > > > @@ -365,6 +366,32 @@ UsbHcInitMemPool ( > > > > > return Pool; > > > > > } > > > > > > > > > > +/** > > > > > + Unlink the memory block from the pool's list. > > > > > + > > > > > + @param Head The block list head of the memory's pool. > > > > > + @param BlockToUnlink The memory block to unlink. > > > > > + > > > > > +**/ > > > > > +VOID > > > > > +UsbHcUnlinkMemBlock ( > > > > > + IN USBHC_MEM_BLOCK *Head, > > > > > + IN USBHC_MEM_BLOCK *BlockToUnlink > > > > > + ) > > > > > +{ > > > > > + USBHC_MEM_BLOCK *Block; > > > > > + > > > > > + ASSERT ((Head != NULL) && (BlockToUnlink != NULL)); > > > > > + > > > > > + for (Block = Head; Block != NULL; Block = Block->Next) { > > > > > + if (Block->Next == BlockToUnlink) { > > > > > + Block->Next = BlockToUnlink->Next; > > > > > + BlockToUnlink->Next = NULL; > > > > > + break; > > > > > + } > > > > > + } > > > > > +} > > > > > + > > > > > /** > > > > > Release the memory management pool. > > > > > > > > > > @@ -386,8 +413,8 @@ UsbHcFreeMemPool ( > > > > > // first block. > > > > > // > > > > > for (Block = Pool->Head->Next; Block != NULL; Block = > > > > > Pool->Head->Next) { > > > > > - // UsbHcUnlinkMemBlock (Pool->Head, Block); > > > > > UsbHcFreeMemBlock (Pool, Block); > > > > > + UsbHcUnlinkMemBlock (Pool->Head, Block); > > > > > } > > > > > > > > > > UsbHcFreeMemBlock (Pool, Pool->Head); > > > > > -- > > > > > 2.37.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#97578): https://edk2.groups.io/g/devel/message/97578 Mute This Topic: https://groups.io/mt/95582755/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
