mbox series

[v2,0/5] arm64: irqchip/gic-v3: Use compiletime constant PMR values

Message ID 20240617111841.2529370-1-mark.rutland@arm.com (mailing list archive)
Headers show
Series arm64: irqchip/gic-v3: Use compiletime constant PMR values | expand

Message

Mark Rutland June 17, 2024, 11:18 a.m. UTC
This series optimizes the way regular IRQs are masked/unmasked when
GICv3 pseudo-NMIs are used, removing the need for a static key in fast
paths by using a priority value chosen dynamically at boot time.

Thomas, would you be happy for this series to go through the arm64 tree?
The key part of the series is the final patch which changes both arm64
and irqchip, and I expect merge conflicts or functional fallout to be
contained to arm64.

The GIC distributor and PMR/RPR can present different views of the
interrupt priority space dependent upon the values of GICD_CTLR.DS and
SCR_EL3.FIQ. Currently we treat the distributor's view of the priority
space as canonical, and when the two differ we change the way we handle
values in the PMR/RPR, using the `gic_nonsecure_priorities` static key
to decide what to do.

This approach works, but it's sub-optimal. When using pseudo-NMI we
manipulate the distributor rarely, and we manipulate the PMR/RPR
registers very frequently in code spread out throughout the kernel (e.g.
local_irq_{save,restore}()). It would be nicer if we could use fixed
values for the PMR/RPR, and dynamically choose the values programmed
into the distributor.

This series reworks the GIC code and arm64 architecture code to allow
the use of compiletime-constant PMR values. This simplifies the logic
for PMR management, and when using pseudo-NMI this results in smaller
and better generated code for saving/restoring the irqflags, saving ~4K
of text for defconfig + CONFIG_PSEUDO_NMI=y.

The first patch add a new REPEAT_BYTE_U32() helper which can be useful
for drivers. The second is a preparatory cleanup which I think makes sense
regardless of the rest of the series. The third and fourth patches
rework the GICv3 code to be able to choose priorities at boot time, and
the final patch makes the actual switch.

I've given this some light testing atop v6.10-rc1 with pseudo-NMI
enabled (with priority debugging), along with lockdep on the following
systems:

* M1SDP Morello board, bare metal
  Where GICD_CTRL.DS=0, SCR_EL3.FIQ=0
  Using shifted (NS) values in the distributor

* M1SDP Morello board, KVM guest
  Where GICD_CTRL.DS=1, SCR_EL3.FIQ=0
  Using unshifted values in the distributor

* ThunderX2, KVM guest
  Where GICD_CTRL.DS=1, SCR_EL3.FIQ=0
  Using unshifted values in the distributor

On ThunderX2 bare-metal there is an existing boot-time hang when using
pseudo-NMI which is not solved by this series. With this series applied,
the logging added in patch 3 reports that GICD_CTRL.DS=1, SCR_EL3.FIQ=0,
and so this should be using the same priorities which are seem to work
in a guest.

Since v1 [1]:
* Add REPEAT_BYTE_U32()
* Add MarcZ's Reviewed-by & Tested-by tags
* Cleanup commit titles
* Fix typos

[1] https://lore.kernel.org/linux-arm-kernel/20240529172116.1313498-1-mark.rutland@arm.com/

Mark.

Mark Rutland (5):
  wordpart.h: Add REPEAT_BYTE_U32()
  irqchip/gic-common: Remove sync_access callback
  irqchip/gic-v3: Make distributor priorities variables
  irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
  arm64: irqchip/gic-v3: Select priorities at boot time

 arch/arm64/include/asm/arch_gicv3.h     |  15 --
 arch/arm64/include/asm/ptrace.h         |  35 +---
 arch/arm64/kernel/image-vars.h          |   5 -
 drivers/irqchip/irq-gic-common.c        |  22 +--
 drivers/irqchip/irq-gic-common.h        |   7 +-
 drivers/irqchip/irq-gic-v3-its.c        |  11 +-
 drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
 drivers/irqchip/irq-gic.c               |  10 +-
 drivers/irqchip/irq-hip04.c             |   6 +-
 include/linux/irqchip/arm-gic-common.h  |   4 -
 include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
 include/linux/irqchip/arm-gic-v3.h      |   2 +-
 include/linux/wordpart.h                |   8 +
 13 files changed, 201 insertions(+), 201 deletions(-)
 create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h

Comments

Catalin Marinas June 21, 2024, 5:26 p.m. UTC | #1
Hi Thomas,

On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
> Mark Rutland (5):
>   wordpart.h: Add REPEAT_BYTE_U32()
>   irqchip/gic-common: Remove sync_access callback
>   irqchip/gic-v3: Make distributor priorities variables
>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
>   arm64: irqchip/gic-v3: Select priorities at boot time
> 
>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
>  arch/arm64/include/asm/ptrace.h         |  35 +---
>  arch/arm64/kernel/image-vars.h          |   5 -
>  drivers/irqchip/irq-gic-common.c        |  22 +--
>  drivers/irqchip/irq-gic-common.h        |   7 +-
>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
>  drivers/irqchip/irq-gic.c               |  10 +-
>  drivers/irqchip/irq-hip04.c             |   6 +-
>  include/linux/irqchip/arm-gic-common.h  |   4 -
>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
>  include/linux/wordpart.h                |   8 +
>  13 files changed, 201 insertions(+), 201 deletions(-)
>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h

Are you ok for these patches to go through the arm64 tree (I can put
them on a stable branch) or you'd rather get them through the irqchip
tree? Either way, I don't expect (major) conflicts with the arm64 tree.

Thanks.
Thomas Gleixner June 22, 2024, 9:29 p.m. UTC | #2
On Fri, Jun 21 2024 at 18:26, Catalin Marinas wrote:
> On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
>> Mark Rutland (5):
>>   wordpart.h: Add REPEAT_BYTE_U32()
>>   irqchip/gic-common: Remove sync_access callback
>>   irqchip/gic-v3: Make distributor priorities variables
>>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
>>   arm64: irqchip/gic-v3: Select priorities at boot time
>> 
>>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
>>  arch/arm64/include/asm/ptrace.h         |  35 +---
>>  arch/arm64/kernel/image-vars.h          |   5 -
>>  drivers/irqchip/irq-gic-common.c        |  22 +--
>>  drivers/irqchip/irq-gic-common.h        |   7 +-
>>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
>>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
>>  drivers/irqchip/irq-gic.c               |  10 +-
>>  drivers/irqchip/irq-hip04.c             |   6 +-
>>  include/linux/irqchip/arm-gic-common.h  |   4 -
>>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
>>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
>>  include/linux/wordpart.h                |   8 +
>>  13 files changed, 201 insertions(+), 201 deletions(-)
>>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h
>
> Are you ok for these patches to go through the arm64 tree (I can put
> them on a stable branch) or you'd rather get them through the irqchip
> tree? Either way, I don't expect (major) conflicts with the arm64 tree.

Take them through your tree with my Acked-by. Yes a branch would be
appreciated just in case.

There is also

      https://lore.kernel.org/all/20240529133446.28446-1-Jonathan.Cameron@huawei.com

which fiddles with the GIC but most of this is not irqchip code. No idea
how that is supposed to find it's way into the tree.

Thanks,

        tglx
Catalin Marinas June 24, 2024, 6:12 p.m. UTC | #3
On Mon, 17 Jun 2024 12:18:36 +0100, Mark Rutland wrote:
> This series optimizes the way regular IRQs are masked/unmasked when
> GICv3 pseudo-NMIs are used, removing the need for a static key in fast
> paths by using a priority value chosen dynamically at boot time.
> 
> Thomas, would you be happy for this series to go through the arm64 tree?
> The key part of the series is the final patch which changes both arm64
> and irqchip, and I expect merge conflicts or functional fallout to be
> contained to arm64.
> 
> [...]

Applied to arm64 (for-next/gic-v3-pmr), thanks!

[1/5] wordpart.h: Add REPEAT_BYTE_U32()
      https://git.kernel.org/arm64/c/118d777c4cb4
[2/5] irqchip/gic-common: Remove sync_access callback
      https://git.kernel.org/arm64/c/e95c64a7fb71
[3/5] irqchip/gic-v3: Make distributor priorities variables
      https://git.kernel.org/arm64/c/a6156e70316b
[4/5] irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
      https://git.kernel.org/arm64/c/d447bf09a401
[5/5] arm64: irqchip/gic-v3: Select priorities at boot time
      https://git.kernel.org/arm64/c/18fdb6348c48
Catalin Marinas June 24, 2024, 6:22 p.m. UTC | #4
On Sat, Jun 22, 2024 at 11:29:50PM +0200, Thomas Gleixner wrote:
> On Fri, Jun 21 2024 at 18:26, Catalin Marinas wrote:
> > On Mon, Jun 17, 2024 at 12:18:36PM +0100, Mark Rutland wrote:
> >> Mark Rutland (5):
> >>   wordpart.h: Add REPEAT_BYTE_U32()
> >>   irqchip/gic-common: Remove sync_access callback
> >>   irqchip/gic-v3: Make distributor priorities variables
> >>   irqchip/gic-v3: Detect GICD_CTRL.DS and SCR_EL3.FIQ earlier
> >>   arm64: irqchip/gic-v3: Select priorities at boot time
> >> 
> >>  arch/arm64/include/asm/arch_gicv3.h     |  15 --
> >>  arch/arm64/include/asm/ptrace.h         |  35 +---
> >>  arch/arm64/kernel/image-vars.h          |   5 -
> >>  drivers/irqchip/irq-gic-common.c        |  22 +--
> >>  drivers/irqchip/irq-gic-common.h        |   7 +-
> >>  drivers/irqchip/irq-gic-v3-its.c        |  11 +-
> >>  drivers/irqchip/irq-gic-v3.c            | 225 ++++++++++++------------
> >>  drivers/irqchip/irq-gic.c               |  10 +-
> >>  drivers/irqchip/irq-hip04.c             |   6 +-
> >>  include/linux/irqchip/arm-gic-common.h  |   4 -
> >>  include/linux/irqchip/arm-gic-v3-prio.h |  52 ++++++
> >>  include/linux/irqchip/arm-gic-v3.h      |   2 +-
> >>  include/linux/wordpart.h                |   8 +
> >>  13 files changed, 201 insertions(+), 201 deletions(-)
> >>  create mode 100644 include/linux/irqchip/arm-gic-v3-prio.h
> >
> > Are you ok for these patches to go through the arm64 tree (I can put
> > them on a stable branch) or you'd rather get them through the irqchip
> > tree? Either way, I don't expect (major) conflicts with the arm64 tree.
> 
> Take them through your tree with my Acked-by. Yes a branch would be
> appreciated just in case.

Thanks. I added them to the arm64 for-next/gic-v3-pmr branch (on
git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git).

> There is also
> 
>       https://lore.kernel.org/all/20240529133446.28446-1-Jonathan.Cameron@huawei.com
> 
> which fiddles with the GIC but most of this is not irqchip code. No idea
> how that is supposed to find it's way into the tree.

I tried to avoid this series so far ;). I'll poke James Morse tomorrow
to see whether he has any outstanding comments (since he started the
work initially). If not, I'll queue them through the arm64 tree (same as
above, on a stable branch in case of conflicts).

Thanks.