* [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
@ 2026-04-17 3:14 Huisong Li
2026-04-21 10:05 ` Sudeep Holla
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Huisong Li @ 2026-04-17 3:14 UTC (permalink / raw)
To: sudeep.holla, jassisinghbrar, linux-kernel, linux-acpi
Cc: linuxarm, jonathan.cameron, zhanjie9, lihuisong
In some scenarios, PCC command may experience probabilistic timeout.
This is primarily caused by the chan_in_use flag being updated after
ringing the doorbell, coupled with a lack of proper memory barriers
across CPU cores.
On fast platforms, a race condition occurs: the platform processing
completes and triggers an interrupt before the local CPU sets
chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
it reads chan_in_use as false and incorrectly ignores the interrupt.
This patch fixes the race by:
1. Moving the chan_in_use update before ringing the doorbell.
2. Using smp_store_release() to ensure the flag update is visible
to other cores before subsequent hardware or software actions
are triggered.
3. Using smp_load_acquire() in the interrupt handler to ensure the
latest flag value is read before deciding to skip the interrupt.
Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
drivers/mailbox/pcc.c | 20 ++++++++++++++++----
1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 636879ae1db7..f45fccd635b6 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -320,8 +320,13 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
return IRQ_NONE;
+ /*
+ * For Master Subspaces, we must ensure we see the latest chan_in_use
+ * value updated by pcc_send_data() on another core. smp_load_acquire
+ * provides the necessary barrier to avoid probabilistic IRQ misses.
+ */
if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
- !pchan->chan_in_use)
+ !smp_load_acquire(&pchan->chan_in_use))
return IRQ_NONE;
if (!pcc_mbox_cmd_complete_check(pchan))
@@ -336,7 +341,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
* where the flag is set again to start new transfer. This is
* required to avoid any possible race in updatation of this flag.
*/
- pchan->chan_in_use = false;
+ smp_store_release(&pchan->chan_in_use, false);
mbox_chan_received_data(chan, NULL);
mbox_chan_txdone(chan, 0);
@@ -438,9 +443,16 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
if (ret)
return ret;
+ /*
+ * Set chan_in_use before ringing the doorbell. Using smp_store_release
+ * ensures the flag is visible to the interrupt handler before the
+ * hardware is actually notified.
+ */
+ if (pchan->plat_irq > 0)
+ smp_store_release(&pchan->chan_in_use, true);
ret = pcc_chan_reg_read_modify_write(&pchan->db);
- if (!ret && pchan->plat_irq > 0)
- pchan->chan_in_use = true;
+ if (ret && pchan->plat_irq > 0)
+ smp_store_release(&pchan->chan_in_use, false);
return ret;
}
--
2.33.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-17 3:14 [PATCH] mailbox: pcc: Fix probabilistic command execution timeout Huisong Li
@ 2026-04-21 10:05 ` Sudeep Holla
2026-04-21 11:21 ` lihuisong (C)
2026-04-27 14:28 ` Robbie King
2026-06-25 9:01 ` lihuisong (C)
2026-06-27 14:31 ` Sudeep Holla
2 siblings, 2 replies; 8+ messages in thread
From: Sudeep Holla @ 2026-04-21 10:05 UTC (permalink / raw)
To: Huisong Li
Cc: jassisinghbrar, linux-kernel, Robbie King, Sudeep Holla,
linux-acpi, linuxarm, jonathan.cameron, zhanjie9
On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
> In some scenarios, PCC command may experience probabilistic timeout.
> This is primarily caused by the chan_in_use flag being updated after
> ringing the doorbell, coupled with a lack of proper memory barriers
> across CPU cores.
>
> On fast platforms, a race condition occurs: the platform processing
> completes and triggers an interrupt before the local CPU sets
> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
> it reads chan_in_use as false and incorrectly ignores the interrupt.
>
> This patch fixes the race by:
> 1. Moving the chan_in_use update before ringing the doorbell.
> 2. Using smp_store_release() to ensure the flag update is visible
> to other cores before subsequent hardware or software actions
> are triggered.
> 3. Using smp_load_acquire() in the interrupt handler to ensure the
> latest flag value is read before deciding to skip the interrupt.
>
Are you seeing the issue on real platforms or you are just reviewing the
code. I would like to test it on the platform I use but I don't have it
handy, so may take some time.
I have added Robbie King who also has helped in testing this PCC driver
in the past.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-21 10:05 ` Sudeep Holla
@ 2026-04-21 11:21 ` lihuisong (C)
2026-04-27 14:28 ` Robbie King
1 sibling, 0 replies; 8+ messages in thread
From: lihuisong (C) @ 2026-04-21 11:21 UTC (permalink / raw)
To: Sudeep Holla
Cc: jassisinghbrar, linux-kernel, Robbie King, linux-acpi, linuxarm,
zhanjie9, lihuisong
On 4/21/2026 6:05 PM, Sudeep Holla wrote:
> On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
>> In some scenarios, PCC command may experience probabilistic timeout.
>> This is primarily caused by the chan_in_use flag being updated after
>> ringing the doorbell, coupled with a lack of proper memory barriers
>> across CPU cores.
>>
>> On fast platforms, a race condition occurs: the platform processing
>> completes and triggers an interrupt before the local CPU sets
>> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
>> it reads chan_in_use as false and incorrectly ignores the interrupt.
>>
>> This patch fixes the race by:
>> 1. Moving the chan_in_use update before ringing the doorbell.
>> 2. Using smp_store_release() to ensure the flag update is visible
>> to other cores before subsequent hardware or software actions
>> are triggered.
>> 3. Using smp_load_acquire() in the interrupt handler to ensure the
>> latest flag value is read before deciding to skip the interrupt.
>>
> Are you seeing the issue on real platforms or you are just reviewing the
> code. I would like to test it on the platform I use but I don't have it
> handy, so may take some time.
Yeah, this is a real issue on my platform. It is probabilistic.
And the problem is gone after this modification.
Gemini AI help me to analyze and suggest that I fix it as this patch done.
>
> I have added Robbie King who also has helped in testing this PCC driver
> in the past.
Thanks. Please help review this patch.
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-21 10:05 ` Sudeep Holla
2026-04-21 11:21 ` lihuisong (C)
@ 2026-04-27 14:28 ` Robbie King
2026-05-26 6:28 ` lihuisong (C)
1 sibling, 1 reply; 8+ messages in thread
From: Robbie King @ 2026-04-27 14:28 UTC (permalink / raw)
To: Sudeep Holla, Huisong Li
Cc: jassisinghbrar, linux-kernel, linux-acpi, linuxarm,
jonathan.cameron, zhanjie9
On 4/21/2026 6:05 AM, Sudeep Holla wrote:
> On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
>> In some scenarios, PCC command may experience probabilistic timeout.
>> This is primarily caused by the chan_in_use flag being updated after
>> ringing the doorbell, coupled with a lack of proper memory barriers
>> across CPU cores.
>>
>> On fast platforms, a race condition occurs: the platform processing
>> completes and triggers an interrupt before the local CPU sets
>> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
>> it reads chan_in_use as false and incorrectly ignores the interrupt.
>>
>> This patch fixes the race by:
>> 1. Moving the chan_in_use update before ringing the doorbell.
>> 2. Using smp_store_release() to ensure the flag update is visible
>> to other cores before subsequent hardware or software actions
>> are triggered.
>> 3. Using smp_load_acquire() in the interrupt handler to ensure the
>> latest flag value is read before deciding to skip the interrupt.
>>
>
> Are you seeing the issue on real platforms or you are just reviewing the
> code. I would like to test it on the platform I use but I don't have it
> handy, so may take some time.
>
> I have added Robbie King who also has helped in testing this PCC driver
> in the past.
>
I was unable to apply the patch to our current kernel version. We are in the
process of updating our kernel modules to support 7.0, once that effort is
finished I can run a few of our current regressions against the patch.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-27 14:28 ` Robbie King
@ 2026-05-26 6:28 ` lihuisong (C)
0 siblings, 0 replies; 8+ messages in thread
From: lihuisong (C) @ 2026-05-26 6:28 UTC (permalink / raw)
To: Robbie King, Sudeep Holla
Cc: jassisinghbrar, linux-kernel, linux-acpi, linuxarm, zhanjie9, lihuisong
On 4/27/2026 10:28 PM, Robbie King wrote:
> On 4/21/2026 6:05 AM, Sudeep Holla wrote:
>> On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
>>> In some scenarios, PCC command may experience probabilistic timeout.
>>> This is primarily caused by the chan_in_use flag being updated after
>>> ringing the doorbell, coupled with a lack of proper memory barriers
>>> across CPU cores.
>>>
>>> On fast platforms, a race condition occurs: the platform processing
>>> completes and triggers an interrupt before the local CPU sets
>>> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
>>> it reads chan_in_use as false and incorrectly ignores the interrupt.
>>>
>>> This patch fixes the race by:
>>> 1. Moving the chan_in_use update before ringing the doorbell.
>>> 2. Using smp_store_release() to ensure the flag update is visible
>>> to other cores before subsequent hardware or software actions
>>> are triggered.
>>> 3. Using smp_load_acquire() in the interrupt handler to ensure the
>>> latest flag value is read before deciding to skip the interrupt.
>>>
>> Are you seeing the issue on real platforms or you are just reviewing the
>> code. I would like to test it on the platform I use but I don't have it
>> handy, so may take some time.
>>
>> I have added Robbie King who also has helped in testing this PCC driver
>> in the past.
>>
> I was unable to apply the patch to our current kernel version. We are in the
> process of updating our kernel modules to support 7.0, once that effort is
> finished I can run a few of our current regressions against the patch.
Hi Robbie King,
Can you help test this patch now?
Or we can fix the conflict to test it on your current kernel.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-17 3:14 [PATCH] mailbox: pcc: Fix probabilistic command execution timeout Huisong Li
2026-04-21 10:05 ` Sudeep Holla
@ 2026-06-25 9:01 ` lihuisong (C)
2026-06-27 14:31 ` Sudeep Holla
2 siblings, 0 replies; 8+ messages in thread
From: lihuisong (C) @ 2026-06-25 9:01 UTC (permalink / raw)
To: sudeep.holla, jassisinghbrar, linux-kernel, linux-acpi
Cc: linuxarm, zhanjie9, lihuisong
Hi Sudeep,
Can you help review this patch?
On 4/17/2026 11:14 AM, Huisong Li wrote:
> In some scenarios, PCC command may experience probabilistic timeout.
> This is primarily caused by the chan_in_use flag being updated after
> ringing the doorbell, coupled with a lack of proper memory barriers
> across CPU cores.
>
> On fast platforms, a race condition occurs: the platform processing
> completes and triggers an interrupt before the local CPU sets
> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
> it reads chan_in_use as false and incorrectly ignores the interrupt.
>
> This patch fixes the race by:
> 1. Moving the chan_in_use update before ringing the doorbell.
> 2. Using smp_store_release() to ensure the flag update is visible
> to other cores before subsequent hardware or software actions
> are triggered.
> 3. Using smp_load_acquire() in the interrupt handler to ensure the
> latest flag value is read before deciding to skip the interrupt.
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
> drivers/mailbox/pcc.c | 20 ++++++++++++++++----
> 1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 636879ae1db7..f45fccd635b6 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -320,8 +320,13 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
> return IRQ_NONE;
>
> + /*
> + * For Master Subspaces, we must ensure we see the latest chan_in_use
> + * value updated by pcc_send_data() on another core. smp_load_acquire
> + * provides the necessary barrier to avoid probabilistic IRQ misses.
> + */
> if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
> - !pchan->chan_in_use)
> + !smp_load_acquire(&pchan->chan_in_use))
> return IRQ_NONE;
>
> if (!pcc_mbox_cmd_complete_check(pchan))
> @@ -336,7 +341,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> * where the flag is set again to start new transfer. This is
> * required to avoid any possible race in updatation of this flag.
> */
> - pchan->chan_in_use = false;
> + smp_store_release(&pchan->chan_in_use, false);
> mbox_chan_received_data(chan, NULL);
> mbox_chan_txdone(chan, 0);
>
> @@ -438,9 +443,16 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
> if (ret)
> return ret;
>
> + /*
> + * Set chan_in_use before ringing the doorbell. Using smp_store_release
> + * ensures the flag is visible to the interrupt handler before the
> + * hardware is actually notified.
> + */
> + if (pchan->plat_irq > 0)
> + smp_store_release(&pchan->chan_in_use, true);
> ret = pcc_chan_reg_read_modify_write(&pchan->db);
> - if (!ret && pchan->plat_irq > 0)
> - pchan->chan_in_use = true;
> + if (ret && pchan->plat_irq > 0)
> + smp_store_release(&pchan->chan_in_use, false);
>
> return ret;
> }
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-04-17 3:14 [PATCH] mailbox: pcc: Fix probabilistic command execution timeout Huisong Li
2026-04-21 10:05 ` Sudeep Holla
2026-06-25 9:01 ` lihuisong (C)
@ 2026-06-27 14:31 ` Sudeep Holla
2026-06-29 13:46 ` lihuisong (C)
2 siblings, 1 reply; 8+ messages in thread
From: Sudeep Holla @ 2026-06-27 14:31 UTC (permalink / raw)
To: Huisong Li
Cc: jassisinghbrar, linux-kernel, Sudeep Holla, linux-acpi, linuxarm,
jonathan.cameron, zhanjie9
Hi Huisong,
Sorry for the delay, this seem to have slipped through the cracks.
On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
> In some scenarios, PCC command may experience probabilistic timeout.
> This is primarily caused by the chan_in_use flag being updated after
> ringing the doorbell, coupled with a lack of proper memory barriers
> across CPU cores.
>
> On fast platforms, a race condition occurs: the platform processing
> completes and triggers an interrupt before the local CPU sets
> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
> it reads chan_in_use as false and incorrectly ignores the interrupt.
>
> This patch fixes the race by:
> 1. Moving the chan_in_use update before ringing the doorbell.
> 2. Using smp_store_release() to ensure the flag update is visible
> to other cores before subsequent hardware or software actions
> are triggered.
> 3. Using smp_load_acquire() in the interrupt handler to ensure the
> latest flag value is read before deciding to skip the interrupt.
>
I don't think we need store_release/load_acquire semantics here. IIUC
we don't need to make all preceding memory updates visible to the IRQ handler.
i.e. seeing the flag updated doesn't require observing all the updates before
the flag itself. This flag is not publishing extra data to be consumed after
the check.
How about something like below ? If you agree and it works, add my
Reviewed-by and post.
Regards,
Sudeep
-->8
mailbox: pcc: Fix command timeout due to missed interrupt
PCC command execution can time out when a fast platform completes a
transaction and signals the platform interrupt before pcc_send_data()
marks the channel as in use. For shared platform interrupts, the type 3
handler uses chan_in_use to decide whether the interrupt belongs to the
channel. If it observes false, it ignores the completion and the caller
waits until timeout.
Publish chan_in_use before ringing the doorbell. Use WRITE_ONCE() for
the lockless flag update and READ_ONCE() in the interrupt handler when
filtering shared interrupts. The following ordered MMIO accessor used for
the doorbell keeps the earlier store visible before the platform is
notified.
Clear chan_in_use with WRITE_ONCE() when completing the transaction,
matching the lockless flag protocol used by the interrupt and send paths.
Fixes: 3db174e478cb ("mailbox: pcc: Support shared interrupt for multiple subspaces")
Signed-off-by: Huisong Li <lihuisong@huawei.com>
Link: https://patch.msgid.link/20260417031429.2509443-1-lihuisong@huawei.com
Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
---
drivers/mailbox/pcc.c | 41 +++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 6fbc17d41774..bde5e0852157 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -91,12 +91,11 @@ struct pcc_chan_reg {
* @plat_irq: platform interrupt
* @type: PCC subspace type
* @plat_irq_flags: platform interrupt flags
- * @chan_in_use: this flag is used just to check if the interrupt needs
- * handling when it is shared. Since only one transfer can occur
- * at a time and mailbox takes care of locking, this flag can be
- * accessed without a lock. Note: the type only support the
- * communication from OSPM to Platform, like type3, use it, and
- * other types completely ignore it.
+ * @chan_in_use: lockless flag used by initiator subspaces, such as type 3,
+ * to filter shared platform interrupts. Only one transfer can occur
+ * at a time, but the interrupt handler may sample the flag on another
+ * CPU, so all accesses must use READ_ONCE() or WRITE_ONCE().
+ * Other subspace types ignore it.
*/
struct pcc_chan_info {
struct pcc_mbox_chan chan;
@@ -320,8 +319,13 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
return IRQ_NONE;
+ /*
+ * Master subspaces use this flag to filter shared interrupts. Use
+ * READ_ONCE() to sample the lockless flag written by pcc_send_data()
+ * on another CPU.
+ */
if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
- !pchan->chan_in_use)
+ !READ_ONCE(pchan->chan_in_use))
return IRQ_NONE;
if (!pcc_mbox_cmd_complete_check(pchan))
@@ -331,12 +335,12 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
return IRQ_NONE;
/*
- * Clear this flag after updating interrupt ack register and just
- * before mbox_chan_received_data() which might call pcc_send_data()
- * where the flag is set again to start new transfer. This is
- * required to avoid any possible race in updatation of this flag.
+ * Clear this flag after updating the interrupt ack register and just
+ * before mbox_chan_received_data(), which might call pcc_send_data()
+ * and set the flag again to start a new transfer. Use WRITE_ONCE()
+ * for the lockless update observed by the send and interrupt paths.
*/
- pchan->chan_in_use = false;
+ WRITE_ONCE(pchan->chan_in_use, false);
mbox_chan_received_data(chan, NULL);
mbox_chan_txdone(chan, 0);
@@ -466,9 +470,18 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
if (ret)
return ret;
+ /*
+ * Set chan_in_use before ringing the doorbell so a fast completion
+ * interrupt is not mistaken for a shared interrupt from another
+ * subspace. Use WRITE_ONCE() for the lockless flag update. The
+ * ordered MMIO accessor used to ring the doorbell keeps this store
+ * visible before the platform is notified.
+ */
+ if (pchan->plat_irq > 0)
+ WRITE_ONCE(pchan->chan_in_use, true);
ret = pcc_chan_reg_read_modify_write(&pchan->db);
- if (!ret && pchan->plat_irq > 0)
- pchan->chan_in_use = true;
+ if (ret && pchan->plat_irq > 0)
+ WRITE_ONCE(pchan->chan_in_use, false);
return ret;
}
--
2.43.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] mailbox: pcc: Fix probabilistic command execution timeout
2026-06-27 14:31 ` Sudeep Holla
@ 2026-06-29 13:46 ` lihuisong (C)
0 siblings, 0 replies; 8+ messages in thread
From: lihuisong (C) @ 2026-06-29 13:46 UTC (permalink / raw)
To: Sudeep Holla
Cc: jassisinghbrar, linux-kernel, linux-acpi, linuxarm, zhanjie9, lihuisong
On 6/27/2026 10:31 PM, Sudeep Holla wrote:
> Hi Huisong,
>
> Sorry for the delay, this seem to have slipped through the cracks.
>
> On Fri, Apr 17, 2026 at 11:14:29AM +0800, Huisong Li wrote:
>> In some scenarios, PCC command may experience probabilistic timeout.
>> This is primarily caused by the chan_in_use flag being updated after
>> ringing the doorbell, coupled with a lack of proper memory barriers
>> across CPU cores.
>>
>> On fast platforms, a race condition occurs: the platform processing
>> completes and triggers an interrupt before the local CPU sets
>> chan_in_use to true. When the interrupt handler pcc_mbox_irq() runs,
>> it reads chan_in_use as false and incorrectly ignores the interrupt.
>>
>> This patch fixes the race by:
>> 1. Moving the chan_in_use update before ringing the doorbell.
>> 2. Using smp_store_release() to ensure the flag update is visible
>> to other cores before subsequent hardware or software actions
>> are triggered.
>> 3. Using smp_load_acquire() in the interrupt handler to ensure the
>> latest flag value is read before deciding to skip the interrupt.
>>
> I don't think we need store_release/load_acquire semantics here. IIUC
> we don't need to make all preceding memory updates visible to the IRQ handler.
> i.e. seeing the flag updated doesn't require observing all the updates before
> the flag itself. This flag is not publishing extra data to be consumed after
> the check.
Ack.
>
> How about something like below ? If you agree and it works, add my
> Reviewed-by and post.
It's nice.
Many thanks for your improvements.
I will test it again.
>
> Regards,
> Sudeep
>
> -->8
> mailbox: pcc: Fix command timeout due to missed interrupt
>
> PCC command execution can time out when a fast platform completes a
> transaction and signals the platform interrupt before pcc_send_data()
> marks the channel as in use. For shared platform interrupts, the type 3
> handler uses chan_in_use to decide whether the interrupt belongs to the
> channel. If it observes false, it ignores the completion and the caller
> waits until timeout.
>
> Publish chan_in_use before ringing the doorbell. Use WRITE_ONCE() for
> the lockless flag update and READ_ONCE() in the interrupt handler when
> filtering shared interrupts. The following ordered MMIO accessor used for
> the doorbell keeps the earlier store visible before the platform is
> notified.
>
> Clear chan_in_use with WRITE_ONCE() when completing the transaction,
> matching the lockless flag protocol used by the interrupt and send paths.
>
> Fixes: 3db174e478cb ("mailbox: pcc: Support shared interrupt for multiple subspaces")
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Link: https://patch.msgid.link/20260417031429.2509443-1-lihuisong@huawei.com
> Signed-off-by: Sudeep Holla <sudeep.holla@kernel.org>
> ---
> drivers/mailbox/pcc.c | 41 +++++++++++++++++++++++++++--------------
> 1 file changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> index 6fbc17d41774..bde5e0852157 100644
> --- a/drivers/mailbox/pcc.c
> +++ b/drivers/mailbox/pcc.c
> @@ -91,12 +91,11 @@ struct pcc_chan_reg {
> * @plat_irq: platform interrupt
> * @type: PCC subspace type
> * @plat_irq_flags: platform interrupt flags
> - * @chan_in_use: this flag is used just to check if the interrupt needs
> - * handling when it is shared. Since only one transfer can occur
> - * at a time and mailbox takes care of locking, this flag can be
> - * accessed without a lock. Note: the type only support the
> - * communication from OSPM to Platform, like type3, use it, and
> - * other types completely ignore it.
> + * @chan_in_use: lockless flag used by initiator subspaces, such as type 3,
> + * to filter shared platform interrupts. Only one transfer can occur
> + * at a time, but the interrupt handler may sample the flag on another
> + * CPU, so all accesses must use READ_ONCE() or WRITE_ONCE().
> + * Other subspace types ignore it.
> */
> struct pcc_chan_info {
> struct pcc_mbox_chan chan;
> @@ -320,8 +319,13 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
> return IRQ_NONE;
>
> + /*
> + * Master subspaces use this flag to filter shared interrupts. Use
> + * READ_ONCE() to sample the lockless flag written by pcc_send_data()
> + * on another CPU.
> + */
> if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE &&
> - !pchan->chan_in_use)
> + !READ_ONCE(pchan->chan_in_use))
> return IRQ_NONE;
>
> if (!pcc_mbox_cmd_complete_check(pchan))
> @@ -331,12 +335,12 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
> return IRQ_NONE;
>
> /*
> - * Clear this flag after updating interrupt ack register and just
> - * before mbox_chan_received_data() which might call pcc_send_data()
> - * where the flag is set again to start new transfer. This is
> - * required to avoid any possible race in updatation of this flag.
> + * Clear this flag after updating the interrupt ack register and just
> + * before mbox_chan_received_data(), which might call pcc_send_data()
> + * and set the flag again to start a new transfer. Use WRITE_ONCE()
> + * for the lockless update observed by the send and interrupt paths.
> */
> - pchan->chan_in_use = false;
> + WRITE_ONCE(pchan->chan_in_use, false);
> mbox_chan_received_data(chan, NULL);
> mbox_chan_txdone(chan, 0);
>
> @@ -466,9 +470,18 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
> if (ret)
> return ret;
>
> + /*
> + * Set chan_in_use before ringing the doorbell so a fast completion
> + * interrupt is not mistaken for a shared interrupt from another
> + * subspace. Use WRITE_ONCE() for the lockless flag update. The
> + * ordered MMIO accessor used to ring the doorbell keeps this store
> + * visible before the platform is notified.
> + */
> + if (pchan->plat_irq > 0)
> + WRITE_ONCE(pchan->chan_in_use, true);
> ret = pcc_chan_reg_read_modify_write(&pchan->db);
> - if (!ret && pchan->plat_irq > 0)
> - pchan->chan_in_use = true;
> + if (ret && pchan->plat_irq > 0)
> + WRITE_ONCE(pchan->chan_in_use, false);
>
> return ret;
> }
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-06-29 13:46 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-17 3:14 [PATCH] mailbox: pcc: Fix probabilistic command execution timeout Huisong Li
2026-04-21 10:05 ` Sudeep Holla
2026-04-21 11:21 ` lihuisong (C)
2026-04-27 14:28 ` Robbie King
2026-05-26 6:28 ` lihuisong (C)
2026-06-25 9:01 ` lihuisong (C)
2026-06-27 14:31 ` Sudeep Holla
2026-06-29 13:46 ` lihuisong (C)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®