From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) by mx.groups.io with SMTP id smtpd.web11.1310.1673030576954628820 for ; Fri, 06 Jan 2023 10:42:56 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@quicinc.com header.s=qcppdkim1 header.b=VyhwzIMX; spf=permerror, err=parse error for token &{10 18 %{ir}.%{v}.%{d}.spf.has.pphosted.com}: invalid domain name (domain: quicinc.com, ip: 205.220.168.131, mailfrom: quic_rcran@quicinc.com) Received: from pps.filterd (m0279862.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 306HnlDw012053; Fri, 6 Jan 2023 18:42:46 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=message-id : date : mime-version : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding; s=qcppdkim1; bh=iHjKVKY2l2ZnT+TCxpSOy/hA29mWHjDb05NJKsyUBIU=; b=VyhwzIMXR8ZwGxQRVZzGD5wx8jP+HUdk9/DXQBTLZj2l2tYcM+1nzM7//3wBCDERyi/e 7ZOLeBssGGiybvJVcjqpOYEvX8G1N8q6ZXWBoWBx8fHukWlfaZGH/e4kcVqLfSdHr3N4 Zk3RumhcfMZ8kEuJxSYljSRkAtYoVm+dGC9lynddX48M6rH5S3kUMW+WLAhf3pULs2ug ynryfsMhH3aULO1IYq53n8vWm8kJ8Gvu86arf8LrIvyiXaQ69wiDo3eSHpX4g5Olv+ln SabONXaRu/jBZLdV+n4FDhzwR7UaJwHUHqxx0xbqKjMn82qF8u7+TXPp8k715Mndmknd TQ== Received: from nasanppmta05.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3mxaqj9udr-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 06 Jan 2023 18:42:46 +0000 Received: from nasanex01b.na.qualcomm.com (corens_vlan604_snip.qualcomm.com [10.53.140.1]) by NASANPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 306IgkEo027578 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 6 Jan 2023 18:42:46 GMT Received: from [10.110.23.237] (10.80.80.8) by nasanex01b.na.qualcomm.com (10.46.141.250) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.36; Fri, 6 Jan 2023 10:42:45 -0800 Message-ID: <421e3d4e-22b3-b0e0-5990-7a6918fc8700@quicinc.com> Date: Fri, 6 Jan 2023 11:42:44 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [edk2-devel] [PATCH v4 0/3] ArmPkg,MdeModulePkg: Implement EFI_MP_SERVICES_PROTOCOL for AArch64 and add an MpServicesTest application to exercise it To: Kun Qin , , CC: Sami Mujawar , Ard Biesheuvel , Leif Lindholm , "Jian J Wang" , Liming Gao , "Tiger Liu" References: <20230104153727.345236-1-rebecca@quicinc.com> <649d9d13-f5d9-ae8c-9067-e27a05b88f6d@gmail.com> From: "Rebecca Cran" In-Reply-To: <649d9d13-f5d9-ae8c-9067-e27a05b88f6d@gmail.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nasanex01b.na.qualcomm.com (10.46.141.250) X-QCInternal: smtphost X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=5800 signatures=585085 X-Proofpoint-ORIG-GUID: fB7NDmO9n2aePQ3VcyOyCi3YoR5V3mGq X-Proofpoint-GUID: fB7NDmO9n2aePQ3VcyOyCi3YoR5V3mGq X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2023-01-06_12,2023-01-06_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 phishscore=0 adultscore=0 mlxscore=0 lowpriorityscore=0 bulkscore=0 spamscore=0 mlxlogscore=999 clxscore=1011 malwarescore=0 impostorscore=0 priorityscore=1501 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2301060144 X-MIME-Autoconverted: from 8bit to quoted-printable by mx0a-0031df01.pphosted.com id 306HnlDw012053 Content-Language: en-US Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable The patches aren't attached to the cover letter, but are separate=20 emails. You should be able to find them by looking for "PATCH v4 1/3",=20 "PATCH v4 2/3" etc. I've made several changes to the WaitEvent handling in the latest patch:=20 could you check if the problem has been fixed please? --=20 Rebecca Cran On 1/5/23 22:11, Kun Qin wrote: > Hi Ard/Rebecca, >=20 > Thanks for bringing this to the mailing list. But somehow I cannot find= =20 > the patches sent along with this > v4 cover letter. Could you please point me to them? >=20 > I have been running the previous version of this patch and noticed a=20 > minor issue when the wait event is > specified but the timeout is 0. In this case the CheckApStatus function= =20 > could fall into the timeout scenario > regardless and signal the event timeout incorrectly. I attempted a=20 > simple fix here: > https://github.com/kuqin12/mu_silicon_arm_tiano/commit/591462cc32e621c1da= 470a8319f7deda723e8509 >=20 > Please let me know if I misunderstood anything for the above case.=20 > Thanks in advance! >=20 > Regards, > Kun >=20 > On 1/5/2023 9:59 AM, Ard Biesheuvel wrote: >> On Thu, 5 Jan 2023 at 18:39, Ard Biesheuvel wrote: >>> On Wed, 4 Jan 2023 at 16:37, Rebecca Cran wrote: >>>> Implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls for AArch64, an= d >>>> add an MpServicesTest application to exercise it. >>>> >>>> Changes from v3: >>>> >>>> Removed Ard's 'Reviewed-by' line from the commits since the code has= =20 >>>> changed >>>> sufficiently that it's no longer valid. >>>> >>>> Rebecca Cran (3): >>>> =C2=A0=C2=A0 ArmPkg: Add GET_MPIDR_AFFINITY_BITS and MPIDR_MT_BIT to A= rmLib.h >>>> =C2=A0=C2=A0 ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI = calls >>>> =C2=A0=C2=A0 MdeModulePkg: Add new Application/MpServicesTest applicat= ion >>>> >>>> =C2=A0 ArmPkg/ArmPkg.dsc=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2= =A0=C2=A0 1 + >>>> =C2=A0 MdeModulePkg/MdeModulePkg.dsc=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 |=C2=A0=C2=A0=C2=A0 2 + >>>> =C2=A0 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.inf |= =C2=A0=C2=A0 56 + >>>> =C2=A0 MdeModulePkg/Application/MpServicesTest/MpServicesTest.inf=C2= =A0=C2=A0 |=C2=A0=C2=A0 40 + >>>> =C2=A0 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpServicesInternal.h=C2=A0= =C2=A0=C2=A0=C2=A0 | =20 >>>> 344 ++++ >>>> =C2=A0 ArmPkg/Include/Library/ArmLib.h=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | = =20 >>>> 16 +- >>>> =C2=A0 MdeModulePkg/Application/MpServicesTest/Options.h=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |=C2=A0=C2=A0 39 + >>>> =C2=A0 ArmPkg/Drivers/ArmPsciMpServicesDxe/ArmPsciMpServicesDxe.c=C2= =A0=C2=A0 |=20 >>>> 1847 ++++++++++++++++++++ >>>> =C2=A0 MdeModulePkg/Application/MpServicesTest/MpServicesTest.c=C2=A0= =C2=A0=C2=A0=C2=A0 | =20 >>>> 560 ++++++ >>>> =C2=A0 MdeModulePkg/Application/MpServicesTest/Options.c=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 | =20 >>>> 164 ++ >>>> =C2=A0 ArmPkg/Drivers/ArmPsciMpServicesDxe/MpFuncs.S=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 |= =C2=A0=C2=A0 57 + >>>> =C2=A0 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 ( >> >> =C2=A0=C2=A0=C2=A0 gProcessorIDs[Index] =3D MAX_UINT64; >> >> -=C2=A0 // >> -=C2=A0 // The global pointer variables as well as the gProcessorIDs arr= ay=20 >> contents >> -=C2=A0 // are accessed by the other cores so we must clean them to the = PoC >> -=C2=A0 // >> -=C2=A0 WriteBackDataCacheRange (&gProcessorIDs, sizeof (UINT64 *)); >> -=C2=A0 WriteBackDataCacheRange (&gApStacksBase, sizeof (UINT64 *)); >> - >> -=C2=A0 WriteBackDataCacheRange ( >> -=C2=A0=C2=A0=C2=A0 gProcessorIDs, >> -=C2=A0=C2=A0=C2=A0 (mCpuMpData.NumberOfProcessors + 1) * sizeof (UINT64= ) >> -=C2=A0=C2=A0=C2=A0 ); >> - >> =C2=A0=C2=A0=C2=A0 mNonBlockingModeAllowed =3D TRUE; >> >> =C2=A0=C2=A0=C2=A0 Status =3D EfiCreateEventReadyToBootEx ( >> >> >>=20 >> >>