Re: [edk2-devel] [PATCH wave 1 00/10] support QEMU's "SMRAM at default SMBASE" feature

["Laszlo Ersek" <lersek@redhat.com>]

On 09/27/19 13:35, Igor Mammedov wrote:
> On Tue, 24 Sep 2019 13:34:55 +0200
> "Laszlo Ersek" <lersek@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