[v2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

[edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Chang, Abner via groups.io 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97228): https://edk2.groups.io/g/devel/message/97228
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Wu, Hao A 1 year, 4 months ago

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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97241): https://edk2.groups.io/g/devel/message/97241
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Chang, Abner via groups.io 1 year, 4 months ago

[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&amp;data=05%7C01%7
> Cabner.chang%40amd.com%7Ccac49c2820e741b8c48a08dadbf0cc24%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C638064124512265992%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=z1Q7NRxN4GMA%2
> FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97409): https://edk2.groups.io/g/devel/message/97409
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by He, Jiangang via groups.io 1 year, 4 months ago

[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&amp;data=05%7C01%7
> Cabner.chang%40amd.com%7Ccac49c2820e741b8c48a08dadbf0cc24%7C3dd8
> 961fe4884e608e11a82d994e183d%7C0%7C0%7C638064124512265992%7CUnk
> nown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1h
> aWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=z1Q7NRxN4GMA%2
> FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97582): https://edk2.groups.io/g/devel/message/97582
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Wu, Hao A 1 year, 4 months ago

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&amp;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&amp;sdata=z1Q7NRxN4GMA
> %2
> > FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97549): https://edk2.groups.io/g/devel/message/97549
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by He, Jiangang via groups.io 1 year, 4 months ago

[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&amp;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&amp;sdata=z1Q7NRxN4GMA
> %2
> > FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97592): https://edk2.groups.io/g/devel/message/97592
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Wu, Hao A 1 year, 4 months ago

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&amp;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&amp;sdata=z1Q7NRxN4GMA
> > %2
> > > FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#97577): https://edk2.groups.io/g/devel/message/97577
Mute This Topic: https://groups.io/mt/95582755/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH V2] MdeModulePkg/XhciPei: Unlinked XhciPei memory block

Posted by Chang, Abner via groups.io 1 year, 4 months ago

[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&amp;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&amp;sdata=z1Q7NRxN4GMA
> > > %2
> > > > FBxYd2D7Gnkc3aTD23mRnwNF3H5wE0k%3D&amp;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


-=-=-=-=-=-=-=-=-=-=-=-
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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-