[v3] Optimize load uCode performance

[edk2] [Patch v3 0/3] Optimize load uCode performance

Posted by Eric Dong 5 years, 9 months ago

Use below three rules to optimize load uCode performance:
1. Let BSP relocate uCode from flash to memory for better performance.
2. BSP caches the CPU ID and address of uCode so AP doesn\ufffdt need to look 
   for the uCode again if the CPU ID is same as BSP\ufffds.
3. Only apply uCode in one thread of a core when hyper threading is enabled.

v2 changes:
  Fix potential issue if allocate memory failed.

V3 Changes:
  Remove the ASSERT code which is not correct.

Test:
Use an sample platform which has 1 socket, 4 core, 8 threads, the 
CpuMpPei driver cost time reduce from 108.4ms to 27.2ms


Eric Dong (3):
  UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
  UefiCpuPkg/MpInitLib: Load uCode once for each core.
  UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.

 UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 +++++++++++++++++++++++++++++---
 UefiCpuPkg/Library/MpInitLib/MpLib.c     | 38 +++++++++++++++++++++++++---
 UefiCpuPkg/Library/MpInitLib/MpLib.h     | 11 ++++++--
 3 files changed, 84 insertions(+), 8 deletions(-)

-- 
2.15.0.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

Posted by Laszlo Ersek 5 years, 9 months ago

Hi Eric,

On 07/16/18 05:08, Eric Dong wrote:
> Use below three rules to optimize load uCode performance:
> 1. Let BSP relocate uCode from flash to memory for better performance.
> 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
>    for the uCode again if the CPU ID is same as BSP’s.
> 3. Only apply uCode in one thread of a core when hyper threading is enabled.
> 
> v2 changes:
>   Fix potential issue if allocate memory failed.
> 
> V3 Changes:
>   Remove the ASSERT code which is not correct.

Based on the above, my understanding is that you didn't modify patches
#2 and #3, from v2 to v3. Is that correct?

If it's correct, then you should have picked up my Acked-by tags from
the v2 review, for the v3 posting:

http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com

http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com

If you don't pick up my previous review tags for un-modified patches in
new postings of the patch series, then I have to re-review those patches
every single time. I described this workflow element here:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-26

----------
    26. § More frequently though, you will get requests for changes for
        *some* of your patches, while *others* of your patches will be
        fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
        you need to do in this case is:

        * create the next version of your local branch
        * pick up the tags that you got on the list
        * implement the requested changes
        * mark the v2 changes on each patch outside of the commit
          message
        * push the next version to your personal repo again
        * post the next version to the list

        In the following steps, we'll go through each of these in more
        detail.
----------

Thanks
Laszlo

> Test:
> Use an sample platform which has 1 socket, 4 core, 8 threads, the 
> CpuMpPei driver cost time reduce from 108.4ms to 27.2ms
> 
> 
> Eric Dong (3):
>   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
>   UefiCpuPkg/MpInitLib: Load uCode once for each core.
>   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
> 
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43 +++++++++++++++++++++++++++++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c     | 38 +++++++++++++++++++++++++---
>  UefiCpuPkg/Library/MpInitLib/MpLib.h     | 11 ++++++--
>  3 files changed, 84 insertions(+), 8 deletions(-)
> 
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

Posted by Laszlo Ersek 5 years, 9 months ago

On 07/17/18 18:38, Laszlo Ersek wrote:
> Hi Eric,
> 
> On 07/16/18 05:08, Eric Dong wrote:
>> Use below three rules to optimize load uCode performance:
>> 1. Let BSP relocate uCode from flash to memory for better performance.
>> 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look 
>>    for the uCode again if the CPU ID is same as BSP’s.
>> 3. Only apply uCode in one thread of a core when hyper threading is enabled.
>>
>> v2 changes:
>>   Fix potential issue if allocate memory failed.
>>
>> V3 Changes:
>>   Remove the ASSERT code which is not correct.
> 
> Based on the above, my understanding is that you didn't modify patches
> #2 and #3, from v2 to v3. Is that correct?

I compared the v2 and v3 patches pair-wise; indeed the ASSERT()'s
removal in patch #1 is the only difference.

> If it's correct, then you should have picked up my Acked-by tags from
> the v2 review, for the v3 posting:
> 
> http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-d5d47a0988aa@redhat.com
> 
> http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-1b2edc1f4aef@redhat.com
> 
> If you don't pick up my previous review tags for un-modified patches in
> new postings of the patch series, then I have to re-review those patches
> every single time. I described this workflow element here:
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-26
> 
> ----------
>     26. § More frequently though, you will get requests for changes for
>         *some* of your patches, while *others* of your patches will be
>         fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
>         you need to do in this case is:
> 
>         * create the next version of your local branch
>         * pick up the tags that you got on the list
>         * implement the requested changes
>         * mark the v2 changes on each patch outside of the commit
>           message
>         * push the next version to your personal repo again
>         * post the next version to the list
> 
>         In the following steps, we'll go through each of these in more
>         detail.
> ----------

* For patch #1:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

* For patches #2 and #3:

Acked-by: Laszlo Ersek <lersek@redhat.com>
Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch v3 0/3] Optimize load uCode performance

Posted by Dong, Eric 5 years, 9 months ago

Hi Laszlo,

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, July 18, 2018 12:38 AM
> To: Dong, Eric <eric.dong@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [Patch v3 0/3] Optimize load uCode performance
> 
> Hi Eric,
> 
> On 07/16/18 05:08, Eric Dong wrote:
> > Use below three rules to optimize load uCode performance:
> > 1. Let BSP relocate uCode from flash to memory for better performance.
> > 2. BSP caches the CPU ID and address of uCode so AP doesn’t need to look
> >    for the uCode again if the CPU ID is same as BSP’s.
> > 3. Only apply uCode in one thread of a core when hyper threading is
> enabled.
> >
> > v2 changes:
> >   Fix potential issue if allocate memory failed.
> >
> > V3 Changes:
> >   Remove the ASSERT code which is not correct.
> 
> Based on the above, my understanding is that you didn't modify patches
> #2 and #3, from v2 to v3. Is that correct?
> 
> If it's correct, then you should have picked up my Acked-by tags from the v2
> review, for the v3 posting:
> 
> http://mid.mail-archive.com/dcf4df85-1d35-65e9-2c9b-
> d5d47a0988aa@redhat.com
> 
> http://mid.mail-archive.com/dbc8439f-448e-306c-cdbd-
> 1b2edc1f4aef@redhat.com
> 
> If you don't pick up my previous review tags for un-modified patches in new
> postings of the patch series, then I have to re-review those patches every
> single time. I described this workflow element here:
> 

Got it, will follow this rule later. Thanks.

> https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-
> guide-for-edk2-contributors-and-maintainers#contrib-26
> 
> ----------
>     26. § More frequently though, you will get requests for changes for
>         *some* of your patches, while *others* of your patches will be
>         fine, and garner Reviewed-by, Acked-by, and Tested-by tags. What
>         you need to do in this case is:
> 
>         * create the next version of your local branch
>         * pick up the tags that you got on the list
>         * implement the requested changes
>         * mark the v2 changes on each patch outside of the commit
>           message
>         * push the next version to your personal repo again
>         * post the next version to the list
> 
>         In the following steps, we'll go through each of these in more
>         detail.
> ----------
> 
> Thanks
> Laszlo
> 
> > Test:
> > Use an sample platform which has 1 socket, 4 core, 8 threads, the
> > CpuMpPei driver cost time reduce from 108.4ms to 27.2ms
> >
> >
> > Eric Dong (3):
> >   UefiCpuPkg/MpInitLib: Use BSP uCode for APs if possible.
> >   UefiCpuPkg/MpInitLib: Load uCode once for each core.
> >   UefiCpuPkg/MpInitLib: Relocate uCode to memory to save time.
> >
> >  UefiCpuPkg/Library/MpInitLib/Microcode.c | 43
> +++++++++++++++++++++++++++++---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c     | 38
> +++++++++++++++++++++++++---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h     | 11 ++++++--
> >  3 files changed, 84 insertions(+), 8 deletions(-)
> >
> >
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel