Message-ID: <496af428-841e-4296-9b53-0dd89d8be655@amperemail.onmicrosoft.com>
Date: Wed, 27 Aug 2025 00:57:44 -0400
From: Adam Young <admiyo@...eremail.onmicrosoft.com>
To: admiyo@...amperecomputing.com
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
 Jeremy Kerr <jk@...econstruct.com.au>,
 Matt Johnston <matt@...econstruct.com.au>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Sudeep Holla <sudeep.holla@....com>,
 Jonathan Cameron <Jonathan.Cameron@...wei.com>,
 Huisong Li <lihuisong@...wei.com>
Subject: Re: [PATCH net-next v25 0/1] MCTP Over PCC Transport

Apologies for misnumbering this. It is V26 not V25.  I can resend if 
that is appropriate.


On 8/27/25 00:48, admiyo@...amperecomputing.com wrote:
> From: Adam Young <admiyo@...amperecomputing.com>
>
> This series adds support for the Management Control Transport Protocol (MCTP)
> over the Platform Communication Channel (PCC) mechanism.
>
> DMTF DSP:0292
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0292_1.0.0WIP50.pdf
>
> MCTP defines a communication model intended to
> facilitate communication between Management controllers
> and other management controllers, and between Management
> controllers and management devices
>
> PCC is a mechanism for communication between components within
> the  Platform.  It is a composed of shared memory regions,
> interrupt registers, and status registers.
>
> The MCTP over PCC driver makes use of two PCC channels. For
> sending messages, it uses a Type 3 channel, and for receiving
> messages it uses the paired Type 4 channel.  The device
> and corresponding channels are specified via ACPI.
>
> The first patch in the series implements a mechanism to allow the driver
> to indicate whether an ACK should be sent back to the caller
> after processing the interrupt.  This is an optional feature in
> the PCC code, but has been made explicitly required in another driver.
> The implementation here maintains the backwards compatibility of that
> driver.
>
> MCTP is a general purpose  protocol so  it would  be impossible to enumerate
> all the use cases, but some of the ones that are most topical are attestation
> and RAS support.  There are a handful of protocols built on top of MCTP, to
> include PLDM and SPDM, both specified by the DMTF.
>
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0240_1.0.0.pdf
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pd
>
> SPDM entails various usages, including device identity collection, device
> authentication, measurement collection, and device secure session establishment.
>
> PLDM is more likely to be used  for hardware support: temperature, voltage, or
> fan sensor control.
>
> At least two companies have devices that can make use of the mechanism. One is
> Ampere Computing, my employer.
>
> The mechanism it uses is called Platform Communication Channels is part of the
> ACPI spec: https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/14_Platform_Communications_Channel/Platform_Comm_Channel.html
>
> Since it is a socket interface, the system administrator also has  the ability
> to ignore an MCTP link that they do not want to enable.  This link would be visible
> to the end user, but would not be usable.
>
> If MCTP support is disabled in the Kernel, this driver would also be disabled.
>
> 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. Thus, 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 hard 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
> thus 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.
>
> Previous implementations of the pcc version of the mailbox protocol assumed the
> driver was directly managing the shared memory region.  This lead to duplicated
> code and missed steps of the PCC protocol. The first patch in this series makes
> it possible for mailbox/pcc to manage the writing of the buffer prior to sending
> messages.  It also fixes the notification of message transmission completion.
>
> Previous Version:
> https://lore.kernel.org/lkml/20250819205159.347561-1-admiyo@os.amperecomputing.com/
>
> Changes in V26:
> -  Remove the addition net-device spinlock and use the spinlock already present in skb lists
> -  Use temporary variables to check for success finding the skb in the lists
> -  Remove comment that is no longer relevant
>
> Changes in V25:
> - Use spin lock to control access to queues of sk_buffs
> - removed unused constants
> - added ndo_open and ndo_stop functions.  These two functions do
>    channel creation and cleanup, to remove packets from the mailbox.
>    They do queue cleanup as well.
> - No longer cleans up the channel from the device.
>
> Changes in V24:
> - Removed endianess for PCC header values
> - Kept Column width to under 80 chars
> - Typo in commit message
> - Prereqisite patch for PCC buffer management was merged late in 6.17.
>    See "mailbox/pcc: support mailbox management of the shared buffer"
>
> Changes in V23:
> - Trigger for direct management of shared buffer based on flag in pcc channel
> - Only initialize rx_alloc for inbox, not outbox.
> - Read value for requested IRQ flag out of channel's current_req
> - unqueue an sk_buff that failed to send
> - Move error handling for skb resize error inline instead of goto
>
> Changes in V22:
> - Direct management of the shared buffer in the mailbox layer.
> - Proper checking of command complete flag prior to writing to the buffer.
>
> Changes in V21:
> - Use existing constants PCC_SIGNATURE and PCC_CMD_COMPLETION_NOTIFY
> - Check return code on call to send_data and drop packet if failed
> - use sizeof(*mctp_pcc_header) etc,  instead of structs for resizing buffers
> - simplify check for ares->type != PCC_DWORD_TYPE
> - simply return result devm_add_action_or_reset
> - reduce initializer for  mctp_pcc_lookup_context context = {};
> - move initialization of mbox dev into mctp_pcc_initialize_mailbox
> - minor spacing changes
>
> Changes in V20:
> - corrected typo in RFC version
> - removed spurious space
> - tx spin lock only controls access to shared memory buffer
> - tx spin lock not eheld on error condition
> - tx returns OK if skb can't be expanded
>
> Changes in V19:
> - Rebased on changes to PCC mailbox handling
> - checks for cloned SKB prior to transmission
> - converted doulbe slash comments to C comments
>
> Changes in V18:
> - Added Acked-By
> - Fix minor spacing issue
>
> Changes in V17:
> - No new changes. Rebased on net-next post 6.13 release.
>
> Changes in V16:
> - do not duplicate cleanup after devm_add_action_or_reset calls
>
> Changes in V15:
> - corrected indentation formatting error
> - Corrected TABS issue in MAINTAINER entry
>
> Changes in V14:
> - Do not attempt to unregister a netdev that is never registered
> - Added MAINTAINER entry
>
> Changes in V13:
> - Explicitly Convert PCC header from little endian to machine native
>
> Changes in V12:
> - Explicitly use little endian conversion for PCC header signature
> - Builds clean with make C=1
>
> Changes in V11:
> - Explicitly use little endian types for PCC header
>
> Changes in V11:
> - Switch Big Endian data types to machine local for PCC header
> - use mctp specific function for registering netdev
>
> Changes in V10:
> - sync with net-next branch
> - use dstats helper functions
> - remove duplicate drop stat
> - remove more double spaces
>
> Changes in V9:
> - Prerequisite patch for PCC mailbox has been merged
> - Stats collection now use helper functions
> - many double spaces reduced to single
>
> Changes in V8:
> - change 0 to NULL for pointer check of shmem
> - add semi for static version of pcc_mbox_ioremap
> - convert pcc_mbox_ioremap function to static inline when client code is not being built
> - remove shmem comment from struct pcc_chan_info descriptor
> - copy rx_dropped in mctp_pcc_net_stats
> - removed trailing newline on error message
> - removed double space in dev_dbg string
> - use big endian for header members
> - Fix use full spec ID in description
> - Fix typo in file description
> - Form the complete outbound message in the sk_buff
>
> Changes in V7:
> - Removed the Hardware address as specification is not published.
> - Map the shared buffer in the mailbox and share the mapped region with the driver
> - Use the sk_buff memory to prepare the message before copying to shared region
>
> Changes in V6:
> - Removed patch for ACPICA code that has merged
> - Includes the hardware address in the network device
> - Converted all device resources to devm resources
> - Removed mctp_pcc_driver_remove function
> - uses acpi_driver_module for initialization
> - created helper structure for in and out mailboxes
> - Consolidated code for initializing mailboxes in the add_device function
> - Added specification references
> - Removed duplicate constant PCC_ACK_FLAG_MASK
> - Use the MCTP_SIGNATURE_LENGTH define
> - made naming of header structs consistent
> - use sizeof local variables for offset calculations
> - prefix structure name to avoid potential clash
> - removed unnecessary null initialization from acpi_device_id
>
> Changes in V5
> - Removed Owner field from ACPI module declaration
> - removed unused next field from struct mctp_pcc_ndev
> - Corrected logic reading  RX ACK flag.
> - Added comment for struct pcc_chan_info field shmem_base_addr
> - check against current mtu instead of max mtu for packet length\
> - removed unnecessary lookups of pnd->mdev.dev
>
> Changes in V4
> - Read flags out of shared buffer to trigger ACK for Type 4 RX
> - Remove list of netdevs and cleanup from devices only
> - tag PCCT protocol headers as little endian
> - Remove unused constants
>
> Changes in V3
> - removed unused header
> - removed spurious space
> - removed spurious semis after functiomns
> - removed null assignment for init
> - remove redundant set of device on skb
> - tabify constant declarations
> - added  rtnl_link_stats64 function
> - set MTU to minimum to start
> - clean up logic on driver removal
> - remove cast on void * assignment
> - call cleanup function directly
> - check received length before allocating skb
> - introduce symbolic constatn for ACK FLAG MASK
> - symbolic constant for PCC header flag.
> - Add namespace ID to PCC magic
> - replaced readls with copy from io of PCC header
> - replaced custom modules init and cleanup with ACPI version
>
> Changes in V2
>
> - All Variable Declarations are in reverse Xmass Tree Format
> - All Checkpatch Warnings Are Fixed
> - Removed Dead code
> - Added packet tx/rx stats
> - Removed network physical address.  This is still in
>    disucssion in the spec, and will be added once there
>    is consensus. The protocol can be used with out it.
>    This also lead to the removal of the Big Endian
>    conversions.
> - Avoided using non volatile pointers in copy to and from io space
> - Reorderd the patches to put the ACK check for the PCC Mailbox
>    as a pre-requisite.  The corresponding change for the MCTP
>    driver has been inlined in the main patch.
> - Replaced magic numbers with constants, fixed typos, and other
>    minor changes from code review.
>
> Adam Young (1):
>    mctp pcc: Implement MCTP over PCC Transport
>
>   MAINTAINERS                 |   5 +
>   drivers/net/mctp/Kconfig    |  13 ++
>   drivers/net/mctp/Makefile   |   1 +
>   drivers/net/mctp/mctp-pcc.c | 367 ++++++++++++++++++++++++++++++++++++
>   4 files changed, 386 insertions(+)
>   create mode 100644 drivers/net/mctp/mctp-pcc.c
>

Powered by blists - more mailing lists