On 09/27/19 13:35, Igor Mammedov wrote: > On Tue, 24 Sep 2019 13:34:55 +0200 > "Laszlo Ersek" <ler...@redhat.com> wrote:
>> Going forward, if I understand correctly, the plan is to populate the >> new SMRAM with the hotplug SMI handler. (This would likely be put in >> place by OvmfPkg/PlatformPei, from NASM source code. The >> SmmRelocateBases() function in UefiCpuPkg/PiSmmCpuDxeSmm backs up the >> original contents before, and restores them after, the initial SMBASE >> relocation of the cold-plugged VCPUs. Thus the hotplug SMI handler would >> survive the initial relocation of the cold-plugged VCPUs.) > that's the tricky part, can we safely detect (in SmmRelocateBases) > race condition in case of several hotplugged CPUs are executing > SMI relocation handler at the same time? (crashing system in case > that happens is good enough) Wait, there are *two* initial SMI handlers here, one for cold-plugged CPUs, and another for hot-plugged CPUs. Let's just call them coldplug and hotplug handlers, for simplicity. In chronological order, during boot: (1) OvmfPkg/PlatformPei would put the hotplug handler in place, at the default SMBASE. This code would lie dormant for a long time. (2) UefiCpuPkg/PiSmmCpuDxeSmm backs up the current contents of the default SMBASE area, and installs the coldplug handler. This handler is then actually used, for relocating SMBASE for the present (cold-plugged) CPUs. Finally, UefiCpuPkg/PiSmmCpuDxeSmm restores the original contents of the default SMBASE area. This would in effect re-establish the hotplug handler from (1). (3) At OS runtime, a CPU is hotplugged, and then the hotplug handler runs, at the default SMBASE. The function SmmRelocateBases() is strictly limited to step (2), and has nothing to do with hotplug. Therefore it need not deal with any race conditions related to multi-hotplug. This is just to clarify that your question applies to the initial SMI handler (the hotplug one) that is put in place in step (1), and then used in step (3). The question does not apply to SmmRelocateBases(). With that out of the way, I do think we should be able to detect this, with a simple -- haha, famous last words! -- compare-and-exchange (LOCK CMPXCHG); a kind of semaphore. The initial value of an integer variable in SMRAM should be 1. At the start of the hotplug initial SMI handler, the CPU should try to swap that with 0. If it succeeds, proceed. If it fails (the variable's value is not 1), force a platform reset. Finally, right before the RSM, restore 1 (swap it back). (It does not matter if another hotplug CPU starts the relocation in SMM while the earlier one is left with *only* the RSM instruction in SMM, immediately after the swap-back.) Alternatively, if it's friendlier or simpler, we can spin on the initial LOCK CMPXCHG until it succeeds, executing PAUSE in the loop body. That should be friendly with KVM too (due to pause loop exiting). >> Further, when a VCPU is hot-plugged, we seem to intend to raise an SMI >> for it (perhaps internally to QEMU?), > I'd rather rise SMI from guest side (using IO port write from > ACPI cpu hotplug handler). Which in typical case (QEMU negotiated > SMI broadcast) will trigger pending SMI on hotplugged CPU as well. > (if it's acceptable I can prepare corresponding QEMU patches) What if the OS does not send the SMI (from the GPE handler) at once, and boots the new CPU instead? Previously, you wrote that the new CPU "won't get access to privileged SMRAM so OS can't subvert firmware. The next time SMI broadcast is sent the CPU will use SMI handler at default 30000 SMBASE. It's up to us to define behavior here (for example relocation handler can put such CPU in shutdown state)." How can we discover in the hotplug initial SMI handler at 0x3_0000 that the CPU executing the code should have been relocated *earlier*, and the OS just didn't obey the ACPI GPE code? I think a platform reset would be a proper response, but I don't know how to tell, "this CPU should not be here". > In case of unicast SMIs, a CPU handling guest ACPI triggered > SMI, can send SMI to hotpluged CPU as an additional step. I think it's OK if we restrict this feature to "broadcast SMI" only. > >> which will remain pending until >> the VCPU is launched with INIT-SIPI-SIPI. Once the INIT-SIPI-SIPI is >> sent, the VCPU will immediately enter SMM, and run the SMI handler in >> the locked SMRAM at the default SMBASE. At RSM, it may continue at the >> vector requested in the INIT-SIPI-SIPI. > Firmware can arbitrate/send INIT-SIPI-SIPI to hotplugged CPUs. > Enumerating hotplugged CPUs, can be done by reusing CPU hotplug > interface described at QEMU/docs/specs/acpi_cpu_hotplug.txt. > (preferably in read-only mode) ... I'll have to look at that later :) Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48350): https://edk2.groups.io/g/devel/message/48350 Mute This Topic: https://groups.io/mt/34274934/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
