[RFC PATCH v2 38/45] arm64: smp: Abstract SGI and LPI operations
Jinjie Ruan
ruanjinjie at huawei.com
Fri Aug 28 20:47:13 PDT 2026
在 2026/8/13 16:01, Jinjie Ruan 写道:
>
>
> 在 2026/7/28 0:34, Vladimir Murzin 写道:
>> SGI and LPI backed IPIs require different setup, enable, disable and
>> send operations. These differences are currently handled by repeatedly
>> checking percpu_ipi_descs. As the implementation specific logic grows,
>> these checks make the common IPI code increasingly difficult to
>> follow.
>>
>> Introduce an operations structure for each implementation to
>> encapsulate the specific of SGI and LPI handling, leaving the common
>> IPI paths generic.
>>
>> Signed-off-by: Vladimir Murzin <vladimir.murzin at arm.com>
>> ---
>> arch/arm64/kernel/smp.c | 163 +++++++++++++++++++++++++---------------
>> 1 file changed, 101 insertions(+), 62 deletions(-)
>
> Hi, Vladimir,
>
> I believe that the modifications here do not need to be so extensive. By
> maintaining the order of the related function definitions consistent
> with the original, the changes can be reduced to only 126 lines, and the
> modifications will be easier to review.
Hi, Vladimir,
What do you think?
>
> arch/arm64/kernel/smp.c | 126
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------
> 1 file changed, 81 insertions(+), 45 deletions(-)
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 534fc58df62a..b6b020118712 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -75,11 +75,18 @@ static DEFINE_PER_CPU_READ_MOSTLY(struct ipi_descs,
> pcpu_ipi_desc);
>
> #define get_ipi_desc(__cpu, __ipi) (per_cpu_ptr(&pcpu_ipi_desc,
> __cpu)->descs[__ipi])
>
> -static bool percpu_ipi_descs __ro_after_init;
> +struct ipi_irq_ops {
> + void (*setup)(int ipi, int ncpus);
> + void (*enable)(int cpu, int ipi);
> + void (*disable)(int cpu, int ipi);
> + void (*send)(const cpumask_t *mask, unsigned int nr);
> +};
> +
> +static const struct ipi_irq_ops *ipi_ops __ro_after_init;
>
> static bool crash_stop;
>
> -static void ipi_setup(int cpu);
> +static void ipi_enable(int cpu);
>
> #ifdef CONFIG_HOTPLUG_CPU
> static void ipi_teardown(int cpu);
> @@ -240,7 +247,7 @@ asmlinkage notrace void secondary_start_kernel(void)
> */
> notify_cpu_starting(cpu);
>
> - ipi_setup(cpu);
> + ipi_enable(cpu);
>
> numa_add_cpu(cpu);
>
> @@ -916,13 +923,7 @@ static void __noreturn ipi_cpu_crash_stop(unsigned
> int cpu, struct pt_regs *regs
>
> static void arm64_send_ipi(const cpumask_t *mask, unsigned int nr)
> {
> - unsigned int cpu;
> -
> - if (!percpu_ipi_descs)
> - __ipi_send_mask(get_ipi_desc(0, nr), mask);
> - else
> - for_each_cpu(cpu, mask)
> - __ipi_send_single(get_ipi_desc(cpu, nr), cpu);
> + ipi_ops->send(mask, nr);
> }
>
> static void arm64_backtrace_ipi(cpumask_t *mask)
> @@ -1048,25 +1049,15 @@ static bool ipi_should_be_nmi(enum ipi_msg_type ipi)
> }
> }
>
> -static void ipi_setup(int cpu)
> +static void ipi_enable(int cpu)
> {
> int i;
>
> if (WARN_ON_ONCE(!ipi_irq_base))
> return;
>
> - for (i = 0; i < nr_ipi; i++) {
> - if (!percpu_ipi_descs) {
> - if (ipi_should_be_nmi(i)) {
> - prepare_percpu_nmi(ipi_irq_base + i);
> - enable_percpu_nmi(ipi_irq_base + i, 0);
> - } else {
> - enable_percpu_irq(ipi_irq_base + i, 0);
> - }
> - } else {
> - enable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
> - }
> - }
> + for (i = 0; i < nr_ipi; i++)
> + ipi_ops->enable(cpu, i);
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> @@ -1077,25 +1068,18 @@ static void ipi_teardown(int cpu)
> if (WARN_ON_ONCE(!ipi_irq_base))
> return;
>
> - for (i = 0; i < nr_ipi; i++) {
> - if (!percpu_ipi_descs) {
> - if (ipi_should_be_nmi(i)) {
> - disable_percpu_nmi(ipi_irq_base + i);
> - teardown_percpu_nmi(ipi_irq_base + i);
> - } else {
> - disable_percpu_irq(ipi_irq_base + i);
> - }
> - } else {
> - disable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
> - }
> - }
> + for (i = 0; i < nr_ipi; i++)
> + ipi_ops->disable(cpu, i);
> }
> #endif
>
> -static void ipi_setup_sgi(int ipi)
> +static void ipi_sgi_setup(int ipi, int ncpus)
> {
> int err, irq, cpu;
>
> + if (WARN_ON_ONCE(ncpus))
> + return;
> +
> irq = ipi_irq_base + ipi;
>
> if (ipi_should_be_nmi(ipi)) {
> @@ -1112,7 +1096,39 @@ static void ipi_setup_sgi(int ipi)
> irq_set_status_flags(irq, IRQ_HIDDEN);
> }
>
> -static void ipi_setup_lpi(int ipi, int ncpus)
> +static void ipi_sgi_enable(int cpu, int ipi)
> +{
> + if (ipi_should_be_nmi(ipi)) {
> + prepare_percpu_nmi(ipi_irq_base + ipi);
> + enable_percpu_nmi(ipi_irq_base + ipi, 0);
> + } else {
> + enable_percpu_irq(ipi_irq_base + ipi, 0);
> + }
> +}
> +
> +static void ipi_sgi_disable(int cpu, int ipi)
> +{
> + if (ipi_should_be_nmi(ipi)) {
> + disable_percpu_nmi(ipi_irq_base + ipi);
> + teardown_percpu_nmi(ipi_irq_base + ipi);
> + } else {
> + disable_percpu_irq(ipi_irq_base + ipi);
> + }
> +}
> +
> +static void ipi_sgi_send(const cpumask_t *mask, unsigned int nr)
> +{
> + __ipi_send_mask(get_ipi_desc(0, nr), mask);
> +}
> +
> +static const struct ipi_irq_ops ipi_sgi_ops = {
> + .setup = ipi_sgi_setup,
> + .enable = ipi_sgi_enable,
> + .disable = ipi_sgi_disable,
> + .send = ipi_sgi_send,
> +};
> +
> +static void ipi_lpi_setup(int ipi, int ncpus)
> {
> for (int cpu = 0; cpu < ncpus; cpu++) {
> int err, irq;
> @@ -1132,6 +1148,30 @@ static void ipi_setup_lpi(int ipi, int ncpus)
> }
> }
>
> +static void ipi_lpi_enable(int cpu, int ipi)
> +{
> + enable_irq(irq_desc_get_irq(get_ipi_desc(cpu, ipi)));
> +}
> +
> +static void ipi_lpi_disable(int cpu, int ipi)
> +{
> + disable_irq(irq_desc_get_irq(get_ipi_desc(cpu, ipi)));
> +}
> +
> +static void ipi_lpi_send(const cpumask_t *mask, unsigned int nr) {
> + int cpu;
> +
> + for_each_cpu(cpu, mask)
> + __ipi_send_single(get_ipi_desc(cpu, nr), cpu);
> +}
> +
> +static const struct ipi_irq_ops ipi_lpi_ops = {
> + .setup = ipi_lpi_setup,
> + .enable = ipi_lpi_enable,
> + .disable = ipi_lpi_disable,
> + .send = ipi_lpi_send,
> +};
> +
> void __init set_smp_ipi_range_percpu(int ipi_base, int n, int ncpus)
> {
> int i;
> @@ -1139,18 +1179,14 @@ void __init set_smp_ipi_range_percpu(int
> ipi_base, int n, int ncpus)
> WARN_ON(n < MAX_IPI);
> nr_ipi = min(n, MAX_IPI);
>
> - percpu_ipi_descs = !!ncpus;
> + ipi_ops = ncpus ? &ipi_lpi_ops : &ipi_sgi_ops;
> ipi_irq_base = ipi_base;
>
> - for (i = 0; i < nr_ipi; i++) {
> - if (!percpu_ipi_descs)
> - ipi_setup_sgi(i);
> - else
> - ipi_setup_lpi(i, ncpus);
> - }
> + for (i = 0; i < nr_ipi; i++)
> + ipi_ops->setup(i, ncpus);
>
> /* Setup the boot CPU immediately */
> - ipi_setup(smp_processor_id());
> + ipi_enable(smp_processor_id());
> }
>
>
>>
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index 6e5b673613ca..3dd4bc02caed 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -75,11 +75,18 @@ static DEFINE_PER_CPU_READ_MOSTLY(struct ipi_descs, pcpu_ipi_desc);
>>
>> #define get_ipi_desc(__cpu, __ipi) (per_cpu_ptr(&pcpu_ipi_desc, __cpu)->descs[__ipi])
>>
>> -static bool percpu_ipi_descs __ro_after_init;
>> +struct ipi_irq_ops {
>> + void (*setup)(int ipi, int ncpus);
>> + void (*disable)(int cpu, int ipi);
>> + void (*enable)(int cpu, int ipi);
>> + void (*send)(const cpumask_t *mask, unsigned int nr);
>> +};
>
> Could the order of different callbacks in ipi_sgi_ops and ipi_sgi_ops
> consistent with this definition?
>
>> +
>> +static const struct ipi_irq_ops *ipi_ops __ro_after_init;
>>
>> static bool crash_stop;
>>
>> -static void ipi_setup(int cpu);
>> +static void ipi_enable(int cpu);
>>
>> #ifdef CONFIG_HOTPLUG_CPU
>> static void ipi_teardown(int cpu);
>> @@ -240,7 +247,7 @@ asmlinkage notrace void secondary_start_kernel(void)
>> */
>> notify_cpu_starting(cpu);
>>
>> - ipi_setup(cpu);
>> + ipi_enable(cpu);
>>
>> numa_add_cpu(cpu);
>>
>> @@ -916,13 +923,7 @@ static void __noreturn ipi_cpu_crash_stop(unsigned int cpu, struct pt_regs *regs
>>
>> static void arm64_send_ipi(const cpumask_t *mask, unsigned int nr)
>> {
>> - unsigned int cpu;
>> -
>> - if (!percpu_ipi_descs)
>> - __ipi_send_mask(get_ipi_desc(0, nr), mask);
>> - else
>> - for_each_cpu(cpu, mask)
>> - __ipi_send_single(get_ipi_desc(cpu, nr), cpu);
>> + ipi_ops->send(mask, nr);
>> }
>>
>> static void arm64_backtrace_ipi(cpumask_t *mask)
>> @@ -1048,53 +1049,13 @@ static bool ipi_should_be_nmi(enum ipi_msg_type ipi)
>> }
>> }
>>
>> -static void ipi_setup(int cpu)
>> -{
>> - int i;
>> -
>> - if (WARN_ON_ONCE(!ipi_irq_base))
>> - return;
>> -
>> - for (i = 0; i < nr_ipi; i++) {
>> - if (!percpu_ipi_descs) {
>> - if (ipi_should_be_nmi(i)) {
>> - prepare_percpu_nmi(ipi_irq_base + i);
>> - enable_percpu_nmi(ipi_irq_base + i, 0);
>> - } else {
>> - enable_percpu_irq(ipi_irq_base + i, 0);
>> - }
>> - } else {
>> - enable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
>> - }
>> - }
>> -}
>> -
>> -#ifdef CONFIG_HOTPLUG_CPU
>> -static void ipi_teardown(int cpu)
>> +static void ipi_sgi_setup(int ipi, int ncpus)
>> {
>> - int i;
>> + int err, irq, cpu;
>>
>> - if (WARN_ON_ONCE(!ipi_irq_base))
>> + if (WARN_ON_ONCE(ncpus))
>> return;
>>
>> - for (i = 0; i < nr_ipi; i++) {
>> - if (!percpu_ipi_descs) {
>> - if (ipi_should_be_nmi(i)) {
>> - disable_percpu_nmi(ipi_irq_base + i);
>> - teardown_percpu_nmi(ipi_irq_base + i);
>> - } else {
>> - disable_percpu_irq(ipi_irq_base + i);
>> - }
>> - } else {
>> - disable_irq(irq_desc_get_irq(get_ipi_desc(cpu, i)));
>> - }
>> - }
>> -}
>> -#endif
>> -
>> -static void ipi_setup_sgi(int ipi)
>> -{
>> - int err, irq, cpu;
>>
>> irq = ipi_irq_base + ipi;
>
> An extra blank line.
>
>>
>> @@ -1112,7 +1073,50 @@ static void ipi_setup_sgi(int ipi)
>> irq_set_status_flags(irq, IRQ_HIDDEN);
>> }
>>
>> -static void ipi_setup_lpi(int ipi, int ncpus)
>> +static void ipi_sgi_enable(int cpu, int ipi)
>> +{
>> + if (ipi_should_be_nmi(ipi)) {
>> + prepare_percpu_nmi(ipi_irq_base + ipi);
>> + enable_percpu_nmi(ipi_irq_base + ipi, 0);
>> + } else {
>> + enable_percpu_irq(ipi_irq_base + ipi, 0);
>> + }
>> +}
>> +
>> +static void ipi_sgi_disable(int cpu, int ipi)
>> +{
>> + if (ipi_should_be_nmi(ipi)) {
>> + disable_percpu_nmi(ipi_irq_base + ipi);
>> + teardown_percpu_nmi(ipi_irq_base + ipi);
>> + } else {
>> + disable_percpu_irq(ipi_irq_base + ipi);
>> + }
>> +}
>> +
>> +static void ipi_sgi_send(const cpumask_t *mask, unsigned int nr)
>> +{
>> + __ipi_send_mask(get_ipi_desc(0, nr), mask);
>> +}
>> +
>> +static const struct ipi_irq_ops ipi_sgi_ops = {
>> + .disable = ipi_sgi_disable,
>> + .enable = ipi_sgi_enable,
>> + .setup = ipi_sgi_setup,
>> + .send = ipi_sgi_send,
>> +};
>> +
>> +
>
> An extra blank line.
>
> Best regards,
> Jinjie
>
>> +static void ipi_lpi_enable(int cpu, int ipi)
>> +{
>> + enable_irq(irq_desc_get_irq(get_ipi_desc(cpu, ipi)));
>> +}
>> +
>> +static void ipi_lpi_disable(int cpu, int ipi)
>> +{
>> + disable_irq(irq_desc_get_irq(get_ipi_desc(cpu, ipi)));
>> +}
>> +
>> +static void ipi_lpi_setup(int ipi, int ncpus)
>> {
>> for (int cpu = 0; cpu < ncpus; cpu++) {
>> int err, irq;
>> @@ -1132,6 +1136,44 @@ static void ipi_setup_lpi(int ipi, int ncpus)
>> }
>> }
>>
>> +static void ipi_lpi_send(const cpumask_t *mask, unsigned int nr) {
>> + int cpu;
>> +
>> + for_each_cpu(cpu, mask)
>> + __ipi_send_single(get_ipi_desc(cpu, nr), cpu);
>> +}
>> +
>> +static const struct ipi_irq_ops ipi_lpi_ops = {
>> + .disable = ipi_lpi_disable,
>> + .enable = ipi_lpi_enable,
>> + .setup = ipi_lpi_setup,
>> + .send = ipi_lpi_send,
>> +};
>> +
>> +static void ipi_enable(int cpu)
>> +{
>> + int ipi;
>> +
>> + if (WARN_ON_ONCE(!ipi_irq_base))
>> + return;
>> +
>> + for (ipi = 0; ipi < nr_ipi; ipi++)
>> + ipi_ops->enable(cpu, ipi);
>> +}
>> +
>> +#ifdef CONFIG_HOTPLUG_CPU
>> +static void ipi_teardown(int cpu)
>> +{
>> + int ipi;
>> +
>> + if (WARN_ON_ONCE(!ipi_irq_base))
>> + return;
>> +
>> + for (ipi = 0; ipi < nr_ipi; ipi++)
>> + ipi_ops->disable(cpu, ipi);
>> +}
>> +#endif
>> +
>> void __init set_smp_ipi_range_percpu(int ipi_base, int n, int ncpus)
>> {
>> int i;
>> @@ -1139,18 +1181,15 @@ void __init set_smp_ipi_range_percpu(int ipi_base, int n, int ncpus)
>> WARN_ON(n < MAX_IPI);
>> nr_ipi = min(n, MAX_IPI);
>>
>> - percpu_ipi_descs = !!ncpus;
>> ipi_irq_base = ipi_base;
>>
>> - for (i = 0; i < nr_ipi; i++) {
>> - if (!percpu_ipi_descs)
>> - ipi_setup_sgi(i);
>> - else
>> - ipi_setup_lpi(i, ncpus);
>> - }
>> + ipi_ops = ncpus ? &ipi_lpi_ops : &ipi_sgi_ops;
>> +
>> + for (i = 0; i < nr_ipi; i++)
>> + ipi_ops->setup(i, ncpus);
>>
>> /* Setup the boot CPU immediately */
>> - ipi_setup(smp_processor_id());
>> + ipi_enable(smp_processor_id());
>> }
>>
>> void arch_smp_send_reschedule(int cpu)
>
More information about the linux-arm-kernel
mailing list