linux-kernel - Re: [PATCH 2/2] perf:arm-ni: support PMUs to share IRQs for different clock domains

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <4e25536e-459b-4376-9422-4a7d0156234d@arm.com>
Date: Thu, 17 Apr 2025 15:41:36 +0100
From: Robin Murphy <robin.murphy@....com>
To: Shouping Wang <allen.wang@...micro.com>, will@...nel.org
Cc: mark.rutland@....com, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, peter.du@...micro.com, andy.xu@...micro.com
Subject: Re: [PATCH 2/2] perf:arm-ni: support PMUs to share IRQs for different
 clock domains

On 10/04/2025 12:42 pm, Shouping Wang wrote:
> The ARM NI700 contains multiple clock domains, each with a PMU.
> In some hardware implementations, these PMUs under the same device
> share a common interrupt line. The current codes implementation
> only supports requesting a separate IRQ for each clock domain's PMU.
> 
> Here, a single interrupt handler is registered for shared interrupt.
> Within this handler, the interrupt status of all PMUs sharing the
> interrupt is checked.

Unfortunately this isn't sufficient for sharing an IRQ between multiple 
PMUs - the CPU affinity and hotplug context migration must be kept in 
sync as well.

I guess I really should get back to my old plan to factor out a common 
helper library for all this stuff - that was the main reason I left 
combined IRQ support out of the initial version here rather than do 
another copy-paste of the arm_dmc620 design again...

Thanks,
Robin.

> Signed-off-by: Shouping Wang <allen.wang@...micro.com>
> ---
>   drivers/perf/arm-ni.c | 77 +++++++++++++++++++++++++++++--------------
>   1 file changed, 53 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> index 3f3d2e0f91fa..611085e89436 100644
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -104,6 +104,7 @@ struct arm_ni_cd {
>   	u16 id;
>   	int num_units;
>   	int irq;
> +	s8 irq_friend;
>   	int cpu;
>   	struct hlist_node cpuhp_node;
>   	struct pmu pmu;
> @@ -446,26 +447,31 @@ static irqreturn_t arm_ni_handle_irq(int irq, void *dev_id)
>   {
>   	struct arm_ni_cd *cd = dev_id;
>   	irqreturn_t ret = IRQ_NONE;
> -	u32 reg = readl_relaxed(cd->pmu_base + NI_PMOVSCLR);
> +	u32 reg;
>   
> -	if (reg & (1U << NI_CCNT_IDX)) {
> -		ret = IRQ_HANDLED;
> -		if (!(WARN_ON(!cd->ccnt))) {
> -			arm_ni_event_read(cd->ccnt);
> -			arm_ni_init_ccnt(cd);
> +	for (;;) {
> +		reg = readl_relaxed(cd->pmu_base + NI_PMOVSCLR);
> +		if (reg & (1U << NI_CCNT_IDX)) {
> +			ret = IRQ_HANDLED;
> +			if (!(WARN_ON(!cd->ccnt))) {
> +				arm_ni_event_read(cd->ccnt);
> +				arm_ni_init_ccnt(cd);
> +			}
>   		}
> -	}
> -	for (int i = 0; i < NI_NUM_COUNTERS; i++) {
> -		if (!(reg & (1U << i)))
> -			continue;
> -		ret = IRQ_HANDLED;
> -		if (!(WARN_ON(!cd->evcnt[i]))) {
> -			arm_ni_event_read(cd->evcnt[i]);
> -			arm_ni_init_evcnt(cd, i);
> +		for (int i = 0; i < NI_NUM_COUNTERS; i++) {
> +			if (!(reg & (1U << i)))
> +				continue;
> +			ret = IRQ_HANDLED;
> +			if (!(WARN_ON(!cd->evcnt[i]))) {
> +				arm_ni_event_read(cd->evcnt[i]);
> +				arm_ni_init_evcnt(cd, i);
> +			}
>   		}
> +		writel_relaxed(reg, cd->pmu_base + NI_PMOVSCLR);
> +		if (!cd->irq_friend)
> +			return ret;
> +		cd += cd->irq_friend;
>   	}
> -	writel_relaxed(reg, cd->pmu_base + NI_PMOVSCLR);
> -	return ret;
>   }
>   
>   static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_start)
> @@ -538,12 +544,6 @@ static int arm_ni_init_cd(struct arm_ni *ni, struct arm_ni_node *node, u64 res_s
>   	if (cd->irq < 0)
>   		return cd->irq;
>   
> -	err = devm_request_irq(ni->dev, cd->irq, arm_ni_handle_irq,
> -			       IRQF_NOBALANCING | IRQF_NO_THREAD,
> -			       dev_name(ni->dev), cd);
> -	if (err)
> -		return err;
> -
>   	cd->cpu = cpumask_local_spread(0, dev_to_node(ni->dev));
>   	cd->pmu = (struct pmu) {
>   		.module = THIS_MODULE,
> @@ -603,6 +603,30 @@ static void arm_ni_probe_domain(void __iomem *base, struct arm_ni_node *node)
>   	node->num_components = readl_relaxed(base + NI_CHILD_NODE_INFO);
>   }
>   
> +static int arm_ni_irq_init(struct arm_ni *ni)
> +{
> +	int irq;
> +	int err = 0;
> +
> +	for (int i = 0; i < ni->num_cds; i++) {
> +		irq = ni->cds[i].irq;
> +		for (int j = i; j--; ) {
> +			if (ni->cds[j].irq == irq) {
> +				ni->cds[j].irq_friend = i-j;
> +				goto next;
> +			}
> +		}
> +		err =  devm_request_irq(ni->dev, irq, arm_ni_handle_irq,
> +					IRQF_NOBALANCING | IRQF_NO_THREAD,
> +					 dev_name(ni->dev), &ni->cds[i]);
> +		if (err)
> +			return err;
> +next:
> +		;
> +	}
> +	return 0;
> +}
> +
>   static int arm_ni_probe(struct platform_device *pdev)
>   {
>   	struct arm_ni_node cfg, vd, pd, cd;
> @@ -611,6 +635,7 @@ static int arm_ni_probe(struct platform_device *pdev)
>   	void __iomem *base;
>   	static atomic_t id;
>   	int num_cds;
> +	int ret;
>   	u32 reg, part;
>   
>   	/*
> @@ -669,8 +694,6 @@ static int arm_ni_probe(struct platform_device *pdev)
>   			reg = readl_relaxed(vd.base + NI_CHILD_PTR(p));
>   			arm_ni_probe_domain(base + reg, &pd);
>   			for (int c = 0; c < pd.num_components; c++) {
> -				int ret;
> -
>   				reg = readl_relaxed(pd.base + NI_CHILD_PTR(c));
>   				arm_ni_probe_domain(base + reg, &cd);
>   				ret = arm_ni_init_cd(ni, &cd, res->start);
> @@ -683,6 +706,12 @@ static int arm_ni_probe(struct platform_device *pdev)
>   		}
>   	}
>   
> +	ret = arm_ni_irq_init(ni);
> +	if (ret) {
> +		arm_ni_remove(pdev);
> +		return ret;
> +	}
> +
>   	return 0;
>   }
>