lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ