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.7788.1615899187513312857 for ; Tue, 16 Mar 2021 05:53:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Gy9G88IC; 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=1615899186; 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=bim99uM+T0pNFJTl2jC9M5lWJJ6c3II0gk4yeA0xkdk=; b=Gy9G88IC05gbMla+u9z/vPgAFkPwdz/kxxKVY+u1xnnzgjHN70DypKVjvTLNl7nYEFz4i1 +TLd477SS6SSk7/sKOVQ0qAn4fznWjXXcLnkmFB3mBkzX9sqFPc8ueI1AvTn+9HCkZE3M9 ZOnOhjX5NnUlW04H4Icm2H+gEIweTHY= 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-537-Wo7esQmyOMa1ctVvcLFgUQ-1; Tue, 16 Mar 2021 08:53:02 -0400 X-MC-Unique: Wo7esQmyOMa1ctVvcLFgUQ-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2045618460E0; Tue, 16 Mar 2021 12:53:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-138.ams2.redhat.com [10.36.114.138]) by smtp.corp.redhat.com (Postfix) with ESMTP id 52B705D719; Tue, 16 Mar 2021 12:52:59 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v9 09/10] OvmfPkg/CpuHotplugSmm: do actual CPU hot-eject To: devel@edk2.groups.io, ankur.a.arora@oracle.com Cc: imammedo@redhat.com, boris.ostrovsky@oracle.com, Jordan Justen , Ard Biesheuvel , Aaron Young References: <20210312062656.2477515-1-ankur.a.arora@oracle.com> <20210312062656.2477515-10-ankur.a.arora@oracle.com> From: "Laszlo Ersek" Message-ID: <0554816f-7a59-0fde-281b-6b2dadf30a4b@redhat.com> Date: Tue, 16 Mar 2021 13:52:58 +0100 MIME-Version: 1.0 In-Reply-To: <20210312062656.2477515-10-ankur.a.arora@oracle.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 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 03/12/21 07:26, Ankur Arora wrote: > @@ -214,6 +243,83 @@ EjectCpu ( > { > UINT64 QemuSelector; > > + if (CheckIfBsp ()) { > + UINT32 Idx; > + > + for (Idx = 0; Idx < mCpuHotEjectData->ArrayLength; Idx++) { > + UINT64 QemuSelector; Visual Studio warns that the inner QemuSelector declaration shadows the outer one. I'm going to remove the inner declaration, due to: > + > + QemuSelector = mCpuHotEjectData->QemuSelectorMap[Idx]; > + > + if (QemuSelector != CPU_EJECT_QEMU_SELECTOR_INVALID) { > + // > + // This to-be-ejected-CPU has already received the BSP's SMI exit > + // signal and will execute SmmCpuFeaturesRendezvousExit() > + // followed by this callback or is already penned in the > + // CpuSleep() loop below. > + // > + // Tell QEMU to context-switch it out. > + // > + QemuCpuhpWriteCpuSelector (mMmCpuIo, (UINT32) QemuSelector); > + QemuCpuhpWriteCpuStatus (mMmCpuIo, QEMU_CPUHP_STAT_EJECT); > + > + // > + // Now that we've ejected the CPU corresponding to QemuSelectorMap[Idx], > + // clear its eject status to ensure that an invalid future SMI does > + // not end up trying a spurious eject or a newly hotplugged CPU does > + // not get penned in the CpuSleep() loop. > + // > + // Note that the QemuCpuhpWriteCpuStatus() command above is a write to > + // a different address space and uses the EFI_MM_CPU_IO_PROTOCOL. > + // > + // This means that we are guaranteed that the following assignment > + // will not be reordered before the eject. And, so we can safely > + // do this write here. > + // > + mCpuHotEjectData->QemuSelectorMap[Idx] = > + CPU_EJECT_QEMU_SELECTOR_INVALID; > + > + DEBUG ((DEBUG_INFO, "%a: Unplugged ProcessorNum %u, " > + "QemuSelector %Lu\n", __FUNCTION__, Idx, QemuSelector)); > + } > + } > + > + // > + // We are done until the next hot-unplug; clear the handler. > + // > + // mCpuHotEjectData->Handler is a NOP for any CPU not under ejection. > + // So, once we are done with all the ejections, we can safely reset it > + // here since any CPU dereferencing it would only see either the old > + // or the new value (since it is aligned at a natural boundary.) > + // > + mCpuHotEjectData->Handler = NULL; > + return; > + } > + > + // > + // Reached only on APs > + // > + > + // > + // mCpuHotEjectData->QemuSelectorMap[ProcessorNum] is updated > + // on the BSP in the ongoing SMI at two places: > + // > + // - UnplugCpus() where the BSP determines if a CPU is under ejection > + // or not. As a comment in UnplugCpus() at set-up, and in > + // SmmCpuFeaturesRendezvousExit() where it is dereferenced describe, > + // any such updates are guaranteed to be ordered-before the > + // dereference below. > + // > + // - EjectCpu() on the BSP (above) updates QemuSelectorMap[ProcessorNum] > + // for a CPU once it's ejected. > + // > + // The CPU under ejection: might be executing anywhere between the > + // AllCpusInSync loop in SmiRendezvous(), to about to dereference > + // QemuSelectorMap[ProcessorNum]. > + // As described in the comment above where we do the reset, this > + // is not a problem since the ejected CPU never sees the after value. > + // CPUs not-under ejection: never see any changes so they are fine. > + // > QemuSelector = mCpuHotEjectData->QemuSelectorMap[ProcessorNum]; this reference being to the outer one. Thanks Laszlo > if (QemuSelector == CPU_EJECT_QEMU_SELECTOR_INVALID) { > return; > @@ -495,11 +601,6 @@ CpuHotplugMmi ( > if (EFI_ERROR (Status)) { > goto Fatal; > } > - if (ToUnplugCount > 0) { > - DEBUG ((DEBUG_ERROR, "%a: hot-unplug is not supported yet\n", > - __FUNCTION__)); > - goto Fatal; > - } > > if (PluggedCount > 0) { > Status = ProcessHotAddedCpus (mPluggedApicIds, PluggedCount); >