Re: [PATCH] perf: arm_cspmu: Don't touch interrupt registers if no interrupt was assigned

[Ilkka Koskinen]

On Fri, 5 Apr 2024, Robin Murphy wrote:
> On 2024-03-07 7:31 pm, Ilkka Koskinen wrote:
> > The driver enabled and disabled interrupts even if no interrupt was
> > assigned to the device.
>
> Why's that a concern - if the interrupt isn't routed anywhere, surely it 
> makes no difference what happens at the source end?

The issue is that we have two PMUs attached to the same interrupt line.
Unfortunately, I just don't seem to find time to add support for shared 
interrupts to the cspmu driver. Meanwhile, I assigned the interrupt 
to one of the PMUs while the other one has zero in the APMT table. Without 
the patch, I can trigger "ghost interrupt" in the latter PMU.

Cheers, Ilkka

> Thanks,
> Robin.
>
> > Signed-off-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
> > ---
> >   drivers/perf/arm_cspmu/arm_cspmu.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c 
> > b/drivers/perf/arm_cspmu/arm_cspmu.c
> > index 50b89b989ce7..2cbdb5dcb6ff 100644
> > --- a/drivers/perf/arm_cspmu/arm_cspmu.c
> > +++ b/drivers/perf/arm_cspmu/arm_cspmu.c
> > @@ -795,7 +795,8 @@ static void arm_cspmu_enable_counter(struct arm_cspmu 
> > *cspmu, int idx)
> >   	inten_off = PMINTENSET + (4 * reg_id);
> >   	cnten_off = PMCNTENSET + (4 * reg_id);
> >   -	writel(BIT(reg_bit), cspmu->base0 + inten_off);
> > +	if (cspmu->irq)
> > +		writel(BIT(reg_bit), cspmu->base0 + inten_off);
> >   	writel(BIT(reg_bit), cspmu->base0 + cnten_off);
> >   }
> >   @@ -810,7 +811,8 @@ static void arm_cspmu_disable_counter(struct 
> > arm_cspmu *cspmu, int idx)
> >   	cnten_off = PMCNTENCLR + (4 * reg_id);
> >     	writel(BIT(reg_bit), cspmu->base0 + cnten_off);
> > -	writel(BIT(reg_bit), cspmu->base0 + inten_off);
> > +	if (cspmu->irq)
> > +		writel(BIT(reg_bit), cspmu->base0 + inten_off);
> >   }
> >     static void arm_cspmu_event_update(struct perf_event *event)