Messages in this thread | | | From | "Huang, Kai" <> | Subject | Re: [PATCHv2 02/13] kernel/cpu: Add support for declaring CPU offlining not supported | Date | Mon, 23 Oct 2023 22:07:04 +0000 |
| |
On Mon, 2023-10-23 at 18:31 +0300, kirill.shutemov@linux.intel.com wrote: > 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.
Personally I think this is better because it is odd to have two mechanisms in place even temporarily especially when we can avoid it. But no hard opinion. Up to you.
> > > 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. >
Yeah sure. Can we put this to changelog to make it clearer? :-)
| |