Re: [PATCH v8 3/7] crypto/ccp: Disable CPU hotplug while SNP is active

[Kalra, Ashish]

On 6/21/2026 5:44 AM, Thomas Gleixner wrote:
> On Fri, Jun 19 2026 at 18:51, Ashish Kalra wrote:
>> On 6/19/2026 6:20 PM, Borislav Petkov wrote:
>>> I'd let tglx maybe give a better idea but this cpu_hotplug_disable static var
>>> in kernel/cpu.c could get a getter function and be used instead of you
>>> reinventing the wheel in here.
>>
>> I don't follow — I'm not reinventing anything here. Patch 3 will use
>> the existing CPU-hotplug callback interface: cpuhp_setup_state() with
>> a down callback that returns -EBUSY to refuse the offline while SNP is
>> active. That's the standard mechanism for conditionally preventing a
>> CPU offline, and it keeps no private "hotplug disabled" state of its
>> own — so there's nothing here for a getter on cpu_hotplug_disabled to
>> replace.
> 
> That's not a standard mechanism. That's a hack as you have to start the
> offlining operation in order to prevent something you already know.
> 
> The return code which prevents offlining is there for situations where
> the subsystem/driver is momentarily in a state which cannot be
> resolved.
> 
> That's a very different story than knowing that state at the point of
> installing the callback already.
> 

Sure.

>> I chose the cpuhp callback specifically over
>> cpu_hotplug_disable_offlining(): the callback can be torn down with
>> cpuhp_remove_state() when SNP is fully shut down, which re-enables CPU
>> offlining. cpu_hotplug_disable_offlining() sets
>> cpu_hotplug_offline_disabled, which is __ro_after_init and one-way —
>> there's no interface to clear it, and adding one would mean dropping
>> the __ro_after_init marking and a new core "re-enable offlining"
>> API. So that route can't re-enable offlining on SNP shutdown without
>> new core plumbing, whereas the cpuhp callback gives me that for free.
> 
> That's exactly the wrong attitude. Hack around a core limitation in a
> random driver by abusing the provided mechanism instead of sitting down
> and doing the extra five lines of code which makes it entirely clear
> what's going on.
> 
> When Boris asked me how to disable hotplug, I had the impression that
> this is about permanently preventing it. So I pointed him to
> cpu_hotplug_disable_offlining().
> 
> If I had known that it's about temporary prevention during runtime of
> something, then I'd pointed him to cpu_hotplug_disable()/enable() which
> is five lines farther down in cpu.c. It's not rocket science to find
> them. The first AI chatbot I asked pointed me to it immediately.
> 
> cpu_hotplug_disable()/enable() is _the_ standard mechanism to prevent
> hotplug operations temporarily. They return -EBUSY without invoking any
> callback or changing any related state.
> 

This is the interface i have been using, in fact this current patch (v8) is based
on cpu_hotplug_disable()/enable(), but then this thread started from review feedback
on the v8 patch using cpu_hotplug_disable()/enable() that it looks like a hack — 
the concern being that cpu_hotplug_disable() reads as a temporary lock rather than a
way to enforce a basically-permanent system state.

That's what led me to look at cpu_hotplug_disable_offlining() and a cpuhp down-callback
as alternatives.

Your point that cpu_hotplug_disable()/enable() is the standard mechanism to prevent hotplug
operations temporarily settles it, and the disable/enable pair being reversible is exactly
what's wanted here: it's undone when SNP is fully shut down, so it isn't actually permanent
(unlike cpu_hotplug_disable_offlining(), which is one-way). So I'll stay with 
cpu_hotplug_disable()/enable() and drop the alternatives.

Thanks,
Ashish

> So what's exactly the new core plumbing you need?
> 
> Thanks,
> 
>         tglx
>