From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0b-0031df01.pphosted.com (mx0b-0031df01.pphosted.com [205.220.180.131]) by mx.groups.io with SMTP id smtpd.web11.135866.1669679956252728095 for ; Mon, 28 Nov 2022 15:59:16 -0800 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@quicinc.com header.s=qcppdkim1 header.b=HVAzdMCK; 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.180.131, mailfrom: quic_rcran@quicinc.com) Received: from pps.filterd (m0279869.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 2ASMuKsA028504; Mon, 28 Nov 2022 23:59:05 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; h=message-id : date : mime-version : subject : to : references : from : in-reply-to : content-type : content-transfer-encoding; s=qcppdkim1; bh=XGbjudVlniRSkA1cbDBgCinKmkz0TrnrpmYIZrn3mtQ=; b=HVAzdMCKAWx2GTRrWnDxx5tGD+CMVzizkcvuFk21DgB08DnHUd/j2haOOI2r5gYWKSQw e7TJOV80ShVBqOYi+Et/9oIAiTwDnaIumWkvhFth5N3QKJo0M5rxPNmp6/Fe4A74brH0 2yjfQ/h62UvaL/d89znc6n6PLr+mU0JO2O9WZm9MJC4Ne+kHkq3Is60aA8Dxb1JNPdcQ VrAqwr+yjq//g35NPq8DqBeZGlah4yMyT96BgvECJ21UsZJmJyPeX1FmFeYSiL2zyCvh VjvsR4qAzzgGo3KSYfS5WYBFwC0mZukzcV0/Oc0PpCIx+IT+PEy0/IhckKoHK3PCqwyK Cg== Received: from nasanppmta05.qualcomm.com (i-global254.qualcomm.com [199.106.103.254]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 3m39tg5d2c-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Nov 2022 23:59:05 +0000 Received: from nasanex01b.na.qualcomm.com (nasanex01b.na.qualcomm.com [10.46.141.250]) by NASANPPMTA05.qualcomm.com (8.17.1.5/8.17.1.5) with ESMTPS id 2ASNx418029526 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 28 Nov 2022 23:59:04 GMT Received: from [10.110.4.140] (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; Mon, 28 Nov 2022 15:59:03 -0800 Message-ID: Date: Mon, 28 Nov 2022 16:59:02 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.5.0 Subject: Re: [edk2-devel] [PATCH 1/2] ArmPkg: implement EFI_MP_SERVICES_PROTOCOL based on PSCI calls To: , , , Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Jian J Wang , Liming Gao References: <20220829155955.3767-1-rebecca@quicinc.com> <20220829155955.3767-2-rebecca@quicinc.com> <25c22db8-2974-44c3-9482-3972af6cd08c@gmail.com> From: "Rebecca Cran" In-Reply-To: <25c22db8-2974-44c3-9482-3972af6cd08c@gmail.com> X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) 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-GUID: rHhC678T5JMn4qaZNbDqo5tGMHtT5JmQ X-Proofpoint-ORIG-GUID: rHhC678T5JMn4qaZNbDqo5tGMHtT5JmQ X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.895,Hydra:6.0.545,FMLib:17.11.122.1 definitions=2022-11-28_17,2022-11-28_02,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 bulkscore=0 suspectscore=0 adultscore=0 spamscore=0 lowpriorityscore=0 malwarescore=0 mlxlogscore=999 mlxscore=0 phishscore=0 impostorscore=0 priorityscore=1501 clxscore=1011 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2210170000 definitions=main-2211280172 X-MIME-Autoconverted: from 8bit to quoted-printable by mx0a-0031df01.pphosted.com id 2ASMuKsA028504 Content-Language: en-US Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: quoted-printable Sorry, I missed this originally. I've added my replies inline.][ On 9/29/22 12:45, Kun Qin wrote: > Hi Rebecca, >=20 > Thanks for sending this patch. I have a few questions inline "[KQ]".=20 > Could you please help me to > understand the patch better? Thanks in advance. >=20 > Regards, > Kun >=20 > On 8/29/2022 8:59 AM, Rebecca Cran wrote: >> +#define GET_MPIDR_AFFINITY_BITS(x)=C2=A0 ((x) & 0xFF00FFFFFF) >> + >> +#define MPIDR_MT_BIT=C2=A0 BIT24 > [KQ]: Is there any reason why this is not defined in a more formal=20 > place? I think that will allow the platform to use as well. That's a good point. I can certainly move it somewhere more proper. >> +=C2=A0 BOOLEAN=C2=A0=C2=A0=C2=A0=C2=A0 IsBsp; > [KQ]: Can we add a check to see if BSP is found during this routine? I=20 > think failing to identify a BSP might cause other issues in the=20 > following services? We could certainly log an error and abort if IsBsp is never set to TRUE. >> +=C2=A0 for (Index =3D 0; Index < mCpuMpData.NumberOfProcessors; Index= ++) { >> +=C2=A0=C2=A0=C2=A0 if (GET_MPIDR_AFFINITY_BITS (ArmReadMpidr ()) =3D=3D= =20 >> CoreInfo[Index].Mpidr) { > [KQ]: Does this mean BSP should never set the `MPIDR_MT_BIT` in the=20 > gArmMpCoreInfoGuid HOB data? Otherwise, this logic will never be able t= o=20 > find the BSP? No, here we assume we're running on the BSP, and the code is looking for=20 the entry in the CoreInfo array that matches the BSP's affinity fields. >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 switch (GetApState (CpuData)) { >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 case CpuStateReady: >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 SetApProcedure= (CpuData, Procedure, ProcedureArgument); >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Status =3D Dis= patchCpu (Index); >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (EFI_ERROR = (Status)) { >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Cp= uData->State =3D CpuStateIdle; >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 St= atus=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D EFI_NOT_READY; >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 go= to Done; > [KQ]: Is it really necessary to use "goto Done"? This might cause all=20 > the CPUs after this failing index to not reset the CPU state. > That way, all the subsequent "StartupAllAps" will fail due to=20 > "CheckAllCpusReady" returning FALSE on those "dirty cores". Please > let me know if that is by design or not. Good point. I'll fix it. --=20 Rebecca Cran