From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from aserp2130.oracle.com (aserp2130.oracle.com [141.146.126.79]) by mx.groups.io with SMTP id smtpd.web08.788.1610062988202097777 for ; Thu, 07 Jan 2021 15:43:08 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@oracle.com header.s=corp-2020-01-29 header.b=msLXCEEV; spf=pass (domain: oracle.com, ip: 141.146.126.79, mailfrom: ankur.a.arora@oracle.com) Received: from pps.filterd (aserp2130.oracle.com [127.0.0.1]) by aserp2130.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 107NYpWO045417; Thu, 7 Jan 2021 23:43:02 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oracle.com; h=subject : to : cc : references : from : message-id : date : mime-version : in-reply-to : content-type : content-transfer-encoding; s=corp-2020-01-29; bh=nU8sCTx6NgEPlQ1pT1JHWdCkVLMQzm/yF+BGlKyijq8=; b=msLXCEEVUUKhF1hCZZLKzfBLNWPFY9KVFoca2atP9p1A27OeESiHi50AVcgf+gfdLuYP mQtb2fBJi0VxYI0aQYkYtWQPLg+kFjl0eilwtyX5NsC2ID9LKa3+jGSVkiriLbvYXAt+ 4pBgDmOLJyDUIJxAbYuT0x5hCgvHWXj5TgLo6oGvuwYmb6i9qggd9qTbwYP4cJitgLMt FNP+yl5ACM+K8fquchgAmQ8QtrXm2K56PRb3ORyOhfLjxNrNi7CY1SQdnMtanA2i9eGR klmZEXFcajE2zsHfxDwo8GQk5to5I6cDa2wd7GMxTGQv4fE2l2tA4H1INrmwntG2siIy pQ== Received: from aserp3020.oracle.com (aserp3020.oracle.com [141.146.126.70]) by aserp2130.oracle.com with ESMTP id 35wcuxy9dj-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 07 Jan 2021 23:43:02 +0000 Received: from pps.filterd (aserp3020.oracle.com [127.0.0.1]) by aserp3020.oracle.com (8.16.0.42/8.16.0.42) with SMTP id 107NYx62164548; Thu, 7 Jan 2021 23:43:02 GMT Received: from aserv0121.oracle.com (aserv0121.oracle.com [141.146.126.235]) by aserp3020.oracle.com with ESMTP id 35v1fbuq38-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 07 Jan 2021 23:43:01 +0000 Received: from abhmp0013.oracle.com (abhmp0013.oracle.com [141.146.116.19]) by aserv0121.oracle.com (8.14.4/8.13.8) with ESMTP id 107NgwUP018808; Thu, 7 Jan 2021 23:42:58 GMT Received: from [192.168.0.108] (/70.36.60.91) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 07 Jan 2021 23:42:57 +0000 Subject: Re: [edk2-devel] [PATCH v2 05/10] MdePkg: add MmRegisterShutdownInterface() To: devel@edk2.groups.io, lersek@redhat.com Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jian J Wang , Michael D Kinney , Liming Gao , Zhiguang Liu , Eric Dong , Ray Ni , Rahul Kumar , Aaron Young References: <20210107195515.106158-1-ankur.a.arora@oracle.com> <20210107195515.106158-6-ankur.a.arora@oracle.com> <3496d49a-90c3-da02-0511-a82f9b6399d4@redhat.com> From: "Ankur Arora" Message-ID: <94eb43f0-eb31-c6e0-6ace-558339d29576@oracle.com> Date: Thu, 7 Jan 2021 15:42:56 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 MIME-Version: 1.0 In-Reply-To: <3496d49a-90c3-da02-0511-a82f9b6399d4@redhat.com> X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9857 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 phishscore=0 suspectscore=0 spamscore=0 bulkscore=0 adultscore=0 mlxscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101070132 X-Proofpoint-Virus-Version: vendor=nai engine=6000 definitions=9857 signatures=668683 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 bulkscore=0 clxscore=1015 spamscore=0 impostorscore=0 priorityscore=1501 mlxscore=0 adultscore=0 mlxlogscore=999 lowpriorityscore=0 phishscore=0 malwarescore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2101070132 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 2021-01-07 1:45 p.m., Laszlo Ersek wrote: > On 01/07/21 21:48, Laszlo Ersek wrote: > >> You should basically migrate the contents of CpuUnplugExitWork() (in >> patch#8) into SmmCpuFeaturesRendezvousExit(), without changing the >> prototype of SmmCpuFeaturesRendezvousExit(). This might require you to >> calculate "IsBSP" on the spot, possibly with the help of some global >> data that you set up in advance. > > [...] > >> By adding the business logic to SmmCpuFeaturesRendezvousExit() instead >> of CpuUnplugExitWork, said logic will actually be included in the >> PiSmmCpuDxeSmm binary -- but that's fine. That's why the >> SmmCpuFeaturesRendezvousExit() API exists, as a platform customization >> hook for PiSmmCpuDxeSmm. > > If you need PiSmmCpuDxeSmm and CpuHotplugSmm to collaborate on a > dynamically sized array, such as "mHotUnplugWork", here's a possible > approach. > > Consider how the somewhat similar "mCpuHotPlugData" object is allocated, > and then shared between both drivers, in SMRAM. > > Line numbers at commit 85b8eac59b8c. > > (1) in the PiCpuSmmEntry() function, PiSmmCpuDxeSmm first sets > "mMaxNumberOfCpus" to the value of "PcdCpuMaxLogicalProcessorNumber". > (PcdCpuHotPlugSupport is TRUE in our case.) Line 624. > > (2) "mCpuHotPlugData.ArrayLength" is set to "mMaxNumberOfCpus". Line > 824. > > (3) SmmCpuFeaturesSmmRelocationComplete() is called > [OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c]. Line 930. > > (4) The address of "mCpuHotPlugData" is published through > "PcdCpuHotPlugDataAddress", on line 1021. (Again, PcdCpuHotPlugSupport > is TRUE.) Aah, yeah. Thanks, this is exactly what I needed to do. > > (5) "OvmfPkg/CpuHotplugSmm/CpuHotplug.c" then locates "mCpuHotPlugData" > as follows, in the CpuHotplugEntry() function: > > 17cb8ddba39b5 329) // > 17cb8ddba39b5 330) // Our DEPEX on EFI_SMM_CPU_SERVICE_PROTOCOL guarantees that PiSmmCpuDxeSmm > 17cb8ddba39b5 331) // has pointed PcdCpuHotPlugDataAddress to CPU_HOT_PLUG_DATA in SMRAM. > 17cb8ddba39b5 332) // > 17cb8ddba39b5 333) mCpuHotPlugData = (VOID *)(UINTN)PcdGet64 (PcdCpuHotPlugDataAddress); > > > The same pattern can be repeated for the new data structure that you > (might) need: > > (a) Extend the SmmCpuFeaturesSmmRelocationComplete() function in > "OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c" with the > following logic -- probably implemented as a new STATIC helper function: > > - fetch "PcdCpuMaxLogicalProcessorNumber", so that you know how to size > the "mHotUnplugWork" array > > - allocate the "mHotUnplugWork" array in SMRAM. For the allocation, you > can simply use the AllocatePool() function from MemoryAllocationLib, as > in this SMM library instance, it will allocate SMRAM. > > - if the allocation fails, hang hard (there's really nothing else to do) > > - publish the array's address via a new UINT64 PCD -- introduce a new > PCD in "OvmfPkg.dec", as dynamic-only, set a default 0 value in the DSC > files, and then use a PcdSet64S() call in the code. > > (b) The same DEPEX guarantee continues working in the CpuHotplugSmm > driver as before. Thus, in the CpuHotplugEntry() function, you can > locate "mHotUnplugWork" with another PcdGet64() function call, referring > to the new PCD. > > (c) In the SmmCpuFeaturesRendezvousExit() function -- which is part of > the same library instance as SmmCpuFeaturesSmmRelocationComplete() --, > you can simply refer to "mHotUnplugWork" by name. > > In summary, the ownership of of "mHotUnplugWork" moves from > CpuHotplugSmm to PiSmmCpuSmmDxe. PiSmmCpuSmmDxe itself does not know > about this, because all the new logic is hooked into it through existent > hook functions in "OvmfPkg/Library/SmmCpuFeaturesLib". And CpuHotplugSmm > retains access to "mHotUnplugWork" by reusing the dynamic PCD pattern > that's already employed for "mCpuHotPlugData". Right, this makes perfect sense now. Let me fix up the patches and resend in light of your comments. Thanks Ankur > > (Note that the PCD itself exists in normal RAM, but this is safe, > because the transfer of the "mHotUnplugWork" address through the PCD > occurs way before such code could execute that is not part of the > platform firmware.) > > Thanks, > Laszlo >