From: Adam Young <[email protected]>
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 its corresponding channels are specified via ACPI.
Adam Young (3):
mctp pcc: Implement MCTP over PCC Transport
mctp pcc: Allow PCC Data Type in MCTP resource.
mctp pcc: RFC Check before sending MCTP PCC response ACK
drivers/acpi/acpica/rsaddr.c | 2 +-
drivers/mailbox/pcc.c | 5 +-
drivers/net/mctp/Kconfig | 12 ++
drivers/net/mctp/Makefile | 1 +
drivers/net/mctp/mctp-pcc.c | 372 +++++++++++++++++++++++++++++++++++
include/acpi/pcc.h | 1 +
6 files changed, 391 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/mctp/mctp-pcc.c
--
2.34.1
From: Adam Young <[email protected]>
Note that this patch sfor code that will be merged
in via ACPICA changes. The corresponding patch in ACPCA
has already merged.
Signed-off-by: Adam Young <[email protected]>
---
drivers/acpi/acpica/rsaddr.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/acpica/rsaddr.c b/drivers/acpi/acpica/rsaddr.c
index fff48001d7ef..6bd9704f17b0 100644
--- a/drivers/acpi/acpica/rsaddr.c
+++ b/drivers/acpi/acpica/rsaddr.c
@@ -282,7 +282,7 @@ acpi_rs_get_address_common(struct acpi_resource *resource,
/* Validate the Resource Type */
- if ((address.resource_type > 2) && (address.resource_type < 0xC0)) {
+ if ((address.resource_type > 2) && (address.resource_type < 0xC0) && (address.resource_type != 10)) {
return (FALSE);
}
--
2.34.1
From: Adam Young <[email protected]>
Type 4 PCC channels have an option to send back a response
to the platform when they are done processing the request.
The flag to indicate whether or not to respond is inside
the message body, and thus is not available to the pcc
mailbox. Since only one message can be processed at once per
channel, the value of this flag is checked during message processing
and passed back via the channels global structure.
Ideally, the mailbox callback function would return a value
indicating whether the message requires an ACK, but that
would be a change to the mailbox API. That would involve
some change to all of the mailbox based drivers.
Signed-off-by: Adam Young <[email protected]>
---
drivers/mailbox/pcc.c | 5 ++++-
drivers/net/mctp/mctp-pcc.c | 4 ++++
include/acpi/pcc.h | 1 +
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..774727b89693 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -280,6 +280,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
{
struct pcc_chan_info *pchan;
struct mbox_chan *chan = p;
+ struct pcc_mbox_chan *pmchan;
u64 val;
int ret;
@@ -304,6 +305,8 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
if (pcc_chan_reg_read_modify_write(&pchan->plat_irq_ack))
return IRQ_NONE;
+ pmchan = &pchan->chan;
+ pmchan->ack_rx = true; //TODO default to False
mbox_chan_received_data(chan, NULL);
/*
@@ -312,7 +315,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
*
* The PCC master subspace channel clears chan_in_use to free channel.
*/
- if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+ if ((pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE) && pmchan->ack_rx)
pcc_send_data(chan, NULL);
pchan->chan_in_use = false;
diff --git a/drivers/net/mctp/mctp-pcc.c b/drivers/net/mctp/mctp-pcc.c
index 7242eedd2759..3df45b86ea03 100644
--- a/drivers/net/mctp/mctp-pcc.c
+++ b/drivers/net/mctp/mctp-pcc.c
@@ -96,6 +96,7 @@ static void mctp_pcc_client_rx_callback(struct mbox_client *c, void *)
unsigned long buf_ptr_val;
struct mctp_pcc_ndev *mctp_pcc_dev = container_of(c, struct mctp_pcc_ndev, inbox_client);
void *skb_buf;
+ u32 flags;
mpp = (struct mctp_pcc_packet *)mctp_pcc_dev->pcc_comm_inbox_addr;
buf_ptr_val = (unsigned long)mpp;
@@ -115,6 +116,9 @@ static void mctp_pcc_client_rx_callback(struct mbox_client *c, void *)
cb->halen = 0;
skb->dev = mctp_pcc_dev->mdev.dev;
netif_rx(skb);
+
+ flags = readl(&mpp->pcc_header.flags);
+ mctp_pcc_dev->in_chan->ack_rx = (flags & 1) > 0;
}
static netdev_tx_t mctp_pcc_tx(struct sk_buff *skb, struct net_device *ndev)
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 9b373d172a77..297913378c2b 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -16,6 +16,7 @@ struct pcc_mbox_chan {
u32 latency;
u32 max_access_rate;
u16 min_turnaround_time;
+ bool ack_rx;
};
/* Generic Communications Channel Shared Memory Region */
--
2.34.1
On Mon, May 13, 2024 at 01:35:45PM -0400, [email protected] wrote:
> From: Adam Young <[email protected]>
>
> Note that this patch sfor code that will be merged
> in via ACPICA changes. The corresponding patch in ACPCA
> has already merged.
>
> Signed-off-by: Adam Young <[email protected]>
> ---
> drivers/acpi/acpica/rsaddr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpica/rsaddr.c b/drivers/acpi/acpica/rsaddr.c
> index fff48001d7ef..6bd9704f17b0 100644
> --- a/drivers/acpi/acpica/rsaddr.c
> +++ b/drivers/acpi/acpica/rsaddr.c
> @@ -282,7 +282,7 @@ acpi_rs_get_address_common(struct acpi_resource *resource,
>
> /* Validate the Resource Type */
>
> - if ((address.resource_type > 2) && (address.resource_type < 0xC0)) {
> + if ((address.resource_type > 2) && (address.resource_type < 0xC0) && (address.resource_type != 10)) {
> return (FALSE);
More magic numbers. Please add some #defines.
Andrew
On Mon, May 13, 2024 at 01:35:46PM -0400, [email protected] wrote:
> From: Adam Young <[email protected]>
>
> Type 4 PCC channels have an option to send back a response
> to the platform when they are done processing the request.
> The flag to indicate whether or not to respond is inside
> the message body, and thus is not available to the pcc
> mailbox. Since only one message can be processed at once per
> channel, the value of this flag is checked during message processing
> and passed back via the channels global structure.
>
> Ideally, the mailbox callback function would return a value
> indicating whether the message requires an ACK, but that
> would be a change to the mailbox API. That would involve
> some change to all of the mailbox based drivers.
How many mailbox drivers are there?
Generally, taking the path of least resistance will cost more in the
long run. It is better to do it properly from the start.
Andrew
From: Adam Young <[email protected]>
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.
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 | 361 +++++++++++++++++++++++++++++++++++
include/acpi/pcc.h | 1 +
6 files changed, 381 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/mctp/mctp-pcc.c
--
2.34.1
On 5/29/2024 3:18 AM, [email protected] wrote:
> From: Adam Young <[email protected]>
>
> 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.
>
> 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 | 361 +++++++++++++++++++++++++++++++++++
> include/acpi/pcc.h | 1 +
> 6 files changed, 381 insertions(+), 2 deletions(-)
> create mode 100644 drivers/net/mctp/mctp-pcc.c
>
Tested OK on Arm FVP.
Tested-by: John Chung <[email protected]>