Re: [PATCH 0/8] arm64: Support FIQ controller registration
On 2021-02-24 14:06, Mark Rutland wrote: On Fri, Feb 19, 2021 at 06:10:56PM +, Marc Zyngier wrote: Hi Mark, On Fri, 19 Feb 2021 11:38:56 +, Mark Rutland wrote: > > Hector's M1 support series [1] shows that some platforms have critical > interrupts wired to FIQ, and to support these platforms we need to support > handling FIQ exceptions. Other contemporary platforms don't use FIQ (since e.g. > this is usually routed to EL3), and as we never expect to take an FIQ, we have > the FIQ vector cause a panic. > > Since the use of FIQ is a platform integration detail (which can differ across > bare-metal and virtualized environments), we need be able to explicitly opt-in > to handling FIQs while retaining the existing behaviour otherwise. This series > adds a new set_handle_fiq() hook so that the FIQ controller can do so, and > where no controller is registered the default handler will panic(). For > consistency the set_handle_irq() code is made to do the same. > > The first couple of patches are from Marc's irq/drop-generic_irq_multi_handler > branch [2] on kernel.org, and clean up CONFIG_GENERIC_IRQ_MULTI_HANDLER usage. > The next four patches move arm64 over to a local set_handle_irq() > implementation, which is written to share code with a set_handle_fiq() function > in the last two patches. The only functional difference here is that if an IRQ > is somehow taken prior to set_handle_irq() the default handler will directly > panic() rather than the vector branching to NULL. > > The penultimate patch is cherry-picked from the v2 M1 series, and as per > discussion there [3] will need a few additional fixups. I've included it for > now as the DAIF.IF alignment is necessary for the FIQ exception handling added > in the final patch. > > The final patch adds the low-level FIQ exception handling and registration > mechanism atop the prior rework. Thanks for putting this together. I have an extra patch on top of this series[1] that prevents the kernel from catching fire if a FIQ fires whilst running a guest. Nothing urgent, we can queue it at a later time. Thanks, M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/fiq IIUC for that "invalid_vect" should be changed to "valid_vect", to match the other valid vector entries, but otherwise that looks sane to me. Err, yes. I though I had fixed that, but obviously didn't. I guess we could take that as a prerequisite ahead of the rest? Sure, that's mostly independent anyway. And it would make more sense for an unexpected FIQ to crash the host at at the point where we handle interrupts rather than in KVM with very little debug information. Thanks, M. -- Jazz is not dead. It just smells funny...
Re: [PATCH 0/8] arm64: Support FIQ controller registration
On Fri, Feb 19, 2021 at 06:10:56PM +, Marc Zyngier wrote: > Hi Mark, > > On Fri, 19 Feb 2021 11:38:56 +, > Mark Rutland wrote: > > > > Hector's M1 support series [1] shows that some platforms have critical > > interrupts wired to FIQ, and to support these platforms we need to support > > handling FIQ exceptions. Other contemporary platforms don't use FIQ (since > > e.g. > > this is usually routed to EL3), and as we never expect to take an FIQ, we > > have > > the FIQ vector cause a panic. > > > > Since the use of FIQ is a platform integration detail (which can differ > > across > > bare-metal and virtualized environments), we need be able to explicitly > > opt-in > > to handling FIQs while retaining the existing behaviour otherwise. This > > series > > adds a new set_handle_fiq() hook so that the FIQ controller can do so, and > > where no controller is registered the default handler will panic(). For > > consistency the set_handle_irq() code is made to do the same. > > > > The first couple of patches are from Marc's > > irq/drop-generic_irq_multi_handler > > branch [2] on kernel.org, and clean up CONFIG_GENERIC_IRQ_MULTI_HANDLER > > usage. > > The next four patches move arm64 over to a local set_handle_irq() > > implementation, which is written to share code with a set_handle_fiq() > > function > > in the last two patches. The only functional difference here is that if an > > IRQ > > is somehow taken prior to set_handle_irq() the default handler will directly > > panic() rather than the vector branching to NULL. > > > > The penultimate patch is cherry-picked from the v2 M1 series, and as per > > discussion there [3] will need a few additional fixups. I've included it for > > now as the DAIF.IF alignment is necessary for the FIQ exception handling > > added > > in the final patch. > > > > The final patch adds the low-level FIQ exception handling and registration > > mechanism atop the prior rework. > > Thanks for putting this together. I have an extra patch on top of this > series[1] that prevents the kernel from catching fire if a FIQ fires > whilst running a guest. Nothing urgent, we can queue it at a later time. > > Thanks, > > M. > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/fiq IIUC for that "invalid_vect" should be changed to "valid_vect", to match the other valid vector entries, but otherwise that looks sane to me. I guess we could take that as a prerequisite ahead of the rest? Thanks, Mark.
Re: [PATCH 0/8] arm64: Support FIQ controller registration
Hi Mark, On Fri, 19 Feb 2021 11:38:56 +, Mark Rutland wrote: > > Hector's M1 support series [1] shows that some platforms have critical > interrupts wired to FIQ, and to support these platforms we need to support > handling FIQ exceptions. Other contemporary platforms don't use FIQ (since > e.g. > this is usually routed to EL3), and as we never expect to take an FIQ, we have > the FIQ vector cause a panic. > > Since the use of FIQ is a platform integration detail (which can differ across > bare-metal and virtualized environments), we need be able to explicitly opt-in > to handling FIQs while retaining the existing behaviour otherwise. This series > adds a new set_handle_fiq() hook so that the FIQ controller can do so, and > where no controller is registered the default handler will panic(). For > consistency the set_handle_irq() code is made to do the same. > > The first couple of patches are from Marc's irq/drop-generic_irq_multi_handler > branch [2] on kernel.org, and clean up CONFIG_GENERIC_IRQ_MULTI_HANDLER usage. > The next four patches move arm64 over to a local set_handle_irq() > implementation, which is written to share code with a set_handle_fiq() > function > in the last two patches. The only functional difference here is that if an IRQ > is somehow taken prior to set_handle_irq() the default handler will directly > panic() rather than the vector branching to NULL. > > The penultimate patch is cherry-picked from the v2 M1 series, and as per > discussion there [3] will need a few additional fixups. I've included it for > now as the DAIF.IF alignment is necessary for the FIQ exception handling added > in the final patch. > > The final patch adds the low-level FIQ exception handling and registration > mechanism atop the prior rework. Thanks for putting this together. I have an extra patch on top of this series[1] that prevents the kernel from catching fire if a FIQ fires whilst running a guest. Nothing urgent, we can queue it at a later time. Thanks, M. [1] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/fiq -- Without deviation from the norm, progress is not possible.
Re: [PATCH 0/8] arm64: Support FIQ controller registration
On Sat, Feb 20, 2021 at 12:41:01AM +0900, Hector Martin wrote: > Hi Mark, > > Thanks for tackling this side of the problem! No problem -- I have a vested interest in the arm64 exception management code lookin the way I expect/prefer! ;) > On 19/02/2021 20.38, Mark Rutland wrote: > > I'm hoping that we can somehow queue the first 6 patches of this series as a > > base for the M1 support. With that we can either cherry-pick a later > > version of > > the DAIF.IF patch here, or the M1 support series can take the FIQ handling > > patch. I've pushed the series out to my arm64/fiq branch [4] on kernel.org, > > atop v5.11. > > Looks good! I cherry picked my updated version of the DAIF.IF patch into > your series at [1] (3322522d), and then rebased the M1 series on top of it > (with the change to use set_handle_fiq(), minus all the other obsoleted FIQ > stuff) at [2]. It all boots and works as expected. > > I think it makes sense for you to take the DAIF.IF patch, as it goes along > with this series. Then we can base the M1 series off of it. Sure; that works for me! > If you think that works, I can send it off as a one-off reply to the > version in this series and we can review it here if you want, or > otherwise feel free to cherry-pick it into a v2 (CC as appropriate). If you could do a one-off reply, that'd be fantastic -- that way lore.kernel.org will archive it and it gives people a chance to provide any tags or comments before the next respin of the whole series. > If this all makes sense, the v3 of the M1 series will then be based off of > this patchset as in [2], and I'll link to your tree in the cover letter so > others know where to apply it. As a heads-up, I'm currently treating my arm64/fiq branch as unstable (and I've already applied a typo fix since this posting), but I can tag versions of that to make it possible to refer to a specific version. I'll make sure to do that once I fold in the new DAIF.[IF] patch, since I assume that's the first version worth noting as a base. > Arnd (CCed) is going to be merging that one via the SoC tree, so as > long as we coordinate a stable base once everything is reviewed and > ready to merge, I believe it should all work out fine on the way up. That sounds about right to me. I think the first step is for Marc and I to figure out how the core IRQ bits go in (some of that might be an fix early in the current v5.12 cycle), and I'd expect to have a stable branch atop somewhere between v5.12-rc1 and v5.12-rc4. For context, usually the arm64 core bits get based on the previous rc3/rc4. Thanks, Mark. > Just for completeness, the current DAIF.IF patch in the context of the > original series is at [3] (4dd6330f), in case that's useful to someone for > some reason (since there were conflicts due to the refactoring happening > before it, it changed a bit). > > [1] https://github.com/AsahiLinux/linux/tree/fiq > [2] https://github.com/AsahiLinux/linux/tree/upstream-bringup-v3 > [3] https://github.com/AsahiLinux/linux/tree/upstream-bringup-v2.5 > > -- > Hector Martin (mar...@marcan.st) > Public Key: https://mrcn.st/pub
Re: [PATCH 0/8] arm64: Support FIQ controller registration
Hi Mark, Thanks for tackling this side of the problem! On 19/02/2021 20.38, Mark Rutland wrote: The only functional difference here is that if an IRQ is somehow taken prior to set_handle_irq() the default handler will directly panic() rather than the vector branching to NULL. That sounds like the right thing to do, certainly. The penultimate patch is cherry-picked from the v2 M1 series, and as per discussion there [3] will need a few additional fixups. I've included it for now as the DAIF.IF alignment is necessary for the FIQ exception handling added in the final patch. The final patch adds the low-level FIQ exception handling and registration mechanism atop the prior rework. I'm hoping that we can somehow queue the first 6 patches of this series as a base for the M1 support. With that we can either cherry-pick a later version of the DAIF.IF patch here, or the M1 support series can take the FIQ handling patch. I've pushed the series out to my arm64/fiq branch [4] on kernel.org, atop v5.11. Looks good! I cherry picked my updated version of the DAIF.IF patch into your series at [1] (3322522d), and then rebased the M1 series on top of it (with the change to use set_handle_fiq(), minus all the other obsoleted FIQ stuff) at [2]. It all boots and works as expected. I think it makes sense for you to take the DAIF.IF patch, as it goes along with this series. Then we can base the M1 series off of it. If you think that works, I can send it off as a one-off reply to the version in this series and we can review it here if you want, or otherwise feel free to cherry-pick it into a v2 (CC as appropriate). If this all makes sense, the v3 of the M1 series will then be based off of this patchset as in [2], and I'll link to your tree in the cover letter so others know where to apply it. Arnd (CCed) is going to be merging that one via the SoC tree, so as long as we coordinate a stable base once everything is reviewed and ready to merge, I believe it should all work out fine on the way up. Just for completeness, the current DAIF.IF patch in the context of the original series is at [3] (4dd6330f), in case that's useful to someone for some reason (since there were conflicts due to the refactoring happening before it, it changed a bit). [1] https://github.com/AsahiLinux/linux/tree/fiq [2] https://github.com/AsahiLinux/linux/tree/upstream-bringup-v3 [3] https://github.com/AsahiLinux/linux/tree/upstream-bringup-v2.5 -- Hector Martin (mar...@marcan.st) Public Key: https://mrcn.st/pub
[PATCH 0/8] arm64: Support FIQ controller registration
Hector's M1 support series [1] shows that some platforms have critical interrupts wired to FIQ, and to support these platforms we need to support handling FIQ exceptions. Other contemporary platforms don't use FIQ (since e.g. this is usually routed to EL3), and as we never expect to take an FIQ, we have the FIQ vector cause a panic. Since the use of FIQ is a platform integration detail (which can differ across bare-metal and virtualized environments), we need be able to explicitly opt-in to handling FIQs while retaining the existing behaviour otherwise. This series adds a new set_handle_fiq() hook so that the FIQ controller can do so, and where no controller is registered the default handler will panic(). For consistency the set_handle_irq() code is made to do the same. The first couple of patches are from Marc's irq/drop-generic_irq_multi_handler branch [2] on kernel.org, and clean up CONFIG_GENERIC_IRQ_MULTI_HANDLER usage. The next four patches move arm64 over to a local set_handle_irq() implementation, which is written to share code with a set_handle_fiq() function in the last two patches. The only functional difference here is that if an IRQ is somehow taken prior to set_handle_irq() the default handler will directly panic() rather than the vector branching to NULL. The penultimate patch is cherry-picked from the v2 M1 series, and as per discussion there [3] will need a few additional fixups. I've included it for now as the DAIF.IF alignment is necessary for the FIQ exception handling added in the final patch. The final patch adds the low-level FIQ exception handling and registration mechanism atop the prior rework. I'm hoping that we can somehow queue the first 6 patches of this series as a base for the M1 support. With that we can either cherry-pick a later version of the DAIF.IF patch here, or the M1 support series can take the FIQ handling patch. I've pushed the series out to my arm64/fiq branch [4] on kernel.org, atop v5.11. Thanks, Mark. [1] https://http://lore.kernel.org/r/20210215121713.57687-1-mar...@marcan.st [2] https://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/drop-generic_irq_multi_handler [3] https://lore.kernelo.org/r/20210215121713.57687-9-mar...@marcan.st [4] https://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git/log/?h=arm64/fiq Hector Martin (1): arm64: Always keep DAIF.[IF] in sync Marc Zyngier (5): ARM: ep93xx: Select GENERIC_IRQ_MULTI_HANDLER directly irqchip: Do not blindly select CONFIG_GENERIC_IRQ_MULTI_HANDLER genirq: Allow architectures to override set_handle_irq() fallback arm64: don't use GENERIC_IRQ_MULTI_HANDLER arm64: entry: factor irq triage logic into macros Mark Rutland (2): arm64: irq: add a default handle_irq panic function arm64: irq: allow FIQs to be handled arch/arm/Kconfig | 1 + arch/arm64/Kconfig | 1 - arch/arm64/include/asm/assembler.h | 6 +-- arch/arm64/include/asm/daifflags.h | 4 +- arch/arm64/include/asm/irq.h | 4 ++ arch/arm64/include/asm/irqflags.h | 19 --- arch/arm64/kernel/entry.S | 108 ++--- arch/arm64/kernel/irq.c| 33 +++- drivers/irqchip/Kconfig| 9 include/linux/irq.h| 2 + 10 files changed, 121 insertions(+), 66 deletions(-) -- 2.11.0
