Re: [RFC] ich9:cpuhp: add support for cpu hot-unplug with SMI broadcast enabled

[Laszlo Ersek]

On 11/27/20 05:10, Ankur Arora wrote:

> Yeah I was wondering what would happen for simultaneous hot add and remove.
> I guess we would always do remove first and then the add, unless we hit
> the break due to max_cpus_per_pass and switch to hot-add mode.

Considering the firmware only, I disagree with remove-then-add.

EFI_SMM_CPU_SERVICE_PROTOCOL.AddProcessor() and
EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor() (implemented in
SmmAddProcessor() and SmmRemoveProcessor() in
"UefiCpuPkg/PiSmmCpuDxeSmm/CpuService.c", respectively) only mark the
processors for addition/removal. The actual processing is done only
later, in BSPHandler() --> SmmCpuUpdate(), when "all SMI handlers are
finished" (see the comment in SmmRemoveProcessor()).

Consequently, I would not suggest replacing a valid APIC ID in a
particular mCpuHotPlugData.ApicId[Index] slot with INVALID_APIC_ID
(corresponding to the unplug operation), and then possibly replacing
INVALID_APIC_ID in the *same slot* with the APIC ID of the newly plugged
CPU, in the exact same SMI invocation (= in the same execution of
CpuHotplugMmi()). That might cause some component in edk2 to see the
APIC ID in mCpuHotPlugData.ApicId[Index] to change from one valid ACPI
ID to another valid APIC ID, and I don't even want to think about what
kind of mess that could cause.

So no, please handle plugs first, for which unused slots in
mCpuHotPlugData.ApicId will be populated, and *then* handle removals (in
the same invocation of CpuHotplugMmi()).

By the way, for unplug, you will not have to re-set
mCpuHotPlugData.ApicId[Index] to INVALID_APIC_ID, as
SmmRemoveProcessor() does that internally. You just have to locate the
Index for the APIC ID being removed, for calling
EFI_SMM_CPU_SERVICE_PROTOCOL.RemoveProcessor().


Thanks
Laszlo