[PATCH v45 0/7] MCTP over PCC

[PATCH v45 0/7] MCTP over PCC

[Adam Young]

From: Linux Bot <linuxbot@amperecomputing.com>

THis patch sereis is long lived, and the main
changelog will saty with the MCTP over PCC patch.
Recent changes in AI code review haved shown required
fixes in the PC mailbox layer. The main changes to this
patch series are the addition of those patches.

Adam Young (7):
  mailbox/pcc.c: shmem map/unmap startup/teardown
  mailbox/pcc.c: ignore errors on type 4 channels.
  mailbox/pcc.c: report errors for PCC clients
  mailbox/pcc.c:  add query channel function
  mctp pcc: Implement MCTP over PCC Transport
  synchronize IRQ before releasing shared memory
  wrap pchan->chan_in_use  in READ/WRITE_ONCE

 MAINTAINERS                 |   5 +
 drivers/mailbox/pcc.c       | 112 ++++++---
 drivers/net/mctp/Kconfig    |  15 ++
 drivers/net/mctp/Makefile   |   1 +
 drivers/net/mctp/mctp-pcc.c | 466 ++++++++++++++++++++++++++++++++++++
 include/acpi/pcc.h          |   9 +
 6 files changed, 576 insertions(+), 32 deletions(-)
 create mode 100644 drivers/net/mctp/mctp-pcc.c

-- 
2.43.0

[PATCH v45 1/7] mailbox/pcc.c: shmem map/unmap startup/teardown

[Adam Young]

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

[PATCH v45 2/7] mailbox/pcc.c: ignore errors on type 4 channels.

[Adam Young]

THE ACPI spec states:

"[The Error status register] Contains the processor relative address,
represented in Generic Address Structure (GAS) format, of the Error status
register. This field is ignored by the OSPM on slave channels"

Referring to type 4 channels.

https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html#hw-registers-based-communications-subspace-structure-type-5

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index da26578a8aab..3c059fe87ce4 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -270,6 +270,9 @@ static int pcc_mbox_error_check_and_clear(struct pcc_chan_info *pchan)
 	u64 val;
 	int ret;
 
+	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+		return 0;
+
 	ret = pcc_chan_reg_read(&pchan->error, &val);
 	if (ret)
 		return ret;
-- 
2.43.0

Re: [PATCH v45 2/7] mailbox/pcc.c: ignore errors on type 4 channels.

[Sudeep Holla]

On Tue, Jul 21, 2026 at 01:52:51PM -0400, Adam Young wrote:
> THE ACPI spec states:
> 
> "[The Error status register] Contains the processor relative address,
> represented in Generic Address Structure (GAS) format, of the Error status
> register. This field is ignored by the OSPM on slave channels"
> 

Does anyone have these error status register for Type 4 ? If not it can
be optimised with that check rather than channel type check. I would rather
avoiding adding checks on the type of the channel everywhere in the code.

-- 
Regards,
Sudeep

Re: [PATCH v45 2/7] mailbox/pcc.c: ignore errors on type 4 channels.

[Adam Young]

On 7/22/26 04:55, Sudeep Holla wrote:
> On Tue, Jul 21, 2026 at 01:52:51PM -0400, Adam Young wrote:
>> THE ACPI spec states:
>>
>> "[The Error status register] Contains the processor relative address,
>> represented in Generic Address Structure (GAS) format, of the Error status
>> register. This field is ignored by the OSPM on slave channels"
>>
> Does anyone have these error status register for Type 4 ? If not it can
> be optimised with that check rather than channel type check. I would rather
> avoiding adding checks on the type of the channel everywhere in the code.
>
The requirement that errors are ignored is strange and I have no 
visibility into why it is included.  It may be completely safe to ignore 
this rule. We have not tripped over it in our hardware, which is why I 
did not address it before. I cannot speak for any other 
implementations.  However, I think the exclusion needs to be here for 
compliance with the protocol, otherwise we might end up with a case that 
we cannot support.  If you can suggest an alternate way to implement the 
specification, please let me know and I will try to encode it.

[PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients

[Adam Young]

The tx_done callback function has a return code (rc) parameter
that the tx_done callback can use to determine how to handle an error.
However the IRQ handler was not setting that value if there is an error.

The following clients are affected:

drivers/acpi/cppc_acpi.c
drivers/i2c/busses/i2c-xgene-slimpro.c
drivers/hwmon/xgene-hwmon.c
drivers/soc/hisilicon/kunpeng_hccs.c
drivers/devfreq/hisi_uncore_freq.c

All of these only use the error code to report, so they
are expecting an error code to come thorugh, but they
do not modify behavior based on this code.

In the case of an error code in the IRQ, the handler was returning
IRQ_NONE which is not correct:  the IRQ handler was matched
to the IRQ.  This mean that multiple error codes returned from
a PCC triggered interrupt would end up disabling the device.

In addition, if the error code IRQ was coming from a Type4 Device that was
expecting an IRQ response, that device would then be hung.

Fixes: c45ded7e1135 ("mailbox: pcc: Add support for PCCT extended PCC subspaces(type 3/4)")
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>

---
---
 drivers/mailbox/pcc.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 3c059fe87ce4..b1c7d3e4c5e3 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -317,6 +317,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
+	int rc;
 
 	pchan = chan->con_priv;
 
@@ -330,8 +331,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	if (!pcc_mbox_cmd_complete_check(pchan))
 		return IRQ_NONE;
 
-	if (pcc_mbox_error_check_and_clear(pchan))
-		return IRQ_NONE;
+	rc = pcc_mbox_error_check_and_clear(pchan);
 
 	/*
 	 * Clear this flag after updating interrupt ack register and just
@@ -340,8 +340,9 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	 * required to avoid any possible race in updatation of this flag.
 	 */
 	pchan->chan_in_use = false;
-	mbox_chan_received_data(chan, NULL);
-	mbox_chan_txdone(chan, 0);
+	if (!rc)
+		mbox_chan_received_data(chan, NULL);
+	mbox_chan_txdone(chan, rc);
 
 	pcc_chan_acknowledge(pchan);
 
-- 
2.43.0

Re: [PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients

[Sudeep Holla]

On Tue, Jul 21, 2026 at 01:52:52PM -0400, Adam Young wrote:
> The tx_done callback function has a return code (rc) parameter
> that the tx_done callback can use to determine how to handle an error.
> However the IRQ handler was not setting that value if there is an error.
> 
> The following clients are affected:
> 
> drivers/acpi/cppc_acpi.c
> drivers/i2c/busses/i2c-xgene-slimpro.c
> drivers/hwmon/xgene-hwmon.c
> drivers/soc/hisilicon/kunpeng_hccs.c
> drivers/devfreq/hisi_uncore_freq.c
> 
> All of these only use the error code to report, so they
> are expecting an error code to come thorugh, but they
> do not modify behavior based on this code.
> 
> In the case of an error code in the IRQ, the handler was returning
> IRQ_NONE which is not correct:  the IRQ handler was matched
> to the IRQ.  This mean that multiple error codes returned from
> a PCC triggered interrupt would end up disabling the device.
> 
> In addition, if the error code IRQ was coming from a Type4 Device that was
> expecting an IRQ response, that device would then be hung.
> 

I recall reviewing something similar and may have had comments, although I
cannot locate them at the moment, thanks to your random patch inclusion
exclusion scheme in this MCTP over PCC.

Could you please avoid continuing the MCTP-over-PCC series in its current
form? In the previous v44 revisions, PCC changes appeared to be removed in
some versions and replaced with unrelated changes in others, which has made
the series difficult to follow and review.

Please post PCC changes that are independent of MCTP as a separate series.
Where the MCTP implementation depends directly on those PCC changes, they may
be posted together. However, the current series appears to include several
unrelated cleanups described as part of the MCTP-over-PCC work.

To keep the review manageable, please separate those changes going forward. I
may need to NACK future revisions if unrelated PCC cleanups continue to be
bundled into this series. I will be away and would like to review any PCC
code, so your patience will be much appreciated.

-- 
Regards,
Sudeep

Re: [PATCH v45 3/7] mailbox/pcc.c: report errors for PCC clients

[Adam Young]

On 7/22/26 05:07, Sudeep Holla wrote:
> On Tue, Jul 21, 2026 at 01:52:52PM -0400, Adam Young wrote:
>> The tx_done callback function has a return code (rc) parameter
>> that the tx_done callback can use to determine how to handle an error.
>> However the IRQ handler was not setting that value if there is an error.
>>
>> The following clients are affected:
>>
>> drivers/acpi/cppc_acpi.c
>> drivers/i2c/busses/i2c-xgene-slimpro.c
>> drivers/hwmon/xgene-hwmon.c
>> drivers/soc/hisilicon/kunpeng_hccs.c
>> drivers/devfreq/hisi_uncore_freq.c
>>
>> All of these only use the error code to report, so they
>> are expecting an error code to come thorugh, but they
>> do not modify behavior based on this code.
>>
>> In the case of an error code in the IRQ, the handler was returning
>> IRQ_NONE which is not correct:  the IRQ handler was matched
>> to the IRQ.  This mean that multiple error codes returned from
>> a PCC triggered interrupt would end up disabling the device.
>>
>> In addition, if the error code IRQ was coming from a Type4 Device that was
>> expecting an IRQ response, that device would then be hung.
>>
> I recall reviewing something similar and may have had comments, although I
> cannot locate them at the moment, thanks to your random patch inclusion
> exclusion scheme in this MCTP over PCC.

That was Re: [PATCH v02] mailbox: pcc: report errors for PCC clients


>
> Could you please avoid continuing the MCTP-over-PCC series in its current
> form? In the previous v44 revisions, PCC changes appeared to be removed in
> some versions and replaced with unrelated changes in others, which has made
> the series difficult to follow and review.
>
> Please post PCC changes that are independent of MCTP as a separate series.
> Where the MCTP implementation depends directly on those PCC changes, they may
> be posted together. However, the current series appears to include several
> unrelated cleanups described as part of the MCTP-over-PCC work.
>
> To keep the review manageable, please separate those changes going forward. I
> may need to NACK future revisions if unrelated PCC cleanups continue to be
> bundled into this series. I will be away and would like to review any PCC
> code, so your patience will be much appreciated.

Everything here is related to, and required by, MCTP over PCC.  I had 
posted some of the PCC patches in stand alone changes, but got feedback 
from the network  reviewers that MCTP over PCC could not go in until 
these were fixed.  MCTP over PCC seems to be one of the first drivers, 
if not the first to make use of the Type 4 interface, and getting fixes 
in place to make that work is prerequisite.

So, apologies for the earlier separate of patches.  I am trying to 
follow the guidelines for getting the patch into the kernel, but this 
spans two very different subsystems with different maintainers and 
guidelines. This is one of the reasons this patch series is at version 
45, and why I am still working on this 2+ years after the initial 
submission.

I am going to keep all of this work together.  It is the only way I can 
be sure that all of the work is reviewed end to end, and maintain any 
semblance of order in managing the patches.

The inclusion of AI in the review process has brought a sereis of new 
issues to light.  Before the kernel started producing automated AI 
reviews, the set of reviewers had reduced their concerns to fixes within 
the MCTP Driver.  That expanded earlier this year to numerous changes 
required in the PCC mailbox.  Some of those changes will have the added 
benefit of fixing corresponding bugs that have not been triggered 
elsewhere in the PCC drivers.  Her's a brief synopsis:

shmem map/unmap was added to the PCC layer for the MCTP Driver over a 
year ago.  It was based on a change I suggested for MCTP driver 
start/stop.  These operations are much more likely to be called in a 
network device that will likely be brought up and down on a running 
server, than most HW devices that are only started at machine bringup 
and stopped when the machine turns down.

The changes to  ignore errors on type 4 channels was based on your 
feedback on a different patch. MCTP over PCC will depend on the Type 4 
device functioning correctly, to include error reporting.

In the message Re: [PATCH v02] mailbox: pcc: report errors for PCC clients

The discussion  was:

>> I think we may have to skip the check inside 
>> pcc_mbox_error_check_and_clear()
>> for Type 4 channel as the spec expects OSPM to ignore it. It is a 
>> separate
>> fix, just noting that here.
>
> I think that should be in this patch, for correctness.  It is a small 
> enough change.  I'll update.
>

Actually, it is a fix in its own right, and can be merged regardless of 
this patch, so:
https://lore.kernel.org/lkml/20260604163306.160017-1-admiyo@os.amperecomputing.com/ 


Which got no feedback.


The query channel patch is required for MCTP.  While no other driver 
will need it yet, it does prevent a false-start situation that may help 
other drivers.

I did order the patches to post a couple fixes to PCC after the MCTP 
driver, as they are fixes that affect all drivers, under the general 
idea that general fixes for a subsystem should not hold up bugs in the 
subsystem.  Those are synchronize-IRQ-before-releasing-shared-memory and 
wrap-pchan-chan_in_use-in-READ-WRITE_ONCE.  Again, these were exposed by 
AI code review during the MCTP process, and me splitting them out 
elsewhere would really make it hard for me to track.  A network 
maintainer could argue that the sync IRQ fix really should go in before 
the MCTP driver.

So, while I do appreciate the additional load this puts on you, I think 
you will agree that this set of changes needs to be kept together for 
the larger reviewer community to be able to see.  It is hard  to make 
things easy for everyone.  I appreciate your patience and diligence in 
the review process. I hope you can get to these reviews before you 
disappear.

[PATCH v45 4/7] mailbox/pcc.c: add query channel function

[Adam Young]

Drivers need information about a channel prior to creating a channel
or they risk triggering message delivery on the remote side of a
connection.

Add PCC channel type to records and expose PCC channel type to client.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c | 45 +++++++++++++++++++++++++++++++++++++++++++
 include/acpi/pcc.h    |  9 +++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index b1c7d3e4c5e3..b9a01bfdc95d 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -349,6 +349,50 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 	return IRQ_HANDLED;
 }
 
+/**
+ * pcc_mbox_query_channel - returns information about the channel
+ *              without activating the channel.
+ *
+ * @q_chan: a pointer to an already allocated struct pcc_mbox_chan
+ *              that will be populated with the channel data.
+ * @subspace_id: The PCC Subspace index as parsed in the PCC client
+ *      ACPI package. This is used to lookup the array of PCC
+ *      subspaces as parsed by the PCC Mailbox controller.
+ *
+ * Return: 0 upon success or non-zero upon error.
+ */
+int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+	struct pcc_mbox_chan *pcc_mchan;
+	struct pcc_chan_info *pchan;
+	struct mbox_chan *chan;
+
+	if (!q_chan)
+		return -EINVAL;
+
+	if (subspace_id < 0 || subspace_id >= pcc_chan_count)
+		return -ENOENT;
+	pchan = chan_info + subspace_id;
+	chan = pchan->chan.mchan;
+	if (IS_ERR(chan)) {
+		pr_err("Channel not found for idx: %d\n", subspace_id);
+		return -EBUSY;
+	}
+	pcc_mchan = &pchan->chan;
+
+	q_chan->shmem_base_addr = pcc_mchan->shmem_base_addr;
+	q_chan->shmem = NULL;
+	q_chan->shmem_size = pcc_mchan->shmem_size;
+	q_chan->latency = pcc_mchan->latency;
+	q_chan->max_access_rate = pcc_mchan->max_access_rate;
+	q_chan->min_turnaround_time = pcc_mchan->min_turnaround_time;
+	q_chan->type = pcc_mchan->type;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_query_channel);
+
 /**
  * pcc_mbox_request_channel - PCC clients call this function to
  *		request a pointer to their PCC subspace, from which they
@@ -833,6 +877,7 @@ static int pcc_mbox_probe(struct platform_device *pdev)
 		pcc_parse_subspace_shmem(pchan, pcct_entry);
 
 		pchan->type = pcct_entry->type;
+		pchan->chan.type = pcct_entry->type;
 		pcct_entry = (struct acpi_subtable_header *)
 			((unsigned long) pcct_entry + pcct_entry->length);
 	}
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 840bfc95bae3..bf97f407683d 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -8,6 +8,7 @@
 
 #include <linux/mailbox_controller.h>
 #include <linux/mailbox_client.h>
+#include <linux/acpi.h>
 
 struct pcc_mbox_chan {
 	struct mbox_chan *mchan;
@@ -17,6 +18,7 @@ struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
+	enum acpi_pcct_type type;
 };
 
 /* Generic Communications Channel Shared Memory Region */
@@ -37,6 +39,8 @@ struct pcc_mbox_chan {
 extern struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
 extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
+extern int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id);
 #else
 static inline struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
@@ -44,6 +48,11 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	return ERR_PTR(-ENODEV);
 }
 static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
+static inline int
+pcc_mbox_query_channel(struct pcc_mbox_chan *q_chan, int subspace_id)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* _PCC_H */
-- 
2.43.0

[PATCH v45 5/7] mctp pcc: Implement MCTP over PCC Transport

[Adam Young]

Implementation of network driver for
Management Component Transport Protocol(MCTP)
over Platform Communication Channel(PCC)

DMTF DSP:0292
Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0292.pdf

The transport mechanism is called Platform Communication Channels (PCC)
is part of the ACPI spec:

Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html

The PCC mechanism is managed via a mailbox implemented at
drivers/mailbox/pcc.c

MCTP devices are specified via ACPI by entries in DSDT/SSDT and
reference channels specified in the PCCT. Messages are sent on a type
3 and received on a type 4 channel.  Communication with other devices
use the PCC based doorbell mechanism; a shared memory segment with a
corresponding interrupt and a memory register used to trigger remote
interrupts.

The shared buffer must be at least 68 bytes long as that is the minimum
MTU as defined by the MCTP specification.

Unlike the existing PCC Type 2 based drivers, the mssg parameter to
mbox_send_msg is actively used. The data section of the struct sk_buff
that contains the outgoing packet is sent to the mailbox, already
properly formatted as a PCC exctended message.

If the mailbox ring buffer is full, the driver stops the incoming
packet queues until a message has been sent, freeing space in the
ring buffer.

When the Type 3 channel outbox receives a txdone response interrupt,
it consumes the outgoing sk_buff, allowing it to be freed.

Bringing up an interface creates the channel between the network driver
and the mailbox driver. This enables communication with the remote
endpoint, to include the receipt of new messages. Bringing down an
interface removes the channel, and no new messages can be delivered.
Stopping the interface will leave any packets that are cached in the
mailbox ringbuffer. They cannot safely be freed until the PCC mailbox
attempts to deliver them and has removed them from the ring buffer.

PCC is based on a shared buffer and a set of I/O mapped memory locations
that the Spec calls registers.  This mechanism exists regardless of the
existence of the driver. If the user has the ability to map these
physical location to virtual locations, they have the ability to drive the
hardware.  Thus, there is a security aspect to this mechanism that extends
beyond the responsibilities of the operating system.

If the hardware does not expose the PCC in the ACPI table, this device
will never be enabled. Thus it is only an issue on hardware that does
support PCC. In that case, it is up to the remote controller to sanitize
communication; MCTP will be exposed as a socket interface, and userland
can send any crafted packet it wants. It would also be incumbent on
the hardware manufacturer to allow the end user to disable MCTP over PCC
communication if they did not want to expose it.

Although the config Allow 32Bit builds, they are untested.

Link: https://www.dmtf.org/sites/default/files/standards/documents/DSP0292_1.0.0WIP50.pdf
Link: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html
Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>

---

Previous Version:
https://lore.kernel.org/lkml/20260522193610.234166-1-admiyo@os.amperecomputing.com/

Changes from Previous version

- Compares inbox and outbox buffer size to get smallest MTU
- uses mctp-pcc query channel information without opening channel.
  Opening the channel can trigger the sending of
  a message from the remote side before the driver
  is ready to read it.  Take advantage of the API that allows
  querying of the channel data without opening the channel.
-  Sets network queue to default length instead of 0
- Removed reference to MCTP spec
- Remove reference to endianess
- Rebased on top of PCC mailbox change for atomit startup/teardown

Remove reference to endianness

remove reference to DSP0256
---
 MAINTAINERS                 |   5 +
 drivers/net/mctp/Kconfig    |  15 ++
 drivers/net/mctp/Makefile   |   1 +
 drivers/net/mctp/mctp-pcc.c | 466 ++++++++++++++++++++++++++++++++++++
 4 files changed, 487 insertions(+)
 create mode 100644 drivers/net/mctp/mctp-pcc.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 6940aa3d498b..9f48dee0f4c6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15582,6 +15582,11 @@ F:	include/net/mctpdevice.h
 F:	include/net/netns/mctp.h
 F:	net/mctp/
 
+MANAGEMENT COMPONENT TRANSPORT PROTOCOL (MCTP) over PCC (MCTP-PCC) Driver
+M:	Adam Young <admiyo@os.amperecomputing.com>
+S:	Maintained
+F:	drivers/net/mctp/mctp-pcc.c
+
 MAPLE TREE
 M:	Liam R. Howlett <liam@infradead.org>
 R:	Alice Ryhl <aliceryhl@google.com>
diff --git a/drivers/net/mctp/Kconfig b/drivers/net/mctp/Kconfig
index cf325ab0b1ef..c9b8f38ff0fb 100644
--- a/drivers/net/mctp/Kconfig
+++ b/drivers/net/mctp/Kconfig
@@ -47,6 +47,21 @@ config MCTP_TRANSPORT_I3C
 	  A MCTP protocol network device is created for each I3C bus
 	  having a "mctp-controller" devicetree property.
 
+config MCTP_TRANSPORT_PCC
+	tristate "MCTP PCC transport"
+	depends on ACPI
+	depends on PCC
+	depends on CPU_LITTLE_ENDIAN
+	help
+	  Provides a driver to access MCTP devices over PCC transport,
+	  A MCTP protocol network device is created via ACPI for each
+	  entry in the DSDT/SSDT that matches the identifier. The Platform
+	  communication channels are selected from the corresponding
+	  entries in the PCCT.
+
+	  Say y here if you need to connect to MCTP endpoints over PCC. To
+	  compile as a module, use m; the module will be called mctp-pcc.
+
 config MCTP_TRANSPORT_USB
 	tristate "MCTP USB transport"
 	depends on USB
diff --git a/drivers/net/mctp/Makefile b/drivers/net/mctp/Makefile
index c36006849a1e..0a591299ffa9 100644
--- a/drivers/net/mctp/Makefile
+++ b/drivers/net/mctp/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_MCTP_SERIAL) += mctp-serial.o
 obj-$(CONFIG_MCTP_TRANSPORT_I2C) += mctp-i2c.o
 obj-$(CONFIG_MCTP_TRANSPORT_I3C) += mctp-i3c.o
+obj-$(CONFIG_MCTP_TRANSPORT_PCC) += mctp-pcc.o
 obj-$(CONFIG_MCTP_TRANSPORT_USB) += mctp-usb.o
diff --git a/drivers/net/mctp/mctp-pcc.c b/drivers/net/mctp/mctp-pcc.c
new file mode 100644
index 000000000000..57c4f3b21513
--- /dev/null
+++ b/drivers/net/mctp/mctp-pcc.c
@@ -0,0 +1,466 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mctp-pcc.c - Driver for MCTP over PCC.
+ * Copyright (c) 2024-2026, Ampere Computing LLC
+ *
+ */
+
+/* Implementation of
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0292.pdf
+ */
+
+#include <linux/acpi.h>
+#include <linux/hrtimer.h>
+#include <linux/if_arp.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_client.h>
+#include <linux/module.h>
+#include <linux/netdevice.h>
+#include <linux/platform_device.h>
+#include <linux/skbuff.h>
+#include <linux/string.h>
+
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <acpi/acrestyp.h>
+#include <acpi/actbl.h>
+#include <acpi/pcc.h>
+#include <net/mctp.h>
+#include <net/mctpdevice.h>
+#include <net/pkt_sched.h>
+
+#define MCTP_SIGNATURE          "MCTP"
+#define MCTP_SIGNATURE_LENGTH   (sizeof(MCTP_SIGNATURE) - 1)
+#define MCTP_MIN_MTU            68
+#define PCC_HEADER_SIZE         sizeof(struct acpi_pcct_ext_pcc_shared_memory)
+#define MCTP_PCC_MIN_SIZE       (PCC_HEADER_SIZE + MCTP_MIN_MTU)
+#define PCC_EXTRA_LEN           (PCC_HEADER_SIZE - sizeof(pcc_header.command))
+struct mctp_pcc_mailbox {
+	u32 index;
+	struct pcc_mbox_chan *chan;
+	struct mbox_client client;
+};
+
+/* The netdev structure. One of these per PCC adapter. */
+struct mctp_pcc_ndev {
+	struct net_device *ndev;
+	struct acpi_device *acpi_device;
+	struct mctp_pcc_mailbox inbox;
+	struct mctp_pcc_mailbox outbox;
+};
+
+static void mctp_pcc_client_rx_callback(struct mbox_client *cl, void *mssg)
+{
+	struct acpi_pcct_ext_pcc_shared_memory pcc_header;
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	struct mctp_pcc_mailbox *inbox;
+	struct mctp_skb_cb *cb;
+	struct sk_buff *skb;
+	int size;
+
+	mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, inbox.client);
+	inbox = &mctp_pcc_ndev->inbox;
+	memcpy_fromio(&pcc_header, inbox->chan->shmem, sizeof(pcc_header));
+
+	// The message must at least have the PCC command indicating it is an MCTP
+	// message followed by the MCTP header, or we have a malformed message.
+	if (pcc_header.length < sizeof(pcc_header.command) + sizeof(struct mctp_hdr))
+		goto error;
+
+	// If the reported size is larger than the shared memory minus headers,
+	// something is wrong and treat the buffer as corrupted data.
+	if (pcc_header.length > inbox->chan->shmem_size - PCC_EXTRA_LEN)
+		goto error;
+
+	if (memcmp(&pcc_header.command, MCTP_SIGNATURE, MCTP_SIGNATURE_LENGTH) != 0)
+		goto error;
+
+	size = pcc_header.length + PCC_EXTRA_LEN;
+	skb = netdev_alloc_skb(mctp_pcc_ndev->ndev, size);
+	if (!skb)
+		goto error;
+
+	skb_put(skb, size);
+	skb->protocol = htons(ETH_P_MCTP);
+	memcpy_fromio(skb->data, inbox->chan->shmem, size);
+	dev_dstats_rx_add(mctp_pcc_ndev->ndev, size);
+	skb_pull(skb, sizeof(pcc_header));
+	skb_reset_mac_header(skb);
+	skb_reset_network_header(skb);
+	cb = __mctp_cb(skb);
+	cb->halen = 0;
+	netif_rx(skb);
+	return;
+
+error:
+	dev_dstats_rx_dropped(mctp_pcc_ndev->ndev);
+}
+
+static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device *ndev)
+{
+	struct acpi_pcct_ext_pcc_shared_memory *pcc_header;
+	struct mctp_pcc_ndev *mpnd = netdev_priv(ndev);
+	int len = skb->len;
+
+	if (skb_cow_head(skb, sizeof(*pcc_header)))
+		goto error;
+
+	pcc_header = skb_push(skb, sizeof(*pcc_header));
+	pcc_header->signature = PCC_SIGNATURE | mpnd->outbox.index;
+	pcc_header->flags = PCC_CMD_COMPLETION_NOTIFY;
+	memcpy(&pcc_header->command, MCTP_SIGNATURE, MCTP_SIGNATURE_LENGTH);
+	pcc_header->length = len + MCTP_SIGNATURE_LENGTH;
+
+	if (skb->len > mpnd->outbox.chan->shmem_size)
+		goto error;
+
+	/*
+	 * There is a possibility that the mailbox can be cleared on
+	 * another thread. If that is the case, and we don't restart
+	 * the queue, it will remain permanently stopped.
+	 * Stopping the queue before attempting to send the message
+	 * allows us to always restart it if mbox_send_message succeeds.
+	 */
+	netif_stop_queue(ndev);
+	if (mbox_send_message(mpnd->outbox.chan->mchan, skb) >= 0) {
+		netif_wake_queue(ndev);
+	} else {
+		// Remove the header in case it gets sent again
+		skb_pull(skb, sizeof(*pcc_header));
+		return NETDEV_TX_BUSY;
+	}
+	return NETDEV_TX_OK;
+
+error:
+	dev_dstats_tx_dropped(ndev);
+	kfree_skb(skb);
+	return NETDEV_TX_OK;
+}
+
+static void mctp_pcc_tx_prepare(struct mbox_client *cl, void *mssg)
+{
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	struct mctp_pcc_mailbox *outbox;
+	struct sk_buff *skb = mssg;
+
+	mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, outbox.client);
+	outbox = &mctp_pcc_ndev->outbox;
+
+	/* The PCC Mailbox typically does not make use of the mssg pointer
+	 * The mctp-over pcc driver is the only client that uses it.
+	 * This value should always be non-null; it is possible
+	 * that a change in the Mailbox level will break that assumption.
+	 */
+	if (!skb) {
+		netdev_warn_once(mctp_pcc_ndev->ndev,
+				 "%s called with null message.\n", __func__);
+		return;
+	}
+	memcpy_toio(outbox->chan->shmem, skb->data, skb->len);
+}
+
+static void mctp_pcc_tx_done(struct mbox_client *c, void *mssg, int rc)
+{
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	struct pcpu_dstats *dstats;
+	struct sk_buff *skb = mssg;
+	unsigned long flags;
+
+	/*
+	 * If there is a packet in flight during driver cleanup
+	 * It may have been freed already.
+	 */
+	if (!mssg)
+		return;
+	mctp_pcc_ndev = container_of(c, struct mctp_pcc_ndev, outbox.client);
+
+	/* Use an IRQ safe update as this is called from HARD IRQ instead of
+	 * dev_dstats_tx_add(mctp_pcc_ndev->ndev, skb->len);
+	 */
+	dstats = this_cpu_ptr(mctp_pcc_ndev->ndev->dstats);
+	flags = u64_stats_update_begin_irqsave(&dstats->syncp);
+
+	if (rc) {
+		u64_stats_inc(&dstats->tx_drops);
+	} else {
+		u64_stats_inc(&dstats->tx_packets);
+		u64_stats_add(&dstats->tx_bytes, skb->len);
+	}
+	u64_stats_update_end_irqrestore(&dstats->syncp, flags);
+	dev_consume_skb_any(skb);
+	netif_wake_queue(mctp_pcc_ndev->ndev);
+}
+
+static int mctp_pcc_open(struct net_device *ndev)
+{
+	struct mctp_pcc_ndev *mctp_pcc_ndev = netdev_priv(ndev);
+	struct mctp_pcc_mailbox *outbox, *inbox;
+
+	outbox = &mctp_pcc_ndev->outbox;
+	inbox = &mctp_pcc_ndev->inbox;
+
+	outbox->chan = pcc_mbox_request_channel(&outbox->client, outbox->index);
+	if (IS_ERR(outbox->chan))
+		return PTR_ERR(outbox->chan);
+	if (outbox->chan->shmem_size < MCTP_PCC_MIN_SIZE) {
+		pcc_mbox_free_channel(outbox->chan);
+		return -EINVAL;
+	}
+
+	inbox->client.rx_callback = mctp_pcc_client_rx_callback;
+	inbox->chan = pcc_mbox_request_channel(&inbox->client, inbox->index);
+	if (IS_ERR(inbox->chan)) {
+		pcc_mbox_free_channel(outbox->chan);
+		return PTR_ERR(inbox->chan);
+	}
+	if (inbox->chan->shmem_size < MCTP_PCC_MIN_SIZE) {
+		pcc_mbox_free_channel(outbox->chan);
+		pcc_mbox_free_channel(inbox->chan);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int mctp_pcc_stop(struct net_device *ndev)
+{
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	unsigned int count, idx;
+	struct mbox_chan *chan;
+	struct sk_buff *skb;
+
+	mctp_pcc_ndev = netdev_priv(ndev);
+	chan = mctp_pcc_ndev->outbox.chan->mchan;
+	pcc_mbox_free_channel(mctp_pcc_ndev->inbox.chan);
+	mctp_pcc_ndev->inbox.chan = NULL;
+	scoped_guard(spinlock_irqsave, &chan->lock) {
+		if (chan->active_req !=  MBOX_NO_MSG) {
+			skb = chan->active_req;
+			chan->active_req = MBOX_NO_MSG;
+			dev_dstats_tx_dropped(ndev);
+			dev_consume_skb_any(skb);
+		}
+		while (chan->msg_count > 0) {
+			count = chan->msg_count;
+			idx = chan->msg_free;
+			if (idx >= count)
+				idx -= count;
+			else
+				idx += MBOX_TX_QUEUE_LEN - count;
+			skb = chan->msg_data[idx];
+			dev_dstats_tx_dropped(ndev);
+			dev_consume_skb_any(skb);
+			chan->msg_count--;
+		}
+	}
+	pcc_mbox_free_channel(mctp_pcc_ndev->outbox.chan);
+	mctp_pcc_ndev->outbox.chan = NULL;
+	/*
+	 * If the queue was stopped because the ring buffer was full
+	 * we can restart it here as we now know the ring buffer has
+	 * been emptied and the queue can be used again if the
+	 * netdev is re-opened.
+	 */
+	netif_wake_queue(mctp_pcc_ndev->ndev);
+	return 0;
+}
+
+static const struct net_device_ops mctp_pcc_netdev_ops = {
+	.ndo_open = mctp_pcc_open,
+	.ndo_stop = mctp_pcc_stop,
+	.ndo_start_xmit = mctp_pcc_tx,
+};
+
+static void mctp_pcc_setup(struct net_device *ndev)
+{
+	ndev->type = ARPHRD_MCTP;
+	ndev->hard_header_len = sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+	ndev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
+	ndev->flags = IFF_NOARP;
+	ndev->netdev_ops = &mctp_pcc_netdev_ops;
+	ndev->needs_free_netdev = true;
+	ndev->pcpu_stat_type = NETDEV_PCPU_STAT_DSTATS;
+}
+
+struct mctp_pcc_lookup_context {
+	int index;
+	u32 inbox_index;
+	u32 outbox_index;
+};
+
+static acpi_status lookup_pcct_indices(struct acpi_resource *ares,
+				       void *context)
+{
+	struct mctp_pcc_lookup_context *luc = context;
+	struct acpi_resource_address32 *addr;
+
+	if (ares->type != ACPI_RESOURCE_TYPE_ADDRESS32)
+		return AE_OK;
+
+	addr = ACPI_CAST_PTR(struct acpi_resource_address32, &ares->data);
+	switch (luc->index) {
+	case 0:
+		luc->outbox_index = addr[0].address.minimum;
+		break;
+	case 1:
+		luc->inbox_index = addr[0].address.minimum;
+		break;
+	default:
+		return AE_ERROR;
+	}
+	luc->index++;
+	return AE_OK;
+}
+
+static void mctp_cleanup_netdev(void *data)
+{
+	struct net_device *ndev = data;
+
+	mctp_unregister_netdev(ndev);
+}
+
+static int check_channel_types(struct mctp_pcc_ndev *mctp_pcc_ndev)
+{
+	struct mctp_pcc_mailbox *outbox;
+	struct mctp_pcc_mailbox *inbox;
+	struct pcc_mbox_chan chan;
+	int actual_type;
+
+	outbox = &mctp_pcc_ndev->outbox;
+	if (pcc_mbox_query_channel(&chan, outbox->index))
+		return -EINVAL;
+	actual_type = chan.type;
+	if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_MASTER_SUBSPACE) {
+		pr_err("MCTP-PCC outbox channel wrong type: %d", actual_type);
+		return -EINVAL;
+	}
+
+	inbox = &mctp_pcc_ndev->inbox;
+	if (pcc_mbox_query_channel(&chan, inbox->index))
+		return -EINVAL;
+	actual_type = chan.type;
+	if (actual_type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) {
+		pr_err("MCTP-PCC inbox channel wrong type: %d", actual_type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int initialize_mtu(struct net_device *ndev)
+{
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	struct mctp_pcc_mailbox *outbox;
+	struct mctp_pcc_mailbox *inbox;
+	struct pcc_mbox_chan out_chan;
+	struct pcc_mbox_chan in_chan;
+	int mctp_pcc_max_mtu;
+	int inbox_max_mtu;
+
+	mctp_pcc_ndev = netdev_priv(ndev);
+	outbox = &mctp_pcc_ndev->outbox;
+	if (pcc_mbox_query_channel(&out_chan, outbox->index))
+		return -EINVAL;
+	if (out_chan.shmem_size < MCTP_MIN_MTU + sizeof(struct acpi_pcct_ext_pcc_shared_memory))
+		return -EINVAL;
+	mctp_pcc_max_mtu = out_chan.shmem_size - sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+	inbox = &mctp_pcc_ndev->inbox;
+	if (pcc_mbox_query_channel(&in_chan, inbox->index))
+		return -EINVAL;
+	if (in_chan.shmem_size < MCTP_MIN_MTU + sizeof(struct acpi_pcct_ext_pcc_shared_memory))
+		return -EINVAL;
+	inbox_max_mtu = in_chan.shmem_size - sizeof(struct acpi_pcct_ext_pcc_shared_memory);
+
+	if (inbox_max_mtu < mctp_pcc_max_mtu)
+		mctp_pcc_max_mtu = inbox_max_mtu;
+
+	ndev->mtu = MCTP_MIN_MTU;
+	ndev->max_mtu = mctp_pcc_max_mtu;
+	ndev->min_mtu = MCTP_MIN_MTU;
+
+	return 0;
+}
+
+static int mctp_pcc_driver_add(struct acpi_device *acpi_dev)
+{
+	struct mctp_pcc_lookup_context context = {0};
+	struct mctp_pcc_ndev *mctp_pcc_ndev;
+	struct device *dev = &acpi_dev->dev;
+	struct net_device *ndev;
+	acpi_handle dev_handle;
+	acpi_status status;
+	char name[32];
+	int rc;
+
+	dev_dbg(dev, "Adding mctp_pcc device for HID %s\n",
+		acpi_device_hid(acpi_dev));
+	dev_handle = acpi_device_handle(acpi_dev);
+	status = acpi_walk_resources(dev_handle, "_CRS", lookup_pcct_indices,
+				     &context);
+	if (!ACPI_SUCCESS(status)) {
+		dev_err(dev, "FAILED to lookup PCC indexes from CRS\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Ensure we have exactly 2 channels: an outbox and an inbox.
+	 */
+	if (context.index != 2)
+		return -EINVAL;
+
+	snprintf(name, sizeof(name), "mctppcc%d", context.inbox_index);
+	ndev = alloc_netdev(sizeof(*mctp_pcc_ndev), name, NET_NAME_PREDICTABLE,
+			    mctp_pcc_setup);
+	if (!ndev)
+		return -ENOMEM;
+
+	mctp_pcc_ndev = netdev_priv(ndev);
+	mctp_pcc_ndev->inbox.index = context.inbox_index;
+	mctp_pcc_ndev->inbox.client.dev = dev;
+	mctp_pcc_ndev->outbox.index = context.outbox_index;
+	mctp_pcc_ndev->outbox.client.dev = dev;
+
+	mctp_pcc_ndev->outbox.client.tx_prepare = mctp_pcc_tx_prepare;
+	mctp_pcc_ndev->outbox.client.tx_done = mctp_pcc_tx_done;
+	mctp_pcc_ndev->acpi_device = acpi_dev;
+	mctp_pcc_ndev->ndev = ndev;
+	acpi_dev->driver_data = mctp_pcc_ndev;
+	rc = check_channel_types(mctp_pcc_ndev);
+	if (rc != 0)
+		goto free_netdev;
+
+	rc = initialize_mtu(ndev);
+	if (rc)
+		goto free_netdev;
+
+	rc = mctp_register_netdev(ndev, NULL, MCTP_PHYS_BINDING_PCC);
+	if (rc)
+		goto free_netdev;
+
+	return devm_add_action_or_reset(dev, mctp_cleanup_netdev, ndev);
+free_netdev:
+	free_netdev(ndev);
+	return rc;
+}
+
+static const struct acpi_device_id mctp_pcc_device_ids[] = {
+	{ "DMT0001" },
+	{}
+};
+
+static struct acpi_driver mctp_pcc_driver = {
+	.name = "mctp_pcc",
+	.class = "Unknown",
+	.ids = mctp_pcc_device_ids,
+	.ops = {
+		.add = mctp_pcc_driver_add,
+	},
+};
+
+module_acpi_driver(mctp_pcc_driver);
+
+MODULE_DEVICE_TABLE(acpi, mctp_pcc_device_ids);
+
+MODULE_DESCRIPTION("MCTP PCC ACPI device");
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Adam Young <admiyo@os.amperecomputing.com>");
-- 
2.43.0

[PATCH v45 6/7] synchronize IRQ before releasing shared memory

[Adam Young]

Make sure a final interrupt request does not
asccess the shared buffer after it has been release.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index b9a01bfdc95d..2ce2afd255f0 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -537,6 +537,7 @@ static void pcc_shutdown(struct mbox_chan *chan)
 
 	if (pchan->plat_irq > 0)
 		devm_free_irq(chan->mbox->dev, pchan->plat_irq, chan);
+	synchronize_irq(pchan->plat_irq);
 
 	pcc_mbox_chan = &pchan->chan;
 	if (pcc_mbox_chan->shmem) {
-- 
2.43.0

[PATCH v45 7/7] wrap pchan->chan_in_use in READ/WRITE_ONCE

[Adam Young]

read/write happening from both userspace and Hard IRQ context
chan_in_use is used a flag to sychronize access.  Volitile
semantics ensure we don't have a reordering that accidentally
provide dual access.

Signed-off-by: Adam Young <admiyo@os.amperecomputing.com>
---
 drivers/mailbox/pcc.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 2ce2afd255f0..dbfe8dd92ddd 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -325,7 +325,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 		return IRQ_NONE;
 
 	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))
@@ -339,7 +339,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;
+	WRITE_ONCE(pchan->chan_in_use, false);
 	if (!rc)
 		mbox_chan_received_data(chan, NULL);
 	mbox_chan_txdone(chan, rc);
@@ -471,7 +471,7 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
 
 	ret = pcc_chan_reg_read_modify_write(&pchan->db);
 	if (!ret && pchan->plat_irq > 0)
-		pchan->chan_in_use = true;
+		WRITE_ONCE(pchan->chan_in_use, true);
 
 	return ret;
 }
-- 
2.43.0

Re: [PATCH v45 7/7] wrap pchan->chan_in_use in READ/WRITE_ONCE

[Sudeep Holla]

On Tue, Jul 21, 2026 at 01:52:56PM -0400, Adam Young wrote:
> read/write happening from both userspace and Hard IRQ context
> chan_in_use is used a flag to sychronize access.  Volitile
> semantics ensure we don't have a reordering that accidentally
> provide dual access.
>

With much better description [1]

-- 
Regards,
Sudeep

[1] https://lore.kernel.org/all/20260717075649.467172-4-sudeep.holla@kernel.org

Re: [PATCH v45 7/7] wrap pchan->chan_in_use in READ/WRITE_ONCE

[Adam Young]

On 7/22/26 04:50, Sudeep Holla wrote:
> On Tue, Jul 21, 2026 at 01:52:56PM -0400, Adam Young wrote:
>> read/write happening from both userspace and Hard IRQ context
>> chan_in_use is used a flag to sychronize access.  Volitile
>> semantics ensure we don't have a reordering that accidentally
>> provide dual access.
>>
> With much better description [1]
>
Excellent.  I will add a tested-by.