Re: [PATCH v2 0/3] MCTP over PCC

Adam Young
2024-06-20T03:10:24Z

Apologies.  I just saw that there was more feedback on the previous
patchset.  I am addressing that now.

On 6/19/24 16:05, admiyo@os.amperecomputing.com wrote:
> From: Adam Young <admiyo@os.amperecomputing.com>
>
> This series adds support for the Management Control Transport Protocol (MCTP)
> over the Platform Communication Channel (PCC) mechanism.
>
> 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.
>
> The second patch in the series is the required change from ACPICA
> code that will be imported into the Linux kernel when synchronized
> with the ACPICA repository. It ahs already merged there and will
> be merged in as is. It is included here so that the patch series
> can run and be tested prior to that merge.
>
> 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
>
> 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.
>
> Code Review Change not made
>
> - Did not change the module init unload function to use the
> ACPI equivalent as they do not remove all devices prior
> to unload, leading to dangling references and seg faults.
>
> Adam Young (3):
> mctp pcc: Check before sending MCTP PCC response ACK
> mctp pcc: Allow PCC Data Type in MCTP resource.
> mctp pcc: Implement MCTP over PCC Transport
>
> drivers/acpi/acpica/rsaddr.c | 2 +-
> drivers/mailbox/pcc.c | 5 +-
> drivers/net/mctp/Kconfig | 13 ++
> drivers/net/mctp/Makefile | 1 +
> drivers/net/mctp/mctp-pcc.c | 373 +++++++++++++++++++++++++++++++++++
> include/acpi/pcc.h | 1 +
> 6 files changed, 393 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/mctp/mctp-pcc.c
>