diff mbox series

[v7,1/2] mctp pcc: Check before sending MCTP PCC response ACK

Message ID 20241114024928.60004-2-admiyo@os.amperecomputing.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series MCTP Over PCC Transport | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Guessed tree name to be net-next, async
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit fail Errors and warnings before: 4 this patch: 5
netdev/build_tools success Errors and warnings before: 0 (+0) this patch: 0 (+0)
netdev/cc_maintainers warning 3 maintainers not CCed: rafael.j.wysocki@intel.com acpica-devel@lists.linux.dev linux-acpi@vger.kernel.org
netdev/build_clang fail Errors and warnings before: 4 this patch: 5
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn fail Errors and warnings before: 13 this patch: 16
netdev/checkpatch warning CHECK: extern prototypes should be avoided in .h files
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/source_inline success Was 0 now: 0

Commit Message

admiyo@os.amperecomputing.com Nov. 14, 2024, 2:49 a.m. UTC 
From: Adam Young <admiyo@os.amperecomputing.com>

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.

In order to read the flag, this patch maps the shared
buffer to virtual memory.

If the flag is not set, still set command completion
bit after processing message.

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

Comments

kernel test robot Nov. 17, 2024, 1:06 a.m. UTC | #1 
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.12-rc7 next-20241115]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/admiyo-os-amperecomputing-com/mctp-pcc-Check-before-sending-MCTP-PCC-response-ACK/20241114-105151
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241114024928.60004-2-admiyo%40os.amperecomputing.com
patch subject: [PATCH v7 1/2] mctp pcc: Check before sending MCTP PCC response ACK
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20241117/202411170849.DynEPZlM-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411170849.DynEPZlM-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/202411170849.DynEPZlM-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/mailbox/pcc.c:115: warning: Excess struct member 'shmem_base_addr' description in 'pcc_chan_info'


vim +115 drivers/mailbox/pcc.c

800cda7b63f22b Sudeep Holla 2021-09-17   82  
80b2bdde002c52 Sudeep Holla 2021-09-17   83  /**
80b2bdde002c52 Sudeep Holla 2021-09-17   84   * struct pcc_chan_info - PCC channel specific information
80b2bdde002c52 Sudeep Holla 2021-09-17   85   *
0f2591e21b2e85 Sudeep Holla 2021-09-17   86   * @chan: PCC channel information with Shared Memory Region info
bf18123e78f4d1 Sudeep Holla 2021-09-17   87   * @db: PCC register bundle for the doorbell register
bf18123e78f4d1 Sudeep Holla 2021-09-17   88   * @plat_irq_ack: PCC register bundle for the platform interrupt acknowledge
bf18123e78f4d1 Sudeep Holla 2021-09-17   89   *	register
c45ded7e11352d Sudeep Holla 2021-09-17   90   * @cmd_complete: PCC register bundle for the command complete check register
c45ded7e11352d Sudeep Holla 2021-09-17   91   * @cmd_update: PCC register bundle for the command complete update register
c45ded7e11352d Sudeep Holla 2021-09-17   92   * @error: PCC register bundle for the error status register
7cd506129724fe Adam Young   2024-11-13   93   * @shmem_base_addr: the virtual memory address of the shared buffer
f92ae90e52bb09 Sudeep Holla 2021-09-17   94   * @plat_irq: platform interrupt
60c40b06fa6869 Huisong Li   2023-08-01   95   * @type: PCC subspace type
3db174e478cb0b Huisong Li   2023-08-01   96   * @plat_irq_flags: platform interrupt flags
3db174e478cb0b Huisong Li   2023-08-01   97   * @chan_in_use: this flag is used just to check if the interrupt needs
3db174e478cb0b Huisong Li   2023-08-01   98   *		handling when it is shared. Since only one transfer can occur
3db174e478cb0b Huisong Li   2023-08-01   99   *		at a time and mailbox takes care of locking, this flag can be
3db174e478cb0b Huisong Li   2023-08-01  100   *		accessed without a lock. Note: the type only support the
3db174e478cb0b Huisong Li   2023-08-01  101   *		communication from OSPM to Platform, like type3, use it, and
3db174e478cb0b Huisong Li   2023-08-01  102   *		other types completely ignore it.
80b2bdde002c52 Sudeep Holla 2021-09-17  103   */
80b2bdde002c52 Sudeep Holla 2021-09-17  104  struct pcc_chan_info {
0f2591e21b2e85 Sudeep Holla 2021-09-17  105  	struct pcc_mbox_chan chan;
bf18123e78f4d1 Sudeep Holla 2021-09-17  106  	struct pcc_chan_reg db;
bf18123e78f4d1 Sudeep Holla 2021-09-17  107  	struct pcc_chan_reg plat_irq_ack;
c45ded7e11352d Sudeep Holla 2021-09-17  108  	struct pcc_chan_reg cmd_complete;
c45ded7e11352d Sudeep Holla 2021-09-17  109  	struct pcc_chan_reg cmd_update;
c45ded7e11352d Sudeep Holla 2021-09-17  110  	struct pcc_chan_reg error;
f92ae90e52bb09 Sudeep Holla 2021-09-17  111  	int plat_irq;
60c40b06fa6869 Huisong Li   2023-08-01  112  	u8 type;
3db174e478cb0b Huisong Li   2023-08-01  113  	unsigned int plat_irq_flags;
3db174e478cb0b Huisong Li   2023-08-01  114  	bool chan_in_use;
80b2bdde002c52 Sudeep Holla 2021-09-17 @115  };
80b2bdde002c52 Sudeep Holla 2021-09-17  116
kernel test robot Nov. 17, 2024, 1:28 a.m. UTC | #2 
Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on rafael-pm/linux-next]
[also build test ERROR on rafael-pm/bleeding-edge linus/master v6.12-rc7 next-20241115]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/admiyo-os-amperecomputing-com/mctp-pcc-Check-before-sending-MCTP-PCC-response-ACK/20241114-105151
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241114024928.60004-2-admiyo%40os.amperecomputing.com
patch subject: [PATCH v7 1/2] mctp pcc: Check before sending MCTP PCC response ACK
config: x86_64-randconfig-161-20241117 (https://download.01.org/0day-ci/archive/20241117/202411170906.mCmSa3oY-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411170906.mCmSa3oY-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/202411170906.mCmSa3oY-lkp@intel.com/

All error/warnings (new ones prefixed by >>):

   In file included from drivers/cpufreq/acpi-cpufreq.c:31:
   In file included from include/acpi/cppc_acpi.h:17:
>> include/acpi/pcc.h:51:10: error: expected ';' after return statement
      51 |         return 0
         |                 ^
         |                 ;
>> include/acpi/pcc.h:49:5: warning: no previous prototype for function 'pcc_mbox_ioremap' [-Wmissing-prototypes]
      49 | int pcc_mbox_ioremap(struct mbox_chan *chan)
         |     ^
   include/acpi/pcc.h:49:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      49 | int pcc_mbox_ioremap(struct mbox_chan *chan)
         | ^
         | static 
   1 warning and 1 error generated.
--
   In file included from drivers/cpufreq/intel_pstate.c:30:
   In file included from include/trace/events/power.h:12:
   In file included from include/linux/trace_events.h:6:
   In file included from include/linux/ring_buffer.h:5:
   In file included from include/linux/mm.h:2213:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
     518 |         return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
         |                               ~~~~~~~~~~~ ^ ~~~
   In file included from drivers/cpufreq/intel_pstate.c:48:
   In file included from include/acpi/cppc_acpi.h:17:
>> include/acpi/pcc.h:51:10: error: expected ';' after return statement
      51 |         return 0
         |                 ^
         |                 ;
>> include/acpi/pcc.h:49:5: warning: no previous prototype for function 'pcc_mbox_ioremap' [-Wmissing-prototypes]
      49 | int pcc_mbox_ioremap(struct mbox_chan *chan)
         |     ^
   include/acpi/pcc.h:49:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
      49 | int pcc_mbox_ioremap(struct mbox_chan *chan)
         | ^
         | static 
   4 warnings and 1 error generated.


vim +51 include/acpi/pcc.h

    36	
    37	#ifdef CONFIG_PCC
    38	extern struct pcc_mbox_chan *
    39	pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
    40	extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
    41	extern int pcc_mbox_ioremap(struct mbox_chan *chan);
    42	#else
    43	static inline struct pcc_mbox_chan *
    44	pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
    45	{
    46		return ERR_PTR(-ENODEV);
    47	}
    48	static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
  > 49	int pcc_mbox_ioremap(struct mbox_chan *chan)
    50	{
  > 51		return 0
    52	};
    53	#endif
    54
kernel test robot Nov. 17, 2024, 3:39 p.m. UTC | #3 
Hi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge linus/master v6.12-rc7 next-20241115]
[cannot apply to horms-ipvs/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/admiyo-os-amperecomputing-com/mctp-pcc-Check-before-sending-MCTP-PCC-response-ACK/20241114-105151
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241114024928.60004-2-admiyo%40os.amperecomputing.com
patch subject: [PATCH v7 1/2] mctp pcc: Check before sending MCTP PCC response ACK
config: x86_64-randconfig-123-20241117 (https://download.01.org/0day-ci/archive/20241117/202411172322.HO4ycC00-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241117/202411172322.HO4ycC00-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/202411172322.HO4ycC00-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/mailbox/pcc.c:283:34: sparse: sparse: Using plain integer as NULL pointer
>> drivers/mailbox/pcc.c:296:27: sparse: sparse: incorrect type in argument 1 (different base types) @@     expected restricted __le32 const [usertype] *p @@     got unsigned int * @@
   drivers/mailbox/pcc.c:296:27: sparse:     expected restricted __le32 const [usertype] *p
   drivers/mailbox/pcc.c:296:27: sparse:     got unsigned int *
   drivers/mailbox/pcc.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/slab.h, ...):
   include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
   include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false

vim +283 drivers/mailbox/pcc.c

   272	
   273	static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
   274	{
   275		struct acpi_pcct_ext_pcc_shared_memory pcc_hdr;
   276	
   277		if (pchan->type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
   278			return;
   279		/* If the memory region has not been mapped, we cannot
   280		 * determine if we need to send the message, but we still
   281		 * need to set the cmd_update flag before returning.
   282		 */
 > 283		if (pchan->chan.shmem == 0) {
   284			pcc_chan_reg_read_modify_write(&pchan->cmd_update);
   285			return;
   286		}
   287		memcpy_fromio(&pcc_hdr, pchan->chan.shmem,
   288			      sizeof(struct acpi_pcct_ext_pcc_shared_memory));
   289		/*
   290		 * The PCC slave subspace channel needs to set the command complete bit
   291		 * after processing message. If the PCC_ACK_FLAG is set, it should also
   292		 * ring the doorbell.
   293		 *
   294		 * The PCC master subspace channel clears chan_in_use to free channel.
   295		 */
 > 296		if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
   297			pcc_send_data(chan, NULL);
   298		else
   299			pcc_chan_reg_read_modify_write(&pchan->cmd_update);
   300	}
   301
diff mbox series

Patch

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 94885e411085..3a5779925eb2 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -90,6 +90,7 @@  struct pcc_chan_reg {
  * @cmd_complete: PCC register bundle for the command complete check register
  * @cmd_update: PCC register bundle for the command complete update register
  * @error: PCC register bundle for the error status register
+ * @shmem_base_addr: the virtual memory address of the shared buffer
  * @plat_irq: platform interrupt
  * @type: PCC subspace type
  * @plat_irq_flags: platform interrupt flags
@@ -269,6 +270,35 @@  static bool pcc_mbox_cmd_complete_check(struct pcc_chan_info *pchan)
 	return !!val;
 }
 
+static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
+{
+	struct acpi_pcct_ext_pcc_shared_memory pcc_hdr;
+
+	if (pchan->type != ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
+		return;
+	/* If the memory region has not been mapped, we cannot
+	 * determine if we need to send the message, but we still
+	 * need to set the cmd_update flag before returning.
+	 */
+	if (pchan->chan.shmem == 0) {
+		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
+		return;
+	}
+	memcpy_fromio(&pcc_hdr, pchan->chan.shmem,
+		      sizeof(struct acpi_pcct_ext_pcc_shared_memory));
+	/*
+	 * The PCC slave subspace channel needs to set the command complete bit
+	 * after processing message. If the PCC_ACK_FLAG is set, it should also
+	 * ring the doorbell.
+	 *
+	 * The PCC master subspace channel clears chan_in_use to free channel.
+	 */
+	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
+		pcc_send_data(chan, NULL);
+	else
+		pcc_chan_reg_read_modify_write(&pchan->cmd_update);
+}
+
 /**
  * pcc_mbox_irq - PCC mailbox interrupt handler
  * @irq:	interrupt number
@@ -306,14 +336,7 @@  static irqreturn_t pcc_mbox_irq(int irq, void *p)
 
 	mbox_chan_received_data(chan, NULL);
 
-	/*
-	 * The PCC slave subspace channel needs to set the command complete bit
-	 * and ring doorbell after processing message.
-	 *
-	 * The PCC master subspace channel clears chan_in_use to free channel.
-	 */
-	if (pchan->type == ACPI_PCCT_TYPE_EXT_PCC_SLAVE_SUBSPACE)
-		pcc_send_data(chan, NULL);
+	check_and_ack(pchan, chan);
 	pchan->chan_in_use = false;
 
 	return IRQ_HANDLED;
@@ -365,14 +388,37 @@  EXPORT_SYMBOL_GPL(pcc_mbox_request_channel);
 void pcc_mbox_free_channel(struct pcc_mbox_chan *pchan)
 {
 	struct mbox_chan *chan = pchan->mchan;
+	struct pcc_chan_info *pchan_info;
+	struct pcc_mbox_chan *pcc_mbox_chan;
 
 	if (!chan || !chan->cl)
 		return;
+	pchan_info = chan->con_priv;
+	pcc_mbox_chan = &pchan_info->chan;
+	if (pcc_mbox_chan->shmem) {
+		iounmap(pcc_mbox_chan->shmem);
+		pcc_mbox_chan->shmem = NULL;
+	}
 
 	mbox_free_channel(chan);
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 
+int pcc_mbox_ioremap(struct mbox_chan *chan)
+{
+	struct pcc_chan_info *pchan_info;
+	struct pcc_mbox_chan *pcc_mbox_chan;
+
+	if (!chan || !chan->cl)
+		return -1;
+	pchan_info = chan->con_priv;
+	pcc_mbox_chan = &pchan_info->chan;
+	pcc_mbox_chan->shmem = ioremap(pcc_mbox_chan->shmem_base_addr,
+				       pcc_mbox_chan->shmem_size);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(pcc_mbox_ioremap);
+
 /**
  * pcc_send_data - Called from Mailbox Controller code. Used
  *		here only to ring the channel doorbell. The PCC client
diff --git a/include/acpi/pcc.h b/include/acpi/pcc.h
index 9b373d172a77..f61c481eb478 100644
--- a/include/acpi/pcc.h
+++ b/include/acpi/pcc.h
@@ -12,6 +12,7 @@ 
 struct pcc_mbox_chan {
 	struct mbox_chan *mchan;
 	u64 shmem_base_addr;
+	void __iomem *shmem;
 	u64 shmem_size;
 	u32 latency;
 	u32 max_access_rate;
@@ -31,11 +32,13 @@  struct pcc_mbox_chan {
 #define PCC_CMD_COMPLETION_NOTIFY	BIT(0)
 
 #define MAX_PCC_SUBSPACES	256
+#define PCC_ACK_FLAG_MASK	0x1
 
 #ifdef CONFIG_PCC
 extern struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id);
 extern void pcc_mbox_free_channel(struct pcc_mbox_chan *chan);
+extern int pcc_mbox_ioremap(struct mbox_chan *chan);
 #else
 static inline struct pcc_mbox_chan *
 pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
@@ -43,6 +46,10 @@  pcc_mbox_request_channel(struct mbox_client *cl, int subspace_id)
 	return ERR_PTR(-ENODEV);
 }
 static inline void pcc_mbox_free_channel(struct pcc_mbox_chan *chan) { }
+int pcc_mbox_ioremap(struct mbox_chan *chan)
+{
+	return 0
+};
 #endif
 
 #endif /* _PCC_H */