On Mon, Oct 23, 2023 at 09:30:59AM +0000, Huang, Kai wrote: > IMHO it's a little bit odd to have two mechanisms in place, even in this middle > state patch. Is it better to completely replace CC_ATTR_HOTPLUG_DISABLED with > the new cpu_hotplug_offline_disabled in this patch? You can explicitly call > cpu_hotplug_disable_offlining() in tdx_early_init() so no functional change is > done. I can. But I don't see how it makes a difference. > Or I am wondering why cannot just merge this and the next patch together, with a > proper justification? Because the very next thing reviewers would ask is to split them :P > Btw, IMHO the changelog (this and next patch's) seems didn't explain the true > reason to replace CC_ATTR_HOTPLUG_DISABLED. > > Currently hotplug prevented based on the confidential computing > attribute which is set for Intel TDX. But TDX is not the only possible > user of the wake up method. > > "TDX is not the only possible user of the wake up method" doesn't mean we need > to replace CC_ATTR_HOTPLUG_DISABLED. E.g., other CoCo VM type can also select > CC_ATTR_HOTPLUG_DISABLED if it uses MADT wake up method. > > To me the true reason is the new MADT wake up version actually brings the > support of offlining cpu, thus it's more suitable to decide whether the CoCo VM > needs to disable CPU offline based on the MADT wake up version, but not the CC_* > attributes that is determined by CoCo VM type. No. MADT is orthogonal to CoCo. It can be implemented outside of CoCo environment and CoCo platform can implement other wake up methods. It is not up to TDX/SEV/whatever to decide if offlining is supported. It is property of the wakeup method implemented on the platform. -- Kiryl Shutsemau / Kirill A. Shutemov _______________________________________________ kexec mailing list kexec@xxxxxxxxxxxxxxxxxxx http://lists.infradead.org/mailman/listinfo/kexec
