From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web09.1153.1610055945886272672 for ; Thu, 07 Jan 2021 13:45:46 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=hPa0inCa; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1610055945; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=5Gp2Dld/jruFtZRu8mGNolIHkp0l7yBuViTAfsJA1Pg=; b=hPa0inCa4P/cdnR2p6Kn3YD8Xz7M8YMis0W2xxAKjN0ATXcOtdbhFeVIRV0l/3zX+NLm50 Ok8VgFI3ineiAI3WNFnuCnBLZN/XtwJ13tdUMdguobYlnVRWOq3WgkU1dZBOZQC0USdCfu s/YuaVhvb8/1UkQONOKDFvebUezO/eE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-545-GPxL1RhCOeiHBtFxQa_B5w-1; Thu, 07 Jan 2021 16:45:40 -0500 X-MC-Unique: GPxL1RhCOeiHBtFxQa_B5w-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 44A14AFA86; Thu, 7 Jan 2021 21:45:38 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-164.ams2.redhat.com [10.36.112.164]) by smtp.corp.redhat.com (Postfix) with ESMTP id E46FE19CAD; Thu, 7 Jan 2021 21:45:01 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 05/10] MdePkg: add MmRegisterShutdownInterface() From: "Laszlo Ersek" To: devel@edk2.groups.io, ankur.a.arora@oracle.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 Reply-To: devel@edk2.groups.io, lersek@redhat.com References: <20210107195515.106158-1-ankur.a.arora@oracle.com> <20210107195515.106158-6-ankur.a.arora@oracle.com> Message-ID: <3496d49a-90c3-da02-0511-a82f9b6399d4@redhat.com> Date: Thu, 7 Jan 2021 22:45:00 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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.) (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". (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