[edk2-devel] [PATCH v4 0/3] ArmPkg, MdeModulePkg: Implement EFI_MP_SERVICES_PROTOCOL for AArch64 and add an MpServicesTest application to exercise it
Rebecca Cran
quic_rcran at quicinc.com
Fri Jan 6 18:42:44 UTC 2023
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 at kernel.org> wrote:
>>> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran <rebecca at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list