From: Adam Young <admiyo@os.amperecomputing.com>
To: Sudeep Holla <sudeep.holla@kernel.org>,
Jassi Brar <jassisinghbrar@gmail.com>,
Huisong Li <lihuisong@huawei.com>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
Jeremy Kerr <jk@codeconstruct.com.au>,
Matt Johnston <matt@codeconstruct.com.au>,
"David S . Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Sudeep Holla <sudeep.holla@arm.com>,
Jonathan Cameron <Jonathan.Cameron@huawei.com>
Subject: [PATCH v45 1/7] mailbox/pcc.c: shmem map/unmap startup/teardown
Date: Tue, 21 Jul 2026 13:52:50 -0400 [thread overview]
Message-ID: <20260721175258.87600-2-admiyo@os.amperecomputing.com> (raw)
In-Reply-To: <20260721175258.87600-1-admiyo@os.amperecomputing.com>
The mailbox IRQ and shmems are not cleaned up atomically, so there is a
race condition. If the shmem is torn down while the IRQ is active, a late
interrupt can trigger a write to un-mapped memory.
If the shmem is torn down while the IRQ is active, and another thread
requests the channel again, we can end up with a channel that has had
its shmem unmapped.
By moving the map to start up and the unmap to teardown, we can let
the mailbox mechanism prevent re-entrance into the startup/teardown
functions.
Avoid doubly unmapping the region by removing the unmap in the
direct error handler for the request.
Assisted-by: Codex:gpt-5.4
Fixes: fa362ffafa51 ("mailbox: pcc: Always map the shared memory communication address")
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
drivers/mailbox/pcc.c | 48 +++++++++++++++++++++----------------------
1 file changed, 23 insertions(+), 25 deletions(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 636879ae1db7..da26578a8aab 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -360,7 +360,6 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
struct pcc_mbox_chan *
pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
{
- struct pcc_mbox_chan *pcc_mchan;
struct pcc_chan_info *pchan;
struct mbox_chan *chan;
int rc;
@@ -375,20 +374,10 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
return ERR_PTR(-EBUSY);
}
- pcc_mchan = &pchan->chan;
- pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
- pcc_mchan->shmem_size);
- if (!pcc_mchan->shmem)
- return ERR_PTR(-ENXIO);
-
rc = mbox_bind_client(chan, cl);
- if (rc) {
- iounmap(pcc_mchan->shmem);
- pcc_mchan->shmem = NULL;
- return ERR_PTR(rc);
- }
-
- return pcc_mchan;
+ if (rc)
+ return ERR_PTR(-ENXIO);
+ return &pchan->chan;
}
EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
@@ -400,19 +389,13 @@ EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
*/
void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
{
- struct mbox_chan *chan = pchan->mchan;
- struct pcc_chan_info *pchan_info;
- struct pcc_mbox_chan *pcc_mbox_chan;
+ struct mbox_chan *chan;
+ if (!pchan)
+ return;
+ chan = pchan->mchan;
if (!chan || !chan->cl)
return;
- pchan_info = chan->con_priv;
- pcc_mbox_chan = &pchan_info->chan;
- if (pcc_mbox_chan->shmem) {
- iounmap(pcc_mbox_chan->shmem);
- pcc_mbox_chan->shmem = NULL;
- }
-
mbox_free_channel(chan);
}
EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
@@ -462,9 +445,15 @@ static bool pcc_last_tx_done(struct mbox_chan *chan)
static int pcc_startup(struct mbox_chan *chan)
{
struct pcc_chan_info *pchan = chan->con_priv;
+ struct pcc_mbox_chan *pcc_mchan;
unsigned long irqflags;
int rc;
+ pcc_mchan = &pchan->chan;
+ pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
+ pcc_mchan->shmem_size);
+ if (pcc_mchan->shmem == NULL)
+ return -ENOMEM;
/*
* Clear and acknowledge any pending interrupts on responder channel
* before enabling the interrupt
@@ -479,6 +468,8 @@ static int pcc_startup(struct mbox_chan *chan)
if (unlikely(rc)) {
dev_err(chan->mbox->dev, "failed to register PCC interrupt %d\n",
pchan->plat_irq);
+ iounmap(pcc_mchan->shmem);
+ pcc_mchan->shmem = NULL;
return rc;
}
}
@@ -488,15 +479,22 @@ static int pcc_startup(struct mbox_chan *chan)
/**
* pcc_shutdown - Called from Mailbox Controller code. Used here
- * to free the interrupt.
+ * to free the interrupt and unmap the shared memory.
* @chan: Pointer to Mailbox channel to shutdown.
*/
static void pcc_shutdown(struct mbox_chan *chan)
{
struct pcc_chan_info *pchan = chan->con_priv;
+ struct pcc_mbox_chan *pcc_mbox_chan;
if (pchan->plat_irq > 0)
devm_free_irq(chan->mbox->dev, pchan->plat_irq, chan);
+
+ pcc_mbox_chan = &pchan->chan;
+ if (pcc_mbox_chan->shmem) {
+ iounmap(pcc_mbox_chan->shmem);
+ pcc_mbox_chan->shmem = NULL;
+ }
}
static const struct mbox_chan_ops pcc_chan_ops = {
--
2.43.0
next prev parent reply other threads:[~2026-07-21 17:53 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-21 17:52 [PATCH v45 0/7] MCTP over PCC Adam Young
2026-07-21 17:52 ` Adam Young [this message]
2026-07-21 17:52 ` [PATCH v45 2/7] mailbox/pcc.c: ignore errors on type 4 channels Adam Young
2026-07-22 8:55 ` Sudeep Holla
2026-07-22 17:04 ` Adam Young
2026-07-21 17:52 ` [PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients Adam Young
2026-07-22 9:07 ` Sudeep Holla
2026-07-22 17:07 ` Adam Young
2026-07-21 17:52 ` [PATCH v45 4/7] mailbox/pcc.c: add query channel function Adam Young
2026-07-21 17:52 ` [PATCH v45 5/7] mctp pcc: Implement MCTP over PCC Transport Adam Young
2026-07-21 17:52 ` [PATCH v45 6/7] synchronize IRQ before releasing shared memory Adam Young
2026-07-21 17:52 ` [PATCH v45 7/7] wrap pchan->chan_in_use in READ/WRITE_ONCE Adam Young
2026-07-22 8:50 ` Sudeep Holla
2026-07-22 18:58 ` Adam Young
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=20260721175258.87600-2-admiyo@os.amperecomputing.com \
--to=admiyo@os.amperecomputing.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=jassisinghbrar@gmail.com \
--cc=jk@codeconstruct.com.au \
--cc=kuba@kernel.org \
--cc=lihuisong@huawei.com \
--cc=linux-kernel@vger.kernel.org \
--cc=matt@codeconstruct.com.au \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sudeep.holla@arm.com \
--cc=sudeep.holla@kernel.org \
/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