[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e3ffb5c47126f03ef998f4ae5df81b9c9bd20da7.camel@intel.com>
Date: Mon, 23 Oct 2023 22:07:04 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
CC: "kexec@...ts.infradead.org" <kexec@...ts.infradead.org>,
"linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
"ashish.kalra@....com" <ashish.kalra@....com>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"thomas.lendacky@....com" <thomas.lendacky@....com>,
"Hunter, Adrian" <adrian.hunter@...el.com>,
"Reshetova, Elena" <elena.reshetova@...el.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"mingo@...hat.com" <mingo@...hat.com>,
"Christopherson,, Sean" <seanjc@...gle.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"bhe@...hat.com" <bhe@...hat.com>,
"Nakajima, Jun" <jun.nakajima@...el.com>,
"peterz@...radead.org" <peterz@...radead.org>,
"bp@...en8.de" <bp@...en8.de>,
"Edgecombe, Rick P" <rick.p.edgecombe@...el.com>,
"rafael@...nel.org" <rafael@...nel.org>,
"sathyanarayanan.kuppuswamy@...ux.intel.com"
<sathyanarayanan.kuppuswamy@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>
Subject: Re: [PATCHv2 02/13] kernel/cpu: Add support for declaring CPU
offlining not supported
On Mon, 2023-10-23 at 18:31 +0300, kirill.shutemov@...ux.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? :-)
Powered by blists - more mailing lists