From: Sudeep Holla <sudeep.holla@arm.com>
To: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Sudeep Holla <sudeep.holla@arm.com>,
Jassi Brar <jassisinghbrar@gmail.com>,
Huisong Li <lihuisong@huawei.com>,
Adam Young <admiyo@os.amperecomputing.com>,
Robbie King <robbiek@xsightlabs.com>
Subject: [PATCH v3 02/13] mailbox: pcc: Always clear the platform ack interrupt first
Date: Thu, 13 Mar 2025 15:28:48 +0000 [thread overview]
Message-ID: <20250313-pcc_fixes_updates-v3-2-019a4aa74d0f@arm.com> (raw)
In-Reply-To: <20250313-pcc_fixes_updates-v3-0-019a4aa74d0f@arm.com>
The PCC mailbox interrupt handler (pcc_mbox_irq()) currently checks
for command completion flags and any error status before clearing the
interrupt.
The below sequence highlights an issue in the handling of PCC mailbox
interrupts, specifically when dealing with doorbell notifications and
acknowledgment between the OSPM and the platform where type3 and type4
channels are sharing the interrupt.
-------------------------------------------------------------------------
| T | Platform Firmware | OSPM/Linux PCC driver |
|---|---------------------------------|---------------------------------|
| 1 | | Build message in shmem |
| 2 | | Ring Type3 chan doorbell |
| 3 | Receives the doorbell interrupt | |
| 4 | Process the message from OSPM | |
| 5 | Build response for the message | |
| 6 | Ring Platform ACK interrupt on | |
| | Type3 chan to OSPM | Received the interrupt |
| 7 | Build Notification in Type4 Chan| |
| 8 | | Start processing interrupt in |
| | | pcc_mbox_irq() handler |
| 9 | | Enter PCC handler for Type4 chan|
|10 | | Check command complete cleared |
|11 | | Read the notification |
|12 | | Clear Platform ACK interrupt |
| | No effect from the previous step yet as the Platform ACK |
| | interrupt has not yet been triggered for this channel |
|13 | Ring Platform ACK interrupt on | |
| | Type4 chan to OSPM | |
|14 | | Enter PCC handler for Type3 chan|
|15 | | Command complete is set. |
|16 | | Read the response. |
|17 | | Clear Platform ACK interrupt |
|18 | | Leave PCC handler for Type3 |
|19 | | Leave pcc_mbox_irq() handler |
|20 | | Re-enter pcc_mbox_irq() handler |
|21 | | Enter PCC handler for Type4 chan|
|22 | | Leave PCC handler for Type4 chan|
|23 | | Enter PCC handler for Type3 chan|
|24 | | Leave PCC handler for Type3 chan|
|25 | | Leave pcc_mbox_irq() handler |
-------------------------------------------------------------------------
The key issue occurs when OSPM tries to acknowledge platform ack
interrupt for a notification which is ready to be read and processed
but the interrupt itself is not yet triggered by the platform.
This ineffective acknowledgment leads to an issue later in time where
the interrupt remains pending as we exit the interrupt handler without
clearing the platform ack interrupt as there is no pending response or
notification. The interrupt acknowledgment order is incorrect.
To resolve this issue, the platform acknowledgment interrupt should
always be cleared before processing the interrupt for any notifications
or response.
Reported-by: Robbie King <robbiek@xsightlabs.com>
Reviewed-by: Huisong Li <lihuisong@huawei.com>
Tested-by: Huisong Li <lihuisong@huawei.com>
Tested-by: Adam Young <admiyo@os.amperecomputing.com>
Tested-by: Robbie King <robbiek@xsightlabs.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/mailbox/pcc.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 8fd4d0f79b090eeea1c7779061841ef507083687..f8215a8f656a460b38806d4c002470c3fe1e3c9c 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -313,6 +313,10 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
int ret;
pchan = chan->con_priv;
+
+ if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
+ return IRQ_NONE;
+
if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
!pchan->chan_in_use)
return IRQ_NONE;
@@ -330,9 +334,6 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
return IRQ_NONE;
}
- if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
- return IRQ_NONE;
-
/*
* Clear this flag after updating interrupt ack register and just
* before mbox_chan_received_data() which might call pcc_send_data()
--
2.34.1
next prev parent reply other threads:[~2025-03-13 15:29 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-13 15:28 [PATCH v3 00/13] mailbox: pcc: Fixes and cleanup/refactoring Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 01/13] mailbox: pcc: Fix the possible race in updation of chan_in_use flag Sudeep Holla
2025-03-13 15:28 ` Sudeep Holla [this message]
2025-03-13 15:28 ` [PATCH v3 03/13] mailbox: pcc: Drop unnecessary endianness conversion of pcc_hdr.flags Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 04/13] mailbox: pcc: Return early if no GAS register from pcc_mbox_cmd_complete_check Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 05/13] mailbox: pcc: Use acpi_os_ioremap() instead of ioremap() Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 06/13] mailbox: pcc: Refactor error handling in irq handler into separate function Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 07/13] mailbox: pcc: Always map the shared memory communication address Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 08/13] mailbox: pcc: Refactor and simplify check_and_ack() Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 09/13] soc: hisilicon: kunpeng_hccs: Simplify PCC shared memory region handling Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 10/13] i2c: xgene-slimpro: " Sudeep Holla
2025-03-14 0:48 ` Andi Shyti
2025-03-14 10:23 ` Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 11/13] hwmon: (xgene-hwmon) " Sudeep Holla
2025-04-11 14:15 ` Guenter Roeck
2025-04-11 15:07 ` Sudeep Holla
2025-03-13 15:28 ` [PATCH v3 12/13] ACPI: PCC: " Sudeep Holla
2025-03-17 23:07 ` Adam Young
2025-03-13 15:28 ` [PATCH v3 13/13] ACPI: CPPC: " Sudeep Holla
2025-03-21 13:57 ` [PATCH v3 00/13] mailbox: pcc: Fixes and cleanup/refactoring Sudeep Holla
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250313-pcc_fixes_updates-v3-2-019a4aa74d0f@arm.com \
--to=sudeep.holla@arm.com \
--cc=admiyo@os.amperecomputing.com \
--cc=jassisinghbrar@gmail.com \
--cc=lihuisong@huawei.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=robbiek@xsightlabs.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox