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.web08.39649.1612206529336596985 for ; Mon, 01 Feb 2021 11:08:49 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ezTaoKzk; 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=1612206528; h=from:from: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=n3USl9c4PR5nxTs7wWHxxuKXT5Wcng6RDcyZbdV2Y3w=; b=ezTaoKzkQORRALTHw/elSnR7wZkQoNzL6DT12SuMNDh4x+26RdgPMo3GK8/tSeCSjgXaeh nXBd4bxvcNNIXi3gqXVk48oJc7hPm5OsyDBIkqe3uOoAUYuNuMVY6K/eRGHMs2EdOwGThS JGjCSJl5H5X4ZWblJLoE9cmtiL+a3Cc= 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-211-wJvOYqEYOeKz9RqQw1BYNw-1; Mon, 01 Feb 2021 14:08:45 -0500 X-MC-Unique: wJvOYqEYOeKz9RqQw1BYNw-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 29CF5107ACE3; Mon, 1 Feb 2021 19:08:44 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-95.ams2.redhat.com [10.36.112.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 812365D9DD; Mon, 1 Feb 2021 19:08:42 +0000 (UTC) Subject: Re: [PATCH v6 7/9] OvmfPkg/CpuHotplugSmm: add CpuEject() To: Ankur Arora , devel@edk2.groups.io Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210129005950.467638-1-ankur.a.arora@oracle.com> <20210129005950.467638-8-ankur.a.arora@oracle.com> From: "Laszlo Ersek" Message-ID: Date: Mon, 1 Feb 2021 20:08:41 +0100 MIME-Version: 1.0 In-Reply-To: <20210129005950.467638-8-ankur.a.arora@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 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 apologies, I've got more comments here: On 01/29/21 01:59, Ankur Arora wrote: > /** > + CPU Hot-eject handler, called from SmmCpuFeaturesRendezvousExit(), > + on each CPU at exit from SMM. > + > + If, the executing CPU is not being ejected, nothing to be done. > + If, the executing CPU is being ejected, wait in a CpuDeadLoop() > + until ejected. > + > + @param[in] ProcessorNum Index of executing CPU. > + > +**/ > +VOID > +EFIAPI > +CpuEject ( > + IN UINTN ProcessorNum > + ) > +{ > + // > + // APIC ID is UINT32, but mCpuHotEjectData->ApicIdMap[] is UINT64 > + // so use UINT64 throughout. > + // > + UINT64 ApicId; > + > + ApicId = mCpuHotEjectData->ApicIdMap[ProcessorNum]; > + if (ApicId == CPU_EJECT_INVALID) { > + return; > + } > + > + // > + // CPU(s) being unplugged get here from SmmCpuFeaturesSmiRendezvousExit() > + // after having been cleared to exit the SMI by the monarch and thus have > + // no SMM processing remaining. > + // > + // Given that we cannot allow them to escape to the guest, we pen them > + // here until the SMM monarch tells the HW to unplug them. > + // > + CpuDeadLoop (); > +} (15) There is no such function as SmmCpuFeaturesSmiRendezvousExit() -- it's SmmCpuFeaturesRendezvousExit(). (16) This function uses a data structure for communication between BSP and APs -- mCpuHotEjectData->ApicIdMap is modified in UnplugCpus() on the BSP, and checked above by the APs (too). What guarantees the visibility of mCpuHotEjectData->ApicIdMap? I think we might want to use InterlockedCompareExchange64() in both EjectCpu() and UnplugCpus() (and make "ApicIdMap" volatile, in addition). InterlockedCompareExchange64() can be used just for comparison as well, by passing ExchangeValue=CompareValue. (17) I think a similar observation applies to the "Handler" field too, as APs call it, while the BSP keeps flipping it between NULL and a real function address. We might have to turn that field into an EFI_PHYSICAL_ADDRESS (just a fancy name for UINT64), and use InterlockedCompareExchange64() again. Thanks Laszlo