linux-kernel - Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple AIC driver.

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]

Date:   Thu, 21 Jan 2021 13:50:47 +0100
From:   Mohamed Mediouni <mohamed.mediouni@...amail.com>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Mark Rutland <mark.rutland@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Hector Martin <marcan@...can.st>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
        Stan Skowronek <stan@...ellium.com>
Subject: Re: [RFC PATCH 7/7] irqchip/apple-aic: add SMP support to the Apple
 AIC driver.



> On 21 Jan 2021, at 13:44, Arnd Bergmann <arnd@...nel.org> wrote:
> 
> On Wed, Jan 20, 2021 at 2:27 PM Mohamed Mediouni
> <mohamed.mediouni@...amail.com> wrote:
> 
>> +#ifdef CONFIG_SMP
>> +static void apple_aic_ipi_send_mask(struct irq_data *d,
>> +                                   const struct cpumask *mask)
> 
> Not sure we care about the #ifdef here, given that arch/arm64 does not
> allow building a kernel without CONFIG_SMP.
> 
>> +       /*
>> +     * Ensure that stores to Normal memory are visible to the
>> +     * other CPUs before issuing the IPI.
>> +     */
>> +       wmb();
>> +
>> +       for_each_cpu (cpu, mask) {
>> +               smp_mb__before_atomic();
>> +               atomic_or(1u << irqnr, per_cpu_ptr(&aic_ipi_mask, cpu));
>> +               smp_mb__after_atomic();
>> +               lcpu = get_cpu();
>> +               if (aic.fast_ipi) {
>> +                       if ((lcpu >> 2) == (cpu >> 2))
>> +                               write_sysreg(cpu & 3, SR_APPLE_IPI_LOCAL);
>> +                       else
>> +                               write_sysreg((cpu & 3) | ((cpu >> 2) << 16),
>> +                                            SR_APPLE_IPI_REMOTE);
>> +               } else
>> +                       writel(lcpu == cpu ? REG_IPI_FLAG_SELF :
>> +                                                  (REG_IPI_FLAG_OTHER << cpu),
>> +                              aic.base + REG_IPI_SET);
>> +               put_cpu();
>> +       }
>> +
>> +       /* Force the above writes to be executed */
>> +       if (aic.fast_ipi)
>> +               isb();
>> +}
> 
> Since this just loops over all CPUs, I'd probably just turn it into
> an ipi_send_single() callback and have the caller do the
> loop for simplicity.
> 
> I also have the feeling that splitting one hardware IPI into multiple
> logical interrupts, which are then all registered by the same irq
> handler adds a little more complexity than necessary.
> 
> Changing this would of course require modifications to
> arch/arm64/kernel/smp.c, which is hardwired to use
> CONFIG_GENERIC_IRQ_IPI in smp_cross_call(), and allowing
> a different code path there may be worse than emulating an
> irqchip.
> 
>> @@ -186,8 +325,11 @@ static int __init apple_aic_init(struct device_node *node,
>>       if (WARN(!aic.base, "unable to map aic registers\n"))
>>               return -EINVAL;
>> 
>> +       aic.fast_ipi = of_property_read_bool(node, "fast-ipi");
> 
> Where is this property documented, and what decides which one to use?
It’s getting documented in the next patch set.

This property is there to enable support for older iPhone processors
later on, some of which do not have fast IPI support.

On Apple M1, fast-ipi is always on.

Thank you,
>       Arnd