From: Hector Martin <marcan@marcan.st>
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org,
Marc Zyngier <maz@kernel.org>, Rob Herring <robh@kernel.org>,
Arnd Bergmann <arnd@kernel.org>, Olof Johansson <olof@lixom.net>,
Krzysztof Kozlowski <krzk@kernel.org>,
Mark Kettenis <mark.kettenis@xs4all.nl>,
Tony Lindgren <tony@atomide.com>,
Mohamed Mediouni <mohamed.mediouni@caramail.com>,
Stan Skowronek <stan@corellium.com>,
Alexander Graf <graf@amazon.com>, Will Deacon <will@kernel.org>,
Linus Walleij <linus.walleij@linaro.org>,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync
Date: Thu, 18 Feb 2021 21:51:40 +0900 [thread overview]
Message-ID: <e88fa913-e2ae-35d0-056f-2a5079babbc2@marcan.st> (raw)
In-Reply-To: <20210217122200.GC5556@C02TD0UTHF1T.local>
On 17/02/2021 21.22, Mark Rutland wrote:
>> Root irqchip drivers can discriminate between IRQs and FIQs by checking
>> the ISR_EL1 system register.
>
> I think we can remove this note for now. If we go with seperate handlers
> this won't be necessary, and if not this would be better placed on a
> commit adding the FIQ handling capability.
Indeed, this doesn't make sense any more. Changed for v3.
> Judging by `git grep -Wi daif -- arch/arm64`, with this patch applied,
> we'll also need fixups in:
>
> * gic_arch_enable_irqs() in arch/arm64/include/asm/arch_gicv3.h
> * save_and_disable_irq() in arch/arm64/include/asm/assembler.h (noted below)
> * local_daif_save_flags() in arch/arm64/include/asm/daifflags.h
> (the fake DAIF should have F set too)
> * __cpu_do_idle_irqprio() in arch/arm64/kernel/process.c
Good catches. A few of those are irrelevant for M1 but need to be done
now that we're making this change globally, others I just missed from
the beginning.
There's also an incorrect comment in entry.S:
/*
* DA_F were cleared at start of handling. If anything is set in
* DAIF, we come back from an NMI, so skip preemption
*/
mrs x0, daif
orr x24, x24, x0
Now only DA__ are cleared. This actually pairs with
gic_arch_enable_irqs() and begs the question: in priority masking
systems, do we unmask both IRQ and FIQ (the gic_arch_enable_irqs
change), or do we leave FIQ masked (which instead would need an AND in
that part of entry.S so as to not consider FIQ masked as meaning we're
coming back from an NMI)?
And a minor related one: should init_gic_priority_masking() WARN if FIQ
is masked too? This probably goes with the above.
Either way, this was nontrivial to make sense of, so I'll make that
entry.S comment clearer while I'm touching it.
> I think save_and_diable_irq below needs to be updated too, since it
> only sets DAIF.I and leaves DAIF.F as-is.
Totally missed this one! Fixed for v3.
>> - * FIQ is never expected, but we mask it when we disable debug exceptions, and
>> - * unmask it at all other times.
>> + * FIQ is never expected on most platforms, but we keep it synchronized
>> + * with the IRQ mask status. On platforms that do not expect FIQ, that vector
>> + * triggers a kernel panic. On platforms that do, the FIQ vector is unified
>> + * with the IRQ vector.
>> */
>
> Can we please delete this bit, though? Now that we say IRQ and FIQ are
> masked/unmasked together, I don't think the rest is necessary to
> understand the masking logic, and it's one less thing to keep in sync
> with changes to the entry code.
Gone :)
--
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub
WARNING: multiple messages have this Message-ID (diff)
From: Hector Martin <marcan@marcan.st>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Arnd Bergmann <arnd@kernel.org>, Rob Herring <robh@kernel.org>,
Tony Lindgren <tony@atomide.com>, Marc Zyngier <maz@kernel.org>,
Linus Walleij <linus.walleij@linaro.org>,
linux-kernel@vger.kernel.org,
Krzysztof Kozlowski <krzk@kernel.org>,
devicetree@vger.kernel.org, Alexander Graf <graf@amazon.com>,
Olof Johansson <olof@lixom.net>,
Mohamed Mediouni <mohamed.mediouni@caramail.com>,
Stan Skowronek <stan@corellium.com>,
Will Deacon <will@kernel.org>,
linux-arm-kernel@lists.infradead.org,
Mark Kettenis <mark.kettenis@xs4all.nl>
Subject: Re: [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync
Date: Thu, 18 Feb 2021 21:51:40 +0900 [thread overview]
Message-ID: <e88fa913-e2ae-35d0-056f-2a5079babbc2@marcan.st> (raw)
In-Reply-To: <20210217122200.GC5556@C02TD0UTHF1T.local>
On 17/02/2021 21.22, Mark Rutland wrote:
>> Root irqchip drivers can discriminate between IRQs and FIQs by checking
>> the ISR_EL1 system register.
>
> I think we can remove this note for now. If we go with seperate handlers
> this won't be necessary, and if not this would be better placed on a
> commit adding the FIQ handling capability.
Indeed, this doesn't make sense any more. Changed for v3.
> Judging by `git grep -Wi daif -- arch/arm64`, with this patch applied,
> we'll also need fixups in:
>
> * gic_arch_enable_irqs() in arch/arm64/include/asm/arch_gicv3.h
> * save_and_disable_irq() in arch/arm64/include/asm/assembler.h (noted below)
> * local_daif_save_flags() in arch/arm64/include/asm/daifflags.h
> (the fake DAIF should have F set too)
> * __cpu_do_idle_irqprio() in arch/arm64/kernel/process.c
Good catches. A few of those are irrelevant for M1 but need to be done
now that we're making this change globally, others I just missed from
the beginning.
There's also an incorrect comment in entry.S:
/*
* DA_F were cleared at start of handling. If anything is set in
* DAIF, we come back from an NMI, so skip preemption
*/
mrs x0, daif
orr x24, x24, x0
Now only DA__ are cleared. This actually pairs with
gic_arch_enable_irqs() and begs the question: in priority masking
systems, do we unmask both IRQ and FIQ (the gic_arch_enable_irqs
change), or do we leave FIQ masked (which instead would need an AND in
that part of entry.S so as to not consider FIQ masked as meaning we're
coming back from an NMI)?
And a minor related one: should init_gic_priority_masking() WARN if FIQ
is masked too? This probably goes with the above.
Either way, this was nontrivial to make sense of, so I'll make that
entry.S comment clearer while I'm touching it.
> I think save_and_diable_irq below needs to be updated too, since it
> only sets DAIF.I and leaves DAIF.F as-is.
Totally missed this one! Fixed for v3.
>> - * FIQ is never expected, but we mask it when we disable debug exceptions, and
>> - * unmask it at all other times.
>> + * FIQ is never expected on most platforms, but we keep it synchronized
>> + * with the IRQ mask status. On platforms that do not expect FIQ, that vector
>> + * triggers a kernel panic. On platforms that do, the FIQ vector is unified
>> + * with the IRQ vector.
>> */
>
> Can we please delete this bit, though? Now that we say IRQ and FIQ are
> masked/unmasked together, I don't think the rest is necessary to
> understand the masking logic, and it's one less thing to keep in sync
> with changes to the entry code.
Gone :)
--
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2021-02-18 15:05 UTC|newest]
Thread overview: 185+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-15 12:16 [PATCH v2 00/25] Apple M1 SoC platform bring-up Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 01/25] dt-bindings: vendor-prefixes: Add apple prefix Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 17:44 ` Krzysztof Kozlowski
2021-02-15 17:44 ` Krzysztof Kozlowski
2021-03-02 8:21 ` Linus Walleij
2021-03-02 8:21 ` Linus Walleij
2021-02-15 12:16 ` [PATCH v2 02/25] dt-bindings: arm: apple: Add bindings for Apple ARM platforms Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 17:48 ` Krzysztof Kozlowski
2021-02-15 17:48 ` Krzysztof Kozlowski
2021-02-16 14:30 ` Hector Martin
2021-02-16 14:30 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 03/25] dt-bindings: arm: cpus: Add apple,firestorm & icestorm compatibles Hector Martin
2021-02-15 12:16 ` [PATCH v2 03/25] dt-bindings: arm: cpus: Add apple, firestorm " Hector Martin
2021-02-15 12:16 ` [PATCH v2 04/25] arm64: cputype: Add CPU implementor & types for the Apple M1 cores Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 05/25] dt-bindings: timer: arm,arch_timer: Add interrupt-names support Hector Martin
2021-02-15 12:16 ` [PATCH v2 05/25] dt-bindings: timer: arm, arch_timer: " Hector Martin
2021-02-15 12:16 ` [PATCH v2 06/25] arm64: arch_timer: implement support for interrupt-names Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 13:28 ` Marc Zyngier
2021-02-15 13:28 ` Marc Zyngier
2021-02-15 15:13 ` Hector Martin
2021-02-15 15:13 ` Hector Martin
2021-02-15 18:23 ` Tony Lindgren
2021-02-15 18:23 ` Tony Lindgren
2021-02-16 14:33 ` Hector Martin
2021-02-16 14:33 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 07/25] arm64: cpufeature: Add a feature for FIQ support Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 08/25] arm64: Always keep DAIF.[IF] in sync Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-17 12:22 ` Mark Rutland
2021-02-17 12:22 ` Mark Rutland
2021-02-18 12:51 ` Hector Martin [this message]
2021-02-18 12:51 ` Hector Martin
2021-02-18 14:22 ` Mark Rutland
2021-02-18 14:22 ` Mark Rutland
2021-02-18 14:42 ` Hector Martin
2021-02-18 14:42 ` Hector Martin
2021-02-18 15:26 ` Mark Rutland
2021-02-18 15:26 ` Mark Rutland
2021-02-15 12:16 ` [PATCH v2 09/25] arm64: entry: Map the FIQ vector to IRQ on NEEDS_FIQ platforms Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-17 11:49 ` Mark Rutland
2021-02-17 11:49 ` Mark Rutland
2021-02-17 14:38 ` Marc Zyngier
2021-02-17 14:38 ` Marc Zyngier
2021-02-15 12:16 ` [PATCH v2 10/25] asm-generic/io.h: Add a non-posted variant of ioremap() Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 15:27 ` kernel test robot
2021-02-15 15:27 ` kernel test robot
2021-02-15 16:47 ` Hector Martin
2021-02-16 10:53 ` Christoph Hellwig
2021-02-16 10:53 ` Christoph Hellwig
2021-02-18 13:08 ` Hector Martin
2021-02-18 13:08 ` Hector Martin
2021-02-15 12:16 ` [PATCH v2 11/25] arm64: Implement ioremap_np() to map MMIO as nGnRnE Hector Martin
2021-02-15 12:16 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 12/25] of/address: Add infrastructure to declare MMIO as non-posted Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 13/25] arm64: Add Apple vendor-specific system registers Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 14/25] dt-bindings: interrupt-controller: Add DT bindings for apple-aic Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-16 9:41 ` Arnd Bergmann
2021-02-16 9:41 ` Arnd Bergmann
2021-02-16 11:00 ` Mark Kettenis
2021-02-16 11:00 ` Mark Kettenis
2021-02-16 11:21 ` Arnd Bergmann
2021-02-16 11:21 ` Arnd Bergmann
2021-02-16 11:45 ` Marc Zyngier
2021-02-16 11:45 ` Marc Zyngier
2021-03-02 8:47 ` Linus Walleij
2021-03-02 8:47 ` Linus Walleij
2021-02-15 12:17 ` [PATCH v2 15/25] irqchip/apple-aic: Add support for the Apple Interrupt Controller Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 18:09 ` Marc Zyngier
2021-02-15 18:09 ` Marc Zyngier
2021-02-22 19:35 ` Hector Martin
2021-02-22 19:35 ` Hector Martin
2021-02-23 17:37 ` Marc Zyngier
2021-02-23 17:37 ` Marc Zyngier
2021-02-15 12:17 ` [PATCH v2 16/25] arm64: Kconfig: Introduce CONFIG_ARCH_APPLE Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 17/25] tty: serial: samsung_tty: Separate S3C64XX ops structure Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 18:06 ` Krzysztof Kozlowski
2021-02-15 18:06 ` Krzysztof Kozlowski
2021-02-18 13:24 ` Hector Martin
2021-02-18 13:24 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 18/25] tty: serial: samsung_tty: add s3c24xx_port_type Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 18:26 ` Krzysztof Kozlowski
2021-02-15 18:26 ` Krzysztof Kozlowski
2021-02-18 13:37 ` Hector Martin
2021-02-18 13:37 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 19/25] tty: serial: samsung_tty: IRQ rework Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 18:40 ` Krzysztof Kozlowski
2021-02-15 18:40 ` Krzysztof Kozlowski
2021-02-18 13:53 ` Hector Martin
2021-02-18 13:53 ` Hector Martin
2021-02-20 19:11 ` Krzysztof Kozlowski
2021-02-20 19:11 ` Krzysztof Kozlowski
2021-02-21 13:43 ` Hector Martin
2021-02-21 13:43 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 20/25] tty: serial: samsung_tty: Use devm_ioremap_resource Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 18:51 ` Krzysztof Kozlowski
2021-02-15 18:51 ` Krzysztof Kozlowski
2021-02-18 14:01 ` Hector Martin
2021-02-18 14:01 ` Hector Martin
2021-02-20 19:13 ` Krzysztof Kozlowski
2021-02-20 19:13 ` Krzysztof Kozlowski
2021-02-20 19:17 ` Marc Zyngier
2021-02-20 19:17 ` Marc Zyngier
2021-02-21 14:38 ` Hector Martin
2021-02-21 14:38 ` Hector Martin
2021-02-21 14:59 ` Marc Zyngier
2021-02-21 14:59 ` Marc Zyngier
2021-02-21 17:09 ` Hector Martin
2021-02-21 17:09 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 21/25] dt-bindings: serial: samsung: Add apple,s5l-uart compatible Hector Martin
2021-02-15 12:17 ` [PATCH v2 21/25] dt-bindings: serial: samsung: Add apple, s5l-uart compatible Hector Martin
2021-02-15 18:53 ` [PATCH v2 21/25] dt-bindings: serial: samsung: Add apple,s5l-uart compatible Krzysztof Kozlowski
2021-02-15 18:53 ` Krzysztof Kozlowski
2021-03-02 8:31 ` Linus Walleij
2021-03-02 8:31 ` [PATCH v2 21/25] dt-bindings: serial: samsung: Add apple, s5l-uart compatible Linus Walleij
2021-02-15 12:17 ` [PATCH v2 22/25] tty: serial: samsung_tty: Add support for Apple UARTs Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 19:13 ` Krzysztof Kozlowski
2021-02-15 19:13 ` Krzysztof Kozlowski
2021-02-18 14:16 ` Hector Martin
2021-02-18 14:16 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 23/25] tty: serial: samsung_tty: Add earlycon " Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 19:17 ` Krzysztof Kozlowski
2021-02-15 19:17 ` Krzysztof Kozlowski
2021-02-16 10:18 ` Arnd Bergmann
2021-02-16 10:18 ` Arnd Bergmann
2021-02-16 10:20 ` Krzysztof Kozlowski
2021-02-16 10:20 ` Krzysztof Kozlowski
2021-02-16 10:29 ` Arnd Bergmann
2021-02-16 10:29 ` Arnd Bergmann
2021-02-16 10:50 ` Hector Martin
2021-02-16 10:50 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 24/25] dt-bindings: display: Add apple,simple-framebuffer Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 12:17 ` [PATCH v2 25/25] arm64: apple: Add initial Mac Mini 2020 (M1) devicetree Hector Martin
2021-02-15 12:17 ` Hector Martin
2021-02-15 19:29 ` Krzysztof Kozlowski
2021-02-15 19:29 ` Krzysztof Kozlowski
2021-02-15 21:00 ` Randy Dunlap
2021-02-15 21:00 ` Randy Dunlap
2021-02-16 7:31 ` Krzysztof Kozlowski
2021-02-16 7:31 ` Krzysztof Kozlowski
2021-02-21 14:43 ` Hector Martin
2021-02-21 14:43 ` Hector Martin
2021-02-21 15:32 ` Krzysztof Kozlowski
2021-02-21 15:32 ` Krzysztof Kozlowski
2021-02-15 12:57 ` [PATCH v2 00/25] Apple M1 SoC platform bring-up Arnd Bergmann
2021-02-15 12:57 ` Arnd Bergmann
2021-02-15 13:22 ` gregkh
2021-02-15 13:22 ` gregkh
2021-02-15 15:57 ` Hector Martin
2021-02-15 15:57 ` Hector Martin
2021-02-15 16:12 ` gregkh
2021-02-15 16:12 ` gregkh
2021-02-15 16:54 ` Hector Martin
2021-02-15 16:54 ` Hector Martin
2021-02-15 17:43 ` Krzysztof Kozlowski
2021-02-15 17:43 ` Krzysztof Kozlowski
2021-02-15 19:11 ` Marc Zyngier
2021-02-15 19:11 ` Marc Zyngier
[not found] ` <CAHp75Vd2ObiUJFn-kVWBx+E30my9zXVX5iUtsyRb_c4FcZEDOA@mail.gmail.com>
2021-02-23 9:11 ` Hector Martin
2021-02-23 9:11 ` Hector Martin
2021-02-18 14:36 ` Mark Rutland
2021-02-18 14:36 ` Mark Rutland
2021-02-21 15:20 ` Hector Martin
2021-02-21 15:20 ` Hector Martin
2021-02-24 15:55 ` Hector Martin
2021-02-24 15:55 ` Hector Martin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e88fa913-e2ae-35d0-056f-2a5079babbc2@marcan.st \
--to=marcan@marcan.st \
--cc=arnd@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=graf@amazon.com \
--cc=krzk@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.kettenis@xs4all.nl \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=mohamed.mediouni@caramail.com \
--cc=olof@lixom.net \
--cc=robh@kernel.org \
--cc=stan@corellium.com \
--cc=tony@atomide.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.