Re: [PATCH v2 4/5] arm64: add ARM64_HAS_GIC_PRIO_RELAXED_SYNC cpucap

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

On Thu, Jan 26, 2023 at 08:31:29AM +0000, Marc Zyngier wrote:
> On Wed, 25 Jan 2023 16:38:25 +0000,
> Mark Rutland <mark.rutland@arm.com> wrote:
> 
> [...]
> 
> > @@ -1768,16 +1759,8 @@ static void gic_enable_nmi_support(void)
> >  	for (i = 0; i < gic_data.ppi_nr; i++)
> >  		refcount_set(&ppi_nmi_refs[i], 0);
> >  
> > -	/*
> > -	 * Linux itself doesn't use 1:N distribution, so has no need to
> > -	 * set PMHE. The only reason to have it set is if EL3 requires it
> > -	 * (and we can't change it).
> > -	 */
> 
> I think this is still an important comment as it gives a rationale for
> the extra synchronisation even if Linux doesn't use 1:N distribution:
> If you get secure interrupts in the non-secure priority space, they
> are subjected to the NS PMR setting.
> 
> Could you find a new home for it?

Sure; I'll add it verbatim to the end of the comment block when we detect the
cpucap, i.e.

| static bool has_gic_prio_relaxed_sync(const struct arm64_cpu_capabilities *entry,
|                                       int scope)
| {
|         /*   
|          * If we're not using priority masking then we won't be poking PMR_EL1,
|          * and there's no need to relax synchronization of writes to it, and
|          * ICC_CTLR_EL1 might not be accessible and we must avoid reads from
|          * that.
|          *
|          * ARM64_HAS_GIC_PRIO_MASKING has a lower index, and is a boot CPU
|          * feature, so will be detected earlier.
|          */
|         BUILD_BUG_ON(ARM64_HAS_GIC_PRIO_RELAXED_SYNC <= ARM64_HAS_GIC_PRIO_MASKING);
|         if (!cpus_have_cap(ARM64_HAS_GIC_PRIO_MASKING))
|                 return false;
| 
|         /*   
|          * When Priority Mask Hint Enable (PMHE) == 0b0, PMR is not used as a
|          * hint for interrupt distribution, a DSB is not necessary when
|          * unmasking IRQs via PMR, and we can relax the barrier to a NOP.
|          *
|          * Linux itself doesn't use 1:N distribution, so has no need to
|          * set PMHE. The only reason to have it set is if EL3 requires it
|          * (and we can't change it).
|          */
|         return (gic_read_ctlr() & ICC_CTLR_EL1_PMHE_MASK) == 0;
| }

Thanks,
Mark.

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