* [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
@ 2022-12-10 15:12 Chang, Abner
2022-12-12 3:27 ` Wu, Hao A
0 siblings, 1 reply; 8+ messages in thread
From: Chang, Abner @ 2022-12-10 15:12 UTC (permalink / raw)
To: devel; +Cc: Kuei-Hung.Lin, Hao A Wu, Ray Ni, Garrett Kirkendall, Abner Chang
From: Abner Chang <abner.chang@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.Lin@amd.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Cc: Abner Chang <abner.chang@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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-10 15:12 [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block Chang, Abner
@ 2022-12-12 3:27 ` Wu, Hao A
2022-12-15 2:11 ` Chang, Abner
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Hao A @ 2022-12-12 3:27 UTC (permalink / raw)
To: abner.chang@amd.com, devel@edk2.groups.io
Cc: Kuei-Hung.Lin@amd.com, Ni, Ray, Zeng, Star, Sun, Zhikai,
Garrett Kirkendall
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://edk2.groups.io/g/devel/topic/92833071#92165? Thanks in advance.
Best Regards,
Hao Wu
> -----Original Message-----
> From: abner.chang@amd.com <abner.chang@amd.com>
> Sent: Saturday, December 10, 2022 11:13 PM
> To: devel@edk2.groups.io
> Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> Abner Chang <abner.chang@amd.com>
> Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
>
> From: Abner Chang <abner.chang@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.Lin@amd.com>
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-12 3:27 ` Wu, Hao A
@ 2022-12-15 2:11 ` Chang, Abner
2022-12-15 16:47 ` He, Jiangang
0 siblings, 1 reply; 8+ messages in thread
From: Chang, Abner @ 2022-12-15 2:11 UTC (permalink / raw)
To: Wu, Hao A, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett, He, Jiangang
[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.wu@intel.com>
> Sent: Monday, December 12, 2022 11:27 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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%2Fedk2.gr
> oups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7
> Cabner.chang%40amd.com%7Ccac49c2820e741b8c48a08dadbf0cc24%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C638064124512265992%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=z1Q7NRxN4GMA%2
> FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&reserved=0? Thanks in
> advance.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: abner.chang@amd.com <abner.chang@amd.com>
> > Sent: Saturday, December 10, 2022 11:13 PM
> > To: devel@edk2.groups.io
> > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> > Abner Chang <abner.chang@amd.com>
> > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory
> > block
> >
> > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-15 2:11 ` Chang, Abner
@ 2022-12-15 16:47 ` He, Jiangang
2022-12-19 6:40 ` Wu, Hao A
0 siblings, 1 reply; 8+ messages in thread
From: He, Jiangang @ 2022-12-15 16:47 UTC (permalink / raw)
To: Chang, Abner, Wu, Hao A, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett
[AMD Official Use Only - General]
Yes, it is the same issue discussed in https://edk2.groups.io/g/devel/topic/92833071#92165
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://edk2.groups.io/g/devel/topic/92833071#92165 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.Chang@amd.com>
Sent: Wednesday, December 14, 2022 8:12 PM
To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; He, Jiangang <Jiangang.He@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.wu@intel.com>
> Sent: Monday, December 12, 2022 11:27 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@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%2Fedk2
> .gr
> oups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7
> Cabner.chang%40amd.com%7Ccac49c2820e741b8c48a08dadbf0cc24%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C638064124512265992%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=z1Q7NRxN4GMA%2
> FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&reserved=0? Thanks in advance.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: abner.chang@amd.com <abner.chang@amd.com>
> > Sent: Saturday, December 10, 2022 11:13 PM
> > To: devel@edk2.groups.io
> > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > <ray.ni@intel.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> > Abner Chang <abner.chang@amd.com>
> > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory
> > block
> >
> > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-15 16:47 ` He, Jiangang
@ 2022-12-19 6:40 ` Wu, Hao A
2022-12-19 22:53 ` He, Jiangang
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Hao A @ 2022-12-19 6:40 UTC (permalink / raw)
To: He, Jiangang, Chang, Abner, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett
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://edk2.groups.io/g/devel/topic/92833071#92165, 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.He@amd.com>
> Sent: Friday, December 16, 2022 12:48 AM
> To: Chang, Abner <Abner.Chang@amd.com>; Wu, Hao A
> <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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://edk2.groups.io/g/devel/topic/92833071#92165
>
> 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://edk2.groups.io/g/devel/topic/92833071#92165 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.Chang@amd.com>
> Sent: Wednesday, December 14, 2022 8:12 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>;
> He, Jiangang <Jiangang.He@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.wu@intel.com>
> > Sent: Monday, December 12, 2022 11:27 AM
> > To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@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%2Fedk2
> > .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.chang@amd.com <abner.chang@amd.com>
> > > Sent: Saturday, December 10, 2022 11:13 PM
> > > To: devel@edk2.groups.io
> > > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>; Garrett Kirkendall <garrett.kirkendall@amd.com>;
> > > Abner Chang <abner.chang@amd.com>
> > > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory
> > > block
> > >
> > > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-19 6:40 ` Wu, Hao A
@ 2022-12-19 22:53 ` He, Jiangang
2022-12-20 0:26 ` Wu, Hao A
0 siblings, 1 reply; 8+ messages in thread
From: He, Jiangang @ 2022-12-19 22:53 UTC (permalink / raw)
To: Wu, Hao A, Chang, Abner, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett
[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.wu@intel.com>
Sent: Monday, December 19, 2022 12:40 AM
To: He, Jiangang <Jiangang.He@amd.com>; Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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%2Fedk2.groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126779782%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=uJMNyYXMU22UZNjrKtDetyD2BqrvkualuorPZHAV3wg%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.He@amd.com>
> Sent: Friday, December 16, 2022 12:48 AM
> To: Chang, Abner <Abner.Chang@amd.com>; Wu, Hao A
> <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@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%2Fedk2
> .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7CJiang
> ang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3dd8961fe4884e60
> 8e11a82d994e183d%7C0%7C0%7C638070288126936018%7CUnknown%7CTWFpbGZsb3d8
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3
> 000%7C%7C%7C&sdata=58j41QJxKbrQenhyZKYO4dxYj3Sat2kJejQGioZhtu4%3D&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%2Fedk2.groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126936018%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=58j41QJxKbrQenhyZKYO4dxYj3Sat2kJejQGioZhtu4%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.Chang@amd.com>
> Sent: Wednesday, December 14, 2022 8:12 PM
> To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett
> <Garrett.Kirkendall@amd.com>; He, Jiangang <Jiangang.He@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.wu@intel.com>
> > Sent: Monday, December 12, 2022 11:27 AM
> > To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@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%2Fedk2
> > .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.chang@amd.com <abner.chang@amd.com>
> > > Sent: Saturday, December 10, 2022 11:13 PM
> > > To: devel@edk2.groups.io
> > > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> > > <ray.ni@intel.com>; Garrett Kirkendall
> > > <garrett.kirkendall@amd.com>; Abner Chang <abner.chang@amd.com>
> > > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory
> > > block
> > >
> > > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > Cc: Ray Ni <ray.ni@intel.com>
> > > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-19 22:53 ` He, Jiangang
@ 2022-12-20 0:26 ` Wu, Hao A
2022-12-20 0:27 ` Chang, Abner
0 siblings, 1 reply; 8+ messages in thread
From: Wu, Hao A @ 2022-12-20 0:26 UTC (permalink / raw)
To: He, Jiangang, Chang, Abner, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett
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.He@amd.com>
> Sent: Tuesday, December 20, 2022 6:54 AM
> To: Wu, Hao A <hao.a.wu@intel.com>; Chang, Abner
> <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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.wu@intel.com>
> Sent: Monday, December 19, 2022 12:40 AM
> To: He, Jiangang <Jiangang.He@amd.com>; Chang, Abner
> <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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%2Fedk2
> .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7
> CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3d
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126779782%7C
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> 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.He@amd.com>
> > Sent: Friday, December 16, 2022 12:48 AM
> > To: Chang, Abner <Abner.Chang@amd.com>; Wu, Hao A
> > <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@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%2Fedk2
> > .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%2Fedk2
> .groups.io%2Fg%2Fdevel%2Ftopic%2F92833071%2392165&data=05%7C01%7
> CJiangang.He%40amd.com%7C528424aa303f4a2c9b1808dae18be036%7C3d
> d8961fe4884e608e11a82d994e183d%7C0%7C0%7C638070288126936018%7C
> Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBT
> 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.Chang@amd.com>
> > Sent: Wednesday, December 14, 2022 8:12 PM
> > To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@amd.com>; He, Jiangang <Jiangang.He@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.wu@intel.com>
> > > Sent: Monday, December 12, 2022 11:27 AM
> > > To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> > > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > > <Garrett.Kirkendall@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%2Fedk2
> > > .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.chang@amd.com <abner.chang@amd.com>
> > > > Sent: Saturday, December 10, 2022 11:13 PM
> > > > To: devel@edk2.groups.io
> > > > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni,
> Ray
> > > > <ray.ni@intel.com>; Garrett Kirkendall
> > > > <garrett.kirkendall@amd.com>; Abner Chang <abner.chang@amd.com>
> > > > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory
> > > > block
> > > >
> > > > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > > > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block
2022-12-20 0:26 ` Wu, Hao A
@ 2022-12-20 0:27 ` Chang, Abner
0 siblings, 0 replies; 8+ messages in thread
From: Chang, Abner @ 2022-12-20 0:27 UTC (permalink / raw)
To: Wu, Hao A, He, Jiangang, devel@edk2.groups.io
Cc: Lin, Kuei-Hung (Timothy), Ni, Ray, Zeng, Star, Sun, Zhikai,
Kirkendall, Garrett
[AMD Official Use Only - General]
I will send the update according to the discussion.
Thanks
Abner
> -----Original Message-----
> From: Wu, Hao A <hao.a.wu@intel.com>
> Sent: Tuesday, December 20, 2022 8:27 AM
> To: He, Jiangang <Jiangang.He@amd.com>; Chang, Abner
> <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> <zhikai.sun@intel.com>; Kirkendall, Garrett <Garrett.Kirkendall@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.He@amd.com>
> > Sent: Tuesday, December 20, 2022 6:54 AM
> > To: Wu, Hao A <hao.a.wu@intel.com>; Chang, Abner
> > <Abner.Chang@amd.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@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.wu@intel.com>
> > Sent: Monday, December 19, 2022 12:40 AM
> > To: He, Jiangang <Jiangang.He@amd.com>; Chang, Abner
> > <Abner.Chang@amd.com>; devel@edk2.groups.io
> > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > <Garrett.Kirkendall@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.He@amd.com>
> > > Sent: Friday, December 16, 2022 12:48 AM
> > > To: Chang, Abner <Abner.Chang@amd.com>; Wu, Hao A
> > > <hao.a.wu@intel.com>; devel@edk2.groups.io
> > > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > > <Garrett.Kirkendall@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.Chang@amd.com>
> > > Sent: Wednesday, December 14, 2022 8:12 PM
> > > To: Wu, Hao A <hao.a.wu@intel.com>; devel@edk2.groups.io
> > > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > > <Garrett.Kirkendall@amd.com>; He, Jiangang <Jiangang.He@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.wu@intel.com>
> > > > Sent: Monday, December 12, 2022 11:27 AM
> > > > To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> > > > Cc: Lin, Kuei-Hung (Timothy) <Kuei-Hung.Lin@amd.com>; Ni, Ray
> > > > <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Sun, Zhikai
> > > > <zhikai.sun@intel.com>; Kirkendall, Garrett
> > > > <Garrett.Kirkendall@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.chang@amd.com <abner.chang@amd.com>
> > > > > Sent: Saturday, December 10, 2022 11:13 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Kuei-Hung.Lin@amd.com; Wu, Hao A <hao.a.wu@intel.com>; Ni,
> > Ray
> > > > > <ray.ni@intel.com>; Garrett Kirkendall
> > > > > <garrett.kirkendall@amd.com>; Abner Chang
> <abner.chang@amd.com>
> > > > > Subject: [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei
> > > > > memory block
> > > > >
> > > > > From: Abner Chang <abner.chang@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.Lin@amd.com>
> > > > > Cc: Hao A Wu <hao.a.wu@intel.com>
> > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
> > > > > Cc: Abner Chang <abner.chang@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
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-12-20 0:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-10 15:12 [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block Chang, Abner
2022-12-12 3:27 ` Wu, Hao A
2022-12-15 2:11 ` Chang, Abner
2022-12-15 16:47 ` He, Jiangang
2022-12-19 6:40 ` Wu, Hao A
2022-12-19 22:53 ` He, Jiangang
2022-12-20 0:26 ` Wu, Hao A
2022-12-20 0:27 ` Chang, Abner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox