Hi Adam,

This is looking pretty concise now, nice work. A few things inline:

Minor, but if you end up re-rolling: -2026.

+ *
+ */
+
+/* Implementation of MCTP over PCC DMTF Specification DSP0256
+ * https://www.dmtf.org/sites/default/files/standards/documents/DSP0292_1.0.0WIP50.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>
+
+#define MCTP_SIGNATURE          "MCTP"
+#define MCTP_SIGNATURE_LENGTH   (sizeof(MCTP_SIGNATURE) - 1)
+#define MCTP_MIN_MTU            68
+#define PCC_DWORD_TYPE          0x0c
+
+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;
+       size = pcc_mbox_query_bytes_available(inbox->chan);
+       if (size == 0)
+               return;

Your new pcc_mbox_query_bytes_available() returns -1 on error, but you
don't handle that here.

+       skb = netdev_alloc_skb(mctp_pcc_ndev->ndev, size);
+       if (!skb) {
+               dev_dstats_rx_dropped(mctp_pcc_ndev->ndev);
+               return;
+       }
+       skb_put(skb, size);
+       skb->protocol = htons(ETH_P_MCTP);
+       pcc_mbox_read_from_buffer(inbox->chan, size, skb->data);
+       dev_dstats_rx_add(mctp_pcc_ndev->ndev, skb->len);
+       skb_reset_mac_header(skb);
+       skb_pull(skb, sizeof(pcc_header));

Do you want to check that the header describes a valid MCTP-over-PCC
packet?

+       skb_reset_network_header(skb);
+       cb = __mctp_cb(skb);
+       cb->halen = 0;
+       netif_rx(skb);
+}
+
+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;
+       int rc;
+
+       rc = skb_cow_head(skb, sizeof(*pcc_header));
+       if (rc) {
+               dev_dstats_tx_dropped(ndev);
+               kfree_skb(skb);
+               return NETDEV_TX_OK;
+       }
+
+       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;
+
+       rc = mbox_send_message(mpnd->outbox.chan->mchan, skb);
+       if (rc < 0) {
+               netif_stop_queue(ndev);
+               return NETDEV_TX_BUSY;

If you return NETDEV_TX_BUSY here, and the core does a retransmit,
you will end up prepending another header.

Just to confirm from earlier discussions: you have no way to determine
that the channel is unavailable in advance, right?

+       }
+
+       dev_dstats_tx_add(ndev, len);
+       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;
+       int len_sent;
+
+       mctp_pcc_ndev = container_of(cl, struct mctp_pcc_ndev, outbox.client);
+       outbox = &mctp_pcc_ndev->outbox;
+
+       if (!skb)
+               return;
+
+       len_sent = pcc_mbox_write_to_buffer(outbox->chan, skb->len, skb->data);
+       if (len_sent == 0)
+               netdev_info(mctp_pcc_ndev->ndev,  "packet dropped");

You probably don't want a netdev_info() here, perhaps netdev_dbg().

Or, could you increment the tx_dropped stat? I see you're doing all the
stats accounting in start_xmit - I'm not familiar with the PCC API, but
there seem to be failure paths past that point. Is it possible to
distinguish between successful tx and dropped tx in the tx_done
callback, and do the accounting there, perhaps?

Or is this a trivial-enough case (len > shmem_size) to not worry about
accounting?

Cheers,


Jeremy