Re: [PATCH v2 16/28] drm/panthor: Add AM_RESOURCE_GROUP support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] drm/panthor: lost AW message interrupt in
  rg_irq_raw_handler()
--

commit 4a3c9d76bb48fcfdcf16aaac9521c7316fca8936
Author: Karunika Choo <karunika.choo@arm.com>

drm/panthor: Add AM_RESOURCE_GROUP support

This commit adds resource-group support to the Panthor arbitration driver,
including MMIO and IRQ setup, per-access-window message handling, handshake
negotiation, and runtime PM callbacks.

> diff --git a/drivers/gpu/drm/panthor/arbitration/panthor_resource_group.c b/drivers/gpu/drm/panthor/arbitration/panthor_resource_group.c
> new file mode 100644
> index 0000000000000..e413cfd9e6d25
> --- /dev/null
> +++ b/drivers/gpu/drm/panthor/arbitration/panthor_resource_group.c

[ ... ]

> +static irqreturn_t rg_irq_raw_handler(int irq, void *data)
> +{
> +	struct panthor_resource_group *rg = data;
> +	unsigned long message_mask;
> +	u32 status;
> +	u8 aw_id;
> +
> +	scoped_guard(spinlock_irqsave, &rg->irq_lock) {
> +		if (rg->irq_suspended)
> +			return IRQ_NONE;
> +	}
> +
> +	status = gpu_read(rg->iomem, AM_RG_IRQ_STATUS);
> +	if (!status)
> +		return IRQ_NONE;
> +
> +	message_mask = status;
> +	for_each_set_bit(aw_id, &message_mask, AM_ARB_MAX_AW_COUNT) {
> +		u64 message = panthor_am_msg_read(&rg->aw_data[aw_id].msg);
> +
> +		rg_handle_message(rg, aw_id, message);
> +	}
> +
> +	gpu_write(rg->iomem, AM_RG_IRQ_CLEAR, status);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static void rg_drain_pending_messages(struct panthor_resource_group *rg)
> +{
> +	unsigned long message_mask;
> +	u8 aw_id;
> +
> +	if (pm_runtime_status_suspended(rg->dev))
> +		return;
> +
> +	message_mask = gpu_read(rg->iomem, AM_RG_IRQ_RAWSTAT) & AM_RG_MESSAGE_MASK;
> +
> +	for_each_set_bit(aw_id, &message_mask, AM_ARB_MAX_AW_COUNT) {
> +		u64 message = panthor_am_msg_read(&rg->aw_data[aw_id].msg);
> +
> +		rg_handle_message(rg, aw_id, message);
> +	}
> +
> +	gpu_write(rg->iomem, AM_RG_IRQ_CLEAR, message_mask);
> +}

[Severity: Medium]
Can writing AM_RG_IRQ_CLEAR after the loop in rg_irq_raw_handler() and
rg_drain_pending_messages() clear a follow-up access-window interrupt
before its message is read?

When rg_handle_message() processes an unacknowledged VM_ARB_INIT from an
access window, rg_respond_to_handshake() immediately sends ARB_VM_INIT back
to that access window before the loop finishes and writes AM_RG_IRQ_CLEAR:

CPU0 (resource group)             CPU1 / VM (access window aw_id)
rg_irq_raw_handler()
  status = gpu_read(AM_RG_IRQ_STATUS)
  panthor_am_msg_read()
  rg_handle_message()
    rg_respond_to_handshake()
      rg_send_init_msg(ARB_VM_INIT)
                                  receives ARB_VM_INIT
                                  sends VM_ARB_GPU_REQUEST
                                  (asserts BIT(aw_id) in AM_RG_IRQ_RAWSTAT)
  gpu_write(AM_RG_IRQ_CLEAR, status)
    <-- clears BIT(aw_id) without reading VM_ARB_GPU_REQUEST

If the hardware also keeps the sender's AM_OUTGOING_MESSAGE_STATUS busy
until AM_RG_IRQ_CLEAR is written, delaying the clear until after
rg_handle_message() leaves the access window's outgoing slot busy while it
handles the reply.  In either case, a missed VM_ARB_GPU_REQUEST or
VM_ARB_GPU_STOPPED stalls GPU access acquisition or partition scheduling
until the request timeout expires.

Clearing BIT(aw_id) in AM_RG_IRQ_CLEAR right after panthor_am_msg_read()
and before rg_handle_message() would avoid this race.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110153907.3241978-1-karunika.choo@arm.com?part=16