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
