Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
add an MpServicesTest application to exercise it.

Changes from v3:

Removed Ard's 'Reviewed-by' line from the commits since the code has changed
sufficiently that it's no longer valid.

Rebecca Cran (3):
  ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
  ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
  MdeModulePkg: Add new Application/MpServicesTest application

 ArmPkg/ArmPkg.dsc                                            |    1 +
 MdeModulePkg/MdeModulePkg.dsc                                |    2 +
 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
 MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  344 ++++
 ArmPkg/Include/Library/ArmLib.h                              |   16 +-
 MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1847 ++++++++++++++++++++
 MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  560 ++++++
 MdeModulePkg/Application/MpServicesTest/Options.c            |  164 ++
 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
 11 files changed, 3119 insertions(+), 7 deletions(-)
 create mode 100644 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf
 create mode 100644 MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf
 create mode 100644 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h
 create mode 100644 MdeModulePkg/Application/MpServicesTest/Options.h
 create mode 100644 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
 create mode 100644 MdeModulePkg/Application/MpServicesTest/MpServicesTest.c
 create mode 100644 MdeModulePkg/Application/MpServicesTest/Options.c
 create mode 100644 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S

-- 
2.30.2



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

Hello Rebecca,

I have tested the v4 set on a single-chip N1SDP.
I had to fix N1SDP for the same reasons as in
https://edk2.groups.io/g/devel/message/93633 and I post that fix to the 
list separately.

I think it works but here is the output:

Shell> MpServicesTest -P
Number of CPUs: 4, Enabled: 4
Processor 0:
	ID: 0000000000000000
	Status: BSP | Enabled | Healthy
	Location: Package 0, Core 0, Thread 0
	Extended Information: Package 0, Module 0, Tile 0, Die 0, Core 0, Thread 0

Processor 1:
	ID: 0000000000000100
	Status: AP | Enabled | Healthy
	Location: Package 1, Core 0, Thread 0
	Extended Information: Package 0, Module 0, Tile 0, Die 1, Core 0, Thread 0

Processor 2:
	ID: 0000000000010000
	Status: AP | Enabled | Healthy
	Location: Package 0, Core 0, Thread 0
	Extended Information: Package 1, Module 0, Tile 0, Die 0, Core 0, Thread 0

Processor 3:
	ID: 0000000000010100
	Status: AP | Enabled | Healthy
	Location: Package 1, Core 0, Thread 0
	Extended Information: Package 1, Module 0, Tile 0, Die 1, Core 0, Thread 0

Shell> MpServicesTest -A
Running with SingleThread FALSE, 0 (infinite) timeout...done.
Hello from CPU 1
Hello from CPU 2
Hello from CPU 3
Shell>


One suggestion on the MpServicesTest utility, maybe check the AP number 
to give a bit friendlier error message if the user pass the BSP?

Regards,
Patrik


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

Thanks!

I'm dropping the MpServicesTest patch and people will be able to use the 
application from 
UefiCpuPkg/Test/UnitTest/EfiMpServicesPpiProtocol/EfiMpServiceProtocolShellUnitTest.inf 
instead.

-- 
Rebecca Cran

On 1/11/23 09:41, Patrik Berglund wrote:
> Hello Rebecca,
> 
> I have tested the v4 set on a single-chip N1SDP.
> I had to fix N1SDP for the same reasons as in
> https://edk2.groups.io/g/devel/message/93633 and I post that fix to the 
> list separately.
> 
> I think it works but here is the output:
> 
> Shell> MpServicesTest -P
> Number of CPUs: 4, Enabled: 4
> Processor 0:
>      ID: 0000000000000000
>      Status: BSP | Enabled | Healthy
>      Location: Package 0, Core 0, Thread 0
>      Extended Information: Package 0, Module 0, Tile 0, Die 0, Core 0, 
> Thread 0
> 
> Processor 1:
>      ID: 0000000000000100
>      Status: AP | Enabled | Healthy
>      Location: Package 1, Core 0, Thread 0
>      Extended Information: Package 0, Module 0, Tile 0, Die 1, Core 0, 
> Thread 0
> 
> Processor 2:
>      ID: 0000000000010000
>      Status: AP | Enabled | Healthy
>      Location: Package 0, Core 0, Thread 0
>      Extended Information: Package 1, Module 0, Tile 0, Die 0, Core 0, 
> Thread 0
> 
> Processor 3:
>      ID: 0000000000010100
>      Status: AP | Enabled | Healthy
>      Location: Package 1, Core 0, Thread 0
>      Extended Information: Package 1, Module 0, Tile 0, Die 1, Core 0, 
> Thread 0
> 
> Shell> MpServicesTest -A
> Running with SingleThread FALSE, 0 (infinite) timeout...done.
> Hello from CPU 1
> Hello from CPU 2
> Hello from CPU 3
> Shell>
> 
> 
> One suggestion on the MpServicesTest utility, maybe check the AP number 
> to give a bit friendlier error message if the user pass the BSP?



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

On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca@quicinc.com> wrote:
>
> Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
> add an MpServicesTest application to exercise it.
>
> Changes from v3:
>
> Removed Ard's 'Reviewed-by' line from the commits since the code has changed
> sufficiently that it's no longer valid.
>
> Rebecca Cran (3):
>   ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
>   ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
>   MdeModulePkg: Add new Application/MpServicesTest application
>
>  ArmPkg/ArmPkg.dsc                                            |    1 +
>  MdeModulePkg/MdeModulePkg.dsc                                |    2 +
>  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
>  MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
>  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  344 ++++
>  ArmPkg/Include/Library/ArmLib.h                              |   16 +-
>  MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
>  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1847 ++++++++++++++++++++
>  MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  560 ++++++
>  MdeModulePkg/Application/MpServicesTest/Options.c            |  164 ++
>  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
>  11 files changed, 3119 insertions(+), 7 deletions(-)

Hello Rebecca,

Thanks for reposting this.

Running the secondaries with MMU and caches on is a huge improvement.
I would suggest, though, that we enable the MMU first thing out of
reset, and do so from asm code so we don't have to reason about the
stack (pushing something with the MMU off and popping it with the MMU
on requires cache maintenance as well, and there is no way this can be
done from the code itself without help from the compiler)

So I propose adding the diff below - note that only the variables
holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
the executable image) - everything else will be accessed by the
secondaries with the MMU enabled.

https://paste.debian.net/1266242/

WIth a tweak to the RPI4 platform that I sent out separately, this all
works fine for me (both with and without the diff above applied)

-- 
Ard.


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

On 1/5/23 10:39, Ard Biesheuvel wrote:

> So I propose adding the diff below - note that only the variables
> holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
> the executable image) - everything else will be accessed by the
> secondaries with the MMU enabled.
> 
> https://paste.debian.net/1266242/
> 
> WIth a tweak to the RPI4 platform that I sent out separately, this all
> works fine for me (both with and without the diff above applied)
> 

Sorry I didn't get around to working on this before the paste.debian.org 
link expired. I think I remember what the code was doing though, so I'll 
try and recreate it.

-- 
Rebecca Cran


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

On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca@quicinc.com> wrote:
> >
> > Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
> > add an MpServicesTest application to exercise it.
> >
> > Changes from v3:
> >
> > Removed Ard's 'Reviewed-by' line from the commits since the code has changed
> > sufficiently that it's no longer valid.
> >
> > Rebecca Cran (3):
> >   ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
> >   ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
> >   MdeModulePkg: Add new Application/MpServicesTest application
> >
> >  ArmPkg/ArmPkg.dsc                                            |    1 +
> >  MdeModulePkg/MdeModulePkg.dsc                                |    2 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
> >  MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  344 ++++
> >  ArmPkg/Include/Library/ArmLib.h                              |   16 +-
> >  MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1847 ++++++++++++++++++++
> >  MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  560 ++++++
> >  MdeModulePkg/Application/MpServicesTest/Options.c            |  164 ++
> >  ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
> >  11 files changed, 3119 insertions(+), 7 deletions(-)
>
> Hello Rebecca,
>
> Thanks for reposting this.
>
> Running the secondaries with MMU and caches on is a huge improvement.
> I would suggest, though, that we enable the MMU first thing out of
> reset, and do so from asm code so we don't have to reason about the
> stack (pushing something with the MMU off and popping it with the MMU
> on requires cache maintenance as well, and there is no way this can be
> done from the code itself without help from the compiler)
>
> So I propose adding the diff below - note that only the variables
> holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
> the executable image) - everything else will be accessed by the
> secondaries with the MMU enabled.
>
> https://paste.debian.net/1266242/
>
> WIth a tweak to the RPI4 platform that I sent out separately, this all
> works fine for me (both with and without the diff above applied)
>

Actually, maybe better to retain the hunk below. I saw some weird
hangs without them

diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
index ab439bffd722..eb634a25b33a 100644
--- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
+++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
@@ -1345,18 +1345,6 @@ MpServicesInitialize (

   gProcessorIDs[Index] = MAX_UINT64;

-  //
-  // The global pointer variables as well as the gProcessorIDs array contents
-  // are accessed by the other cores so we must clean them to the PoC
-  //
-  WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *));
-  WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *));
-
-  WriteBackDataCacheRange (
-    gProcessorIDs,
-    (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64)
-    );
-
   mNonBlockingModeAllowed = TRUE;

   Status = EfiCreateEventReadyToBootEx (


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

Hi Ard/Rebecca,

Thanks for bringing this to the mailing list. But somehow I cannot find 
the patches sent along with this
v4 cover letter. Could you please point me to them?

I have been running the previous version of this patch and noticed a 
minor issue when the wait event is
specified but the timeout is 0. In this case the CheckApStatus function 
could fall into the timeout scenario
regardless and signal the event timeout incorrectly. I attempted a 
simple fix here:
https://github.com/kuqin12/mu_silicon_arm_tiano/commit/591462cc32e621c1da470a8319f7deda723e8509

Please let me know if I misunderstood anything for the above case. 
Thanks in advance!

Regards,
Kun

On 1/5/2023 9:59 AM, Ard Biesheuvel wrote:
> On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca@quicinc.com> wrote:
>>> Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
>>> add an MpServicesTest application to exercise it.
>>>
>>> Changes from v3:
>>>
>>> Removed Ard's 'Reviewed-by' line from the commits since the code has changed
>>> sufficiently that it's no longer valid.
>>>
>>> Rebecca Cran (3):
>>>    ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
>>>    ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
>>>    MdeModulePkg: Add new Application/MpServicesTest application
>>>
>>>   ArmPkg/ArmPkg.dsc                                            |    1 +
>>>   MdeModulePkg/MdeModulePkg.dsc                                |    2 +
>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
>>>   MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  344 ++++
>>>   ArmPkg/Include/Library/ArmLib.h                              |   16 +-
>>>   MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 1847 ++++++++++++++++++++
>>>   MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  560 ++++++
>>>   MdeModulePkg/Application/MpServicesTest/Options.c            |  164 ++
>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
>>>   11 files changed, 3119 insertions(+), 7 deletions(-)
>> Hello Rebecca,
>>
>> Thanks for reposting this.
>>
>> Running the secondaries with MMU and caches on is a huge improvement.
>> I would suggest, though, that we enable the MMU first thing out of
>> reset, and do so from asm code so we don't have to reason about the
>> stack (pushing something with the MMU off and popping it with the MMU
>> on requires cache maintenance as well, and there is no way this can be
>> done from the code itself without help from the compiler)
>>
>> So I propose adding the diff below - note that only the variables
>> holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
>> the executable image) - everything else will be accessed by the
>> secondaries with the MMU enabled.
>>
>> https://paste.debian.net/1266242/
>>
>> WIth a tweak to the RPI4 platform that I sent out separately, this all
>> works fine for me (both with and without the diff above applied)
>>
> Actually, maybe better to retain the hunk below. I saw some weird
> hangs without them
>
> diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> index ab439bffd722..eb634a25b33a 100644
> --- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> +++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
> @@ -1345,18 +1345,6 @@ MpServicesInitialize (
>
>     gProcessorIDs[Index] = MAX_UINT64;
>
> -  //
> -  // The global pointer variables as well as the gProcessorIDs array contents
> -  // are accessed by the other cores so we must clean them to the PoC
> -  //
> -  WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *));
> -  WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *));
> -
> -  WriteBackDataCacheRange (
> -    gProcessorIDs,
> -    (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64)
> -    );
> -
>     mNonBlockingModeAllowed = TRUE;
>
>     Status = EfiCreateEventReadyToBootEx (
>
>
> 
>
>


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

The patches aren't attached to the cover letter, but are separate 
emails. You should be able to find them by looking for "PATCH v4 1/3", 
"PATCH v4 2/3" etc.

I've made several changes to the WaitEvent handling in the latest patch: 
could you check if the problem has been fixed please?

-- 
Rebecca Cran

On 1/5/23 22:11, Kun Qin wrote:
> Hi Ard/Rebecca,
> 
> Thanks for bringing this to the mailing list. But somehow I cannot find 
> the patches sent along with this
> v4 cover letter. Could you please point me to them?
> 
> I have been running the previous version of this patch and noticed a 
> minor issue when the wait event is
> specified but the timeout is 0. In this case the CheckApStatus function 
> could fall into the timeout scenario
> regardless and signal the event timeout incorrectly. I attempted a 
> simple fix here:
> https://github.com/kuqin12/mu_silicon_arm_tiano/commit/591462cc32e621c1da470a8319f7deda723e8509
> 
> Please let me know if I misunderstood anything for the above case. 
> Thanks in advance!
> 
> Regards,
> Kun
> 
> On 1/5/2023 9:59 AM, Ard Biesheuvel wrote:
>> On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel <ardb@kernel.org> wrote:
>>> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca@quicinc.com> wrote:
>>>> Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, and
>>>> add an MpServicesTest application to exercise it.
>>>>
>>>> Changes from v3:
>>>>
>>>> Removed Ard's 'Reviewed-by' line from the commits since the code has 
>>>> changed
>>>> sufficiently that it's no longer valid.
>>>>
>>>> Rebecca Cran (3):
>>>>    ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to ArmLib.h
>>>>    ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls
>>>>    MdeModulePkg: Add new Application/MpServicesTest application
>>>>
>>>>   ArmPkg/ArmPkg.dsc                                            |    1 +
>>>>   MdeModulePkg/MdeModulePkg.dsc                                |    2 +
>>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |   56 +
>>>>   MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf   |   40 +
>>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h     |  
>>>> 344 ++++
>>>>   ArmPkg/Include/Library/ArmLib.h                              |   
>>>> 16 +-
>>>>   MdeModulePkg/Application/MpServicesTest/Options.h            |   39 +
>>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c   | 
>>>> 1847 ++++++++++++++++++++
>>>>   MdeModulePkg/Application/MpServicesTest/MpServicesTest.c     |  
>>>> 560 ++++++
>>>>   MdeModulePkg/Application/MpServicesTest/Options.c            |  
>>>> 164 ++
>>>>   ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S                |   57 +
>>>>   11 files changed, 3119 insertions(+), 7 deletions(-)
>>> Hello Rebecca,
>>>
>>> Thanks for reposting this.
>>>
>>> Running the secondaries with MMU and caches on is a huge improvement.
>>> I would suggest, though, that we enable the MMU first thing out of
>>> reset, and do so from asm code so we don't have to reason about the
>>> stack (pushing something with the MMU off and popping it with the MMU
>>> on requires cache maintenance as well, and there is no way this can be
>>> done from the code itself without help from the compiler)
>>>
>>> So I propose adding the diff below - note that only the variables
>>> holding TCR, MAIR and TTBR0 need cache maintenance now (in addition to
>>> the executable image) - everything else will be accessed by the
>>> secondaries with the MMU enabled.
>>>
>>> https://paste.debian.net/1266242/
>>>
>>> WIth a tweak to the RPI4 platform that I sent out separately, this all
>>> works fine for me (both with and without the diff above applied)
>>>
>> Actually, maybe better to retain the hunk below. I saw some weird
>> hangs without them
>>
>> diff --git a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
>> b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
>> index ab439bffd722..eb634a25b33a 100644
>> --- a/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
>> +++ b/ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c
>> @@ -1345,18 +1345,6 @@ MpServicesInitialize (
>>
>>     gProcessorIDs[Index] = MAX_UINT64;
>>
>> -  //
>> -  // The global pointer variables as well as the gProcessorIDs array 
>> contents
>> -  // are accessed by the other cores so we must clean them to the PoC
>> -  //
>> -  WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *));
>> -  WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *));
>> -
>> -  WriteBackDataCacheRange (
>> -    gProcessorIDs,
>> -    (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64)
>> -    );
>> -
>>     mNonBlockingModeAllowed = TRUE;
>>
>>     Status = EfiCreateEventReadyToBootEx (
>>
>>
>> 
>>
>>


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

Hi Rebecca,

Thanks for the information. Somehow these patches (i.e. 
https://edk2.groups.io/g/devel/message/98106) just landed on groups.io 
last night.

I will test the latest version of your patches and reply back here.

Regards,
Kun

On 1/6/2023 10:42 AM, Rebecca Cran wrote:
> The patches aren't attached to the cover letter, but are separate 
> emails. You should be able to find them by looking for "PATCH v4 1/3", 
> "PATCH v4 2/3" etc.
>
> I've made several changes to the WaitEvent handling in the latest 
> patch: could you check if the problem has been fixed please?
>


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