[PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer

admiyo@os.amperecomputing.com posted 2 patches 2 months, 4 weeks ago
There is a newer version of this series
[PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
Posted by admiyo@os.amperecomputing.com 2 months, 4 weeks ago
From: Adam Young <admiyo@os.amperecomputing.com>

Define a new, optional, callback that allows the driver to
specify how the return data buffer is allocated.  If that callback
is set,  mailbox/pcc.c is now responsible for reading from and
writing to the PCC shared buffer.

This also allows for proper checks of the Commnand complete flag
between the PCC sender and receiver.

For Type 4 channels, initialize the command complete flag prior
to accepting messages.

Since the mailbox does not know what memory allocation scheme
to use for response messages, the client now has an optional
callback that allows it to allocate the buffer for a response
message.

When an outbound message is written to the buffer, the mailbox
checks for the flag indicating the client wants an tx complete
notification via IRQ.  Upon reciept of the interrupt It will
pair it with the outgoing message. The expected use is to
free the kernel memory buffer for the previous outgoing message.

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

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index f6714c233f5a..2932c26aaf62 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -306,6 +306,22 @@ static void pcc_chan_acknowledge(struct pcc_chan_info *pchan)
 		pcc_chan_reg_read_modify_write(&pchan->db);
 }
 
+static void *write_response(struct pcc_chan_info *pchan)
+{
+	struct pcc_header pcc_header;
+	void *buffer;
+	int data_len;
+
+	memcpy_fromio(&pcc_header, pchan->chan.shmem,
+		      sizeof(pcc_header));
+	data_len = pcc_header.length - sizeof(u32) + sizeof(struct pcc_header);
+
+	buffer = pchan->chan.rx_alloc(pchan->chan.mchan->cl, data_len);
+	if (buffer != NULL)
+		memcpy_fromio(buffer, pchan->chan.shmem, data_len);
+	return buffer;
+}
+
 /**
  * pcc_mbox_irq - PCC mailbox interrupt handler
  * @irq:	interrupt number
@@ -317,6 +333,7 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p)
 {
 	struct pcc_chan_info *pchan;
 	struct mbox_chan *chan = p;
+	void *handle = NULL;
 
 	pchan = chan->con_priv;
 
@@ -340,7 +357,14 @@ 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);
+
+	if (pchan->chan.rx_alloc)
+		handle = write_response(pchan);
+
+	if (pchan->chan.irq_ack)
+		mbox_chan_txdone(chan, 0);
+
+	mbox_chan_received_data(chan, handle);
 
 	pcc_chan_acknowledge(pchan);
 
@@ -384,9 +408,22 @@ pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	pcc_mchan = &pchan->chan;
 	pcc_mchan->shmem = acpi_os_ioremap(pcc_mchan->shmem_base_addr,
 					   pcc_mchan->shmem_size);
-	if (pcc_mchan->shmem)
-		return pcc_mchan;
+	if (!pcc_mchan->shmem)
+		goto err;
+
+	/* This indicates that the channel is ready to accept messages.
+	 * This needs to happen after the channel has registered
+	 * its callback. There is no access point to do that in
+	 * the mailbox API. That implies that the mailbox client must
+	 * have set the allocate callback function prior to
+	 * sending any messages.
+	 */
+	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
+
+	return pcc_mchan;
 
+err:
 	mbox_free_channel(chan);
 	return ERR_PTR(-ENXIO);
 }
@@ -417,6 +454,27 @@ void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 
+static int pcc_write_to_buffer(struct mbox_chan *chan, void *data)
+{
+	struct pcc_chan_info *pchan = chan->con_priv;
+	struct pcc_mbox_chan *pcc_mbox_chan = &pchan->chan;
+	struct pcc_header *pcc_header = data;
+	/* The PCC header length includes the command field
+	 * but not the other values from the header.
+	 */
+	int len = pcc_header->length - sizeof(u32) + sizeof(struct pcc_header);
+	u64 val;
+
+	pcc_chan_reg_read(&pchan->cmd_complete, &val);
+	if (!val) {
+		pr_info("%s pchan->cmd_complete not set", __func__);
+		return -1;
+	}
+	memcpy_toio(pcc_mbox_chan->shmem,  data, len);
+	return 0;
+}
+
+
 /**
  * pcc_send_data - Called from Mailbox Controller code. Used
  *		here only to ring the channel doorbell. The PCC client
@@ -433,18 +491,44 @@ static int pcc_send_data(struct mbox_chan *chan, void *data)
 {
 	int ret;
 	struct pcc_chan_info *pchan = chan->con_priv;
+	struct acpi_pcct_ext_pcc_shared_memory __iomem *pcc_hdr;
+
+	if (pchan->chan.rx_alloc)
+		ret = pcc_write_to_buffer(chan, data);
+	if (ret)
+		return ret;
 
 	ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
 	if (ret)
 		return ret;
 
+	pcc_hdr = pchan->chan.shmem;
+	if (ioread32(&pcc_hdr->flags) & PCC_CMD_COMPLETION_NOTIFY)
+		pchan->chan.irq_ack = true;
+
 	ret = pcc_chan_reg_read_modify_write(&pchan->db);
+
 	if (!ret && pchan->plat_irq > 0)
 		pchan->chan_in_use = true;
 
 	return ret;
 }
 
+
+static bool pcc_last_tx_done(struct mbox_chan *chan)
+{
+	struct pcc_chan_info *pchan = chan->con_priv;
+	u64 val;
+
+	pcc_chan_reg_read(&pchan->cmd_complete, &val);
+	if (!val)
+		return false;
+	else
+		return true;
+}
+
+
+
 /**
  * pcc_startup - Called from Mailbox Controller code. Used here
  *		to request the interrupt.
@@ -490,6 +574,7 @@ static const struct mbox_chan_ops pcc_chan_ops = {
 	.send_data = pcc_send_data,
 	.startup = pcc_startup,
 	.shutdown = pcc_shutdown,
+	.last_tx_done = pcc_last_tx_done,
 };
 
 /**
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 840bfc95bae3..b5414572912a 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -17,6 +17,25 @@ struct pcc_mbox_chan {
 	u32 latency;
 	u32 max_access_rate;
 	u16 min_turnaround_time;
+
+	/* Set to true When a message is sent that has the flag
+	 * set that the client requests an Interrupt
+	 * indicating that the transmission is complete.
+	 */
+	bool irq_ack;
+	/* Optional callback that allows the driver
+	 * to allocate the memory used for receiving
+	 * messages.  The return value is the location
+	 * inside the buffer where the mailbox should write the data.
+	 */
+	void *(*rx_alloc)(struct mbox_client *cl,  int size);
+};
+
+struct pcc_header {
+	u32 signature;
+	u32 flags;
+	u32 length;
+	u32 command;
 };
 
 /* Generic Communications Channel Shared Memory Region */
-- 
2.43.0
Re: [PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
Posted by Adam Young 2 months, 3 weeks ago
Internal review discovered an error

On 7/10/25 15:12, admiyo@os.amperecomputing.com wrote:
> +	if (pchan->chan.rx_alloc)
> +		ret = pcc_write_to_buffer(chan, data);

This is the wrong check. The rx_alloc is expected to be used  for the 
type4, and the will be called from the type 3.

Going to add an explicit flag instead.


> +	pcc_hdr = pchan->chan.shmem;
> +	if (ioread32(&pcc_hdr->flags) & PCC_CMD_COMPLETION_NOTIFY)
> +		pchan->chan.irq_ack = true;

This flag can be removed and replaced  with a check of the value in the 
original buffer, which is held in chan->current_req.


Updated version of the patch series to follow.


Re: [PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
Posted by Dan Carpenter 2 months, 3 weeks ago
Hi,

kernel test robot noticed the following build warnings:

url:    https://github.com/intel-lab-lkp/linux/commits/admiyo-os-amperecomputing-com/mailbox-pcc-support-mailbox-management-of-the-shared-buffer/20250711-031525
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250710191209.737167-2-admiyo%40os.amperecomputing.com
patch subject: [PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
config: x86_64-randconfig-161-20250711 (https://download.01.org/0day-ci/archive/20250712/202507120609.Myazax08-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202507120609.Myazax08-lkp@intel.com/

smatch warnings:
drivers/mailbox/pcc.c:498 pcc_send_data() error: uninitialized symbol 'ret'.

vim +/ret +498 drivers/mailbox/pcc.c

86c22f8c9a3b71 Ashwin Chaugule    2014-11-12  490  static int pcc_send_data(struct mbox_chan *chan, void *data)
86c22f8c9a3b71 Ashwin Chaugule    2014-11-12  491  {
c45ded7e11352d Sudeep Holla       2021-09-17  492  	int ret;
bf18123e78f4d1 Sudeep Holla       2021-09-17  493  	struct pcc_chan_info *pchan = chan->con_priv;
e332edef98ddac Adam Young         2025-07-10  494  	struct acpi_pcct_ext_pcc_shared_memory __iomem *pcc_hdr;
e332edef98ddac Adam Young         2025-07-10  495  
e332edef98ddac Adam Young         2025-07-10  496  	if (pchan->chan.rx_alloc)
e332edef98ddac Adam Young         2025-07-10  497  		ret = pcc_write_to_buffer(chan, data);

Hi Adam!  :)

ret is uninitialized on the else path.

e332edef98ddac Adam Young         2025-07-10 @498  	if (ret)
e332edef98ddac Adam Young         2025-07-10  499  		return ret;
8b0f57889843af Prakash, Prashanth 2016-02-17  500  
c45ded7e11352d Sudeep Holla       2021-09-17  501  	ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
c45ded7e11352d Sudeep Holla       2021-09-17  502  	if (ret)
c45ded7e11352d Sudeep Holla       2021-09-17  503  		return ret;
c45ded7e11352d Sudeep Holla       2021-09-17  504  
e332edef98ddac Adam Young         2025-07-10  505  	pcc_hdr = pchan->chan.shmem;
e332edef98ddac Adam Young         2025-07-10  506  	if (ioread32(&pcc_hdr->flags) & PCC_CMD_COMPLETION_NOTIFY)
e332edef98ddac Adam Young         2025-07-10  507  		pchan->chan.irq_ack = true;
e332edef98ddac Adam Young         2025-07-10  508  
3db174e478cb0b Huisong Li         2023-08-01  509  	ret = pcc_chan_reg_read_modify_write(&pchan->db);
e332edef98ddac Adam Young         2025-07-10  510  
3db174e478cb0b Huisong Li         2023-08-01  511  	if (!ret && pchan->plat_irq > 0)
3db174e478cb0b Huisong Li         2023-08-01  512  		pchan->chan_in_use = true;
3db174e478cb0b Huisong Li         2023-08-01  513  
3db174e478cb0b Huisong Li         2023-08-01  514  	return ret;
86c22f8c9a3b71 Ashwin Chaugule    2014-11-12  515  }

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
Posted by kernel test robot 2 months, 4 weeks ago
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/admiyo-os-amperecomputing-com/mailbox-pcc-support-mailbox-management-of-the-shared-buffer/20250711-031525
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250710191209.737167-2-admiyo%40os.amperecomputing.com
patch subject: [PATCH net-next v22 1/2] mailbox/pcc: support mailbox management of the shared buffer
config: i386-randconfig-014-20250711 (https://download.01.org/0day-ci/archive/20250711/202507111440.1zQdhxpr-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250711/202507111440.1zQdhxpr-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507111440.1zQdhxpr-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mailbox/pcc.c:496:6: warning: variable 'ret' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
     496 |         if (pchan->chan.rx_alloc)
         |             ^~~~~~~~~~~~~~~~~~~~
   drivers/mailbox/pcc.c:498:6: note: uninitialized use occurs here
     498 |         if (ret)
         |             ^~~
   drivers/mailbox/pcc.c:496:2: note: remove the 'if' if its condition is always true
     496 |         if (pchan->chan.rx_alloc)
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~
     497 |                 ret = pcc_write_to_buffer(chan, data);
   drivers/mailbox/pcc.c:492:9: note: initialize the variable 'ret' to silence this warning
     492 |         int ret;
         |                ^
         |                 = 0
   1 warning generated.


vim +496 drivers/mailbox/pcc.c

   476	
   477	
   478	/**
   479	 * pcc_send_data - Called from Mailbox Controller code. Used
   480	 *		here only to ring the channel doorbell. The PCC client
   481	 *		specific read/write is done in the client driver in
   482	 *		order to maintain atomicity over PCC channel once
   483	 *		OS has control over it. See above for flow of operations.
   484	 * @chan: Pointer to Mailbox channel over which to send data.
   485	 * @data: Client specific data written over channel. Used here
   486	 *		only for debug after PCC transaction completes.
   487	 *
   488	 * Return: Err if something failed else 0 for success.
   489	 */
   490	static int pcc_send_data(struct mbox_chan *chan, void *data)
   491	{
   492		int ret;
   493		struct pcc_chan_info *pchan = chan->con_priv;
   494		struct acpi_pcct_ext_pcc_shared_memory __iomem *pcc_hdr;
   495	
 > 496		if (pchan->chan.rx_alloc)
   497			ret = pcc_write_to_buffer(chan, data);
   498		if (ret)
   499			return ret;
   500	
   501		ret = pcc_chan_reg_read_modify_write(&pchan->cmd_update);
   502		if (ret)
   503			return ret;
   504	
   505		pcc_hdr = pchan->chan.shmem;
   506		if (ioread32(&pcc_hdr->flags) & PCC_CMD_COMPLETION_NOTIFY)
   507			pchan->chan.irq_ack = true;
   508	
   509		ret = pcc_chan_reg_read_modify_write(&pchan->db);
   510	
   511		if (!ret && pchan->plat_irq > 0)
   512			pchan->chan_in_use = true;
   513	
   514		return ret;
   515	}
   516	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki