Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     David Woodhouse <dwmw2@infradead.org>,
        Usama Arif <usama.arif@bytedance.com>, kim.phillips@amd.com,
        brgerst@gmail.com
Cc:     piotrgorski@cachyos.org, oleksandr@natalenko.name,
        arjan@linux.intel.com, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org,
        pbonzini@redhat.com, paulmck@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
        thomas.lendacky@amd.com, seanjc@google.com, pmenzel@molgen.mpg.de,
        fam.zheng@bytedance.com, punit.agrawal@bytedance.com,
        simon.evans@bytedance.com, liangma@liangbit.com,
        gpiccoli@igalia.com
Subject: Re: [PATCH v16 3/8] cpu/hotplug: Add dynamic parallel bringup
 states before CPUHP_BRINGUP_CPU
In-Reply-To: <115b39e0226915b8f69ea0cce2487588f6010995.camel@infradead.org>
References: <20230321194008.785922-1-usama.arif@bytedance.com>
 <20230321194008.785922-4-usama.arif@bytedance.com> <874jqb8588.ffs@tglx>
 <871qlf83wj.ffs@tglx>
 <8dff6ae5ffaebfbcc55a01c04420fd478070b830.camel@infradead.org>
 <87v8ir6j96.ffs@tglx>
 <115b39e0226915b8f69ea0cce2487588f6010995.camel@infradead.org>
Date:   Fri, 24 Mar 2023 15:07:42 +0100
Message-ID: <87lejm6y4x.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Fri, Mar 24 2023 at 09:31, David Woodhouse wrote:
> On Fri, 2023-03-24 at 02:16 +0100, Thomas Gleixner wrote:
>> But that's non-trivial because if you look at bringup_cpu() then you'll
>> notice that this state has an implicit protection against interrupt
>> allocation/free and quite some architectures rely on this for their
>> early bringup code and possibly their STARTING state callbacks.
>
> I literally pointed that out in 2021 (in one of the messages I
> reference below).
>
> For x86 I don't believe that's going to be an issue yet, if at all. I
> think it matters for the lapic_online() call which happens near the end
> of start_secondary(); it's almost the last thing it does before going
> into the generic AP startup thread. It's *also* preceded by a comment
> that at least *suggests* it ought to be fine anyway, although we should
> never entirely trust such comments.
>
>         /*
>          * Lock vector_lock, set CPU online and bring the vector
>          * allocator online. Online must be set with vector_lock held
>          * to prevent a concurrent irq setup/teardown from seeing a
>          * half valid vector space.

That setup/teardown wording is misleading as that's already covered by
sparse_irq_lock.

This is purely x86 specific. Setting the CPU online and onlining the CPU
in the matrix allocator has to be atomic vs. concurrent allocations from
the matrix allocator, which can happen via request/free_irq() and
affinity changes independent of interrupt setup/teardown (e.g. MSI enable).

Thanks,

        tglx