Re: [PATCH 3/4] arm64: add ARM64_HAS_GIC_PRIO_NO_PMHE cpucap

[Mark Rutland <mark.rutland@arm.com>]

On Mon, Jan 23, 2023 at 01:23:31PM +0000, Marc Zyngier wrote:
> On Mon, 23 Jan 2023 12:40:41 +0000,
> Mark Rutland <mark.rutland@arm.com> wrote:
> > 
> > When Priority Mask Hint Enable (PMHE) == 0b1, the GIC may use the PMR
> > value to determine whether to signal an IRQ to a PE, and consequently
> > after a change to the PMR value, a DSB SY may be required to ensure that
> > interrupts are signalled to a CPU in finite time. When PMHE == 0b0,
> > interrupts are always signalled to the relevant PE, and all masking
> > occurs locally, without requiring a DSB SY.
> > 
> > Since commit:
> > 
> >   f226650494c6aa87 ("arm64: Relax ICC_PMR_EL1 accesses when ICC_CTLR_EL1.PMHE is clear")
> > 
> > ... we handle this dynamically: in most cases a static key is used to
> > determine whether to issue a DSB SY, but the entry code must read from
> > ICC_CTLR_EL1 as static keys aren't accessible from plain assembly.
> > 
> > It would be much nicer to use an alternative instruction sequence for
> > the DSB, as this would avoid the need to read from ICC_CTLR_EL1 in the
> > entry code, and for most other code this will result in simpler code
> > generation with fewer instructions and fewer branches.
> > 
> > This patch adds a new ARM64_HAS_GIC_PRIO_NO_PMHE cpucap which is only
> > set when ICC_CTLR_EL1.PMHE == 0b0 (and GIC priority masking is in use).
> > This allows us to replace the existing users of the `gic_pmr_sync`
> > static key with alternative sequences which default to a DSB SY and are
> > relaxed to a NOP when PMHE is not in use.
> 
> I personally find the "negative capability" pretty annoying, specially
> considering that hardly anyone uses PMHE. The way the code reads with
> this patch, it is always some sort of double negation.

For the polarity and double-negation, I could rename this to
ARM64_HAS_GIC_PRIO_RELAXED_SYNC, if that helps?

> Can't the DSB be patched-in instead, making the PMHE cap a "positive"
> one? 

We could; my rationale for doing it this way is that we can use the common NOP
patching helper, and avoid generating a copy of the `DSB SY` instruction per
pmr_sync() call (which gets generated near to the call and never gets free,
unlike the alt_instr entries), which adds up quickly when using pseudo-NMIs.

> It shouldn't affect interrupt distribution as long as the
> patching occurs before we take interrupts. For modules, the patching always
> occurs before we can run the module, so this should be equally safe.

I agree it shouldn't matter either way -- until we've patched in
ARM64_HAS_GIC_PRIO_MASKING alternatives it's not going to matter.

> The patch otherwise looks OK to me.

Thanks!

Do you have a preference between the ARM64_HAS_GIC_PRIO_RELAXED_SYNC or
ARM64_HAS_GIC_PRIO_PMHE options above?

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel