lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <C5DDDB438F5F4EF9+20250813074622.GE944516@nic-Precision-5820-Tower>
Date: Wed, 13 Aug 2025 15:46:22 +0800
From: Yibo Dong <dong100@...se.com>
To: "Anwar, Md Danish" <a0501179@...com>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, horms@...nel.org,
	corbet@....net, gur.stavi@...wei.com, maddy@...ux.ibm.com,
	mpe@...erman.id.au, danishanwar@...com, lee@...ger.us,
	gongfan1@...wei.com, lorenzo@...nel.org, geert+renesas@...der.be,
	Parthiban.Veerasooran@...rochip.com, lukas.bulwahn@...hat.com,
	alexanderduyck@...com, richardcochran@...il.com,
	netdev@...r.kernel.org, linux-doc@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 3/5] net: rnpgbe: Add basic mbx ops support

On Tue, Aug 12, 2025 at 10:00:57PM +0530, Anwar, Md Danish wrote:
> On 8/12/2025 3:09 PM, Dong Yibo wrote:
> > Initialize basic mbx function.
> > 
> > Signed-off-by: Dong Yibo <dong100@...se.com>
> > ---
> >  drivers/net/ethernet/mucse/rnpgbe/Makefile    |   3 +-
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h    |  37 ++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_chip.c   |   5 +
> >  drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h |   2 +
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c    | 443 ++++++++++++++++++
> >  .../net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h    |  31 ++
> >  6 files changed, 520 insertions(+), 1 deletion(-)
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> >  create mode 100644 drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.h
> > 
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/Makefile b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > index 42c359f459d9..5fc878ada4b1 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/Makefile
> > @@ -6,4 +6,5 @@
> >  
> >  obj-$(CONFIG_MGBE) += rnpgbe.o
> >  rnpgbe-objs := rnpgbe_main.o\
> > -	       rnpgbe_chip.o
> > +	       rnpgbe_chip.o\
> > +	       rnpgbe_mbx.o
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > index 0dd3d3cb2a4d..05830bb73d3e 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe.h
> > @@ -5,6 +5,7 @@
> >  #define _RNPGBE_H
> >  
> >  #include <linux/types.h>
> > +#include <linux/mutex.h>
> >  
> >  extern const struct rnpgbe_info rnpgbe_n500_info;
> >  extern const struct rnpgbe_info rnpgbe_n210_info;
> > @@ -40,7 +41,43 @@ struct mucse_mac_info {
> >  	void *back;
> >  };
> >  
> > +struct mucse_hw;
> > +
> > +struct mucse_mbx_operations {
> > +	void (*init_params)(struct mucse_hw *hw);
> > +	int (*read)(struct mucse_hw *hw, u32 *msg,
> > +		    u16 size);
> > +	int (*write)(struct mucse_hw *hw, u32 *msg,
> > +		     u16 size);
> > +	int (*read_posted)(struct mucse_hw *hw, u32 *msg,
> > +			   u16 size);
> > +	int (*write_posted)(struct mucse_hw *hw, u32 *msg,
> > +			    u16 size);
> > +	int (*check_for_msg)(struct mucse_hw *hw);
> > +	int (*check_for_ack)(struct mucse_hw *hw);
> > +	void (*configure)(struct mucse_hw *hw, int num_vec,
> > +			  bool enable);
> > +};
> > +
> > +struct mucse_mbx_stats {
> > +	u32 msgs_tx;
> > +	u32 msgs_rx;
> > +	u32 acks;
> > +	u32 reqs;
> > +	u32 rsts;
> > +};
> > +
> >  struct mucse_mbx_info {
> > +	const struct mucse_mbx_operations *ops;
> > +	struct mucse_mbx_stats stats;
> > +	u32 timeout;
> > +	u32 usec_delay;
> > +	u16 size;
> > +	u16 fw_req;
> > +	u16 fw_ack;
> > +	/* lock for only one use mbx */
> > +	struct mutex lock;
> > +	bool irq_enabled;
> >  	/* fw <--> pf mbx */
> >  	u32 fw_pf_shm_base;
> >  	u32 pf2fw_mbox_ctrl;
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > index 20ec67c9391e..16d0a76114b5 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_chip.c
> > @@ -1,8 +1,11 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  /* Copyright(c) 2020 - 2025 Mucse Corporation. */
> >  
> > +#include <linux/string.h>
> > +
> >  #include "rnpgbe.h"
> >  #include "rnpgbe_hw.h"
> > +#include "rnpgbe_mbx.h"
> >  
> >  /**
> >   * rnpgbe_init_common - Setup common attribute
> > @@ -23,6 +26,8 @@ static void rnpgbe_init_common(struct mucse_hw *hw)
> >  
> >  	mac->mac_addr = hw->hw_addr + RNPGBE_MAC_BASE;
> >  	mac->back = hw;
> > +
> > +	hw->mbx.ops = &mucse_mbx_ops_generic;
> >  }
> >  
> >  /**
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > index fc57258537cf..aee037e3219d 100644
> > --- a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_hw.h
> > @@ -7,6 +7,8 @@
> >  #define RNPGBE_RING_BASE 0x1000
> >  #define RNPGBE_MAC_BASE 0x20000
> >  #define RNPGBE_ETH_BASE 0x10000
> > +/**************** DMA Registers ****************************/
> > +#define RNPGBE_DMA_DUMY 0x000c
> >  /**************** CHIP Resource ****************************/
> >  #define RNPGBE_MAX_QUEUES 8
> >  #endif /* _RNPGBE_HW_H */
> > diff --git a/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > new file mode 100644
> > index 000000000000..1195cf945ad1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/mucse/rnpgbe/rnpgbe_mbx.c
> > @@ -0,0 +1,443 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright(c) 2022 - 2025 Mucse Corporation. */
> > +
> > +#include <linux/pci.h>
> > +#include <linux/errno.h>
> > +#include <linux/delay.h>
> > +#include <linux/iopoll.h>
> > +
> > +#include "rnpgbe.h"
> > +#include "rnpgbe_mbx.h"
> > +#include "rnpgbe_hw.h"
> > +
> > +/**
> > + * mucse_read_mbx - Reads a message from the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> > +int mucse_read_mbx(struct mucse_hw *hw, u32 *msg, u16 size)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	/* limit read size */
> > +	min(size, mbx->size);
> > +	return mbx->ops->read(hw, msg, size);
> > +}
> 
> What's the purpose of min() here if you are anyways passing size to read()?
> 
> The min() call needs to be assigned to size, e.g.: size = min(size,
> mbx->size);
> 

Yes, I will fix this error.

> > +
> > +/**
> > + * mucse_write_mbx - Write a message to the mailbox
> > + * @hw: pointer to the HW structure
> > + * @msg: the message buffer
> > + * @size: length of buffer
> > + *
> > + * @return: 0 on success, negative on failure
> > + **/
> 
> > +
> > +/**
> > + * mucse_mbx_reset - Reset mbx info, sync info from regs
> > + * @hw: pointer to the HW structure
> > + *
> > + * This function reset all mbx variables to default.
> > + **/
> > +static void mucse_mbx_reset(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +	int v;
> > +
> 
> Variable 'v' should be declared as u32 to match the register read.
> 

Got it, I will fix it.

> > +	v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +	hw->mbx.fw_req = v & GENMASK(15, 0);
> > +	hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +	mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +	mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +}
> > +
> > +/**
> > + * mucse_mbx_configure_pf - Configure mbx to use nr_vec interrupt
> > + * @hw: pointer to the HW structure
> > + * @nr_vec: vector number for mbx
> > + * @enable: TRUE for enable, FALSE for disable
> > + *
> > + * This function configure mbx to use interrupt nr_vec.
> > + **/
> > +static void mucse_mbx_configure_pf(struct mucse_hw *hw, int nr_vec,
> > +				   bool enable)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +	u32 v;
> > +
> > +	if (enable) {
> > +		v = mbx_rd32(hw, FW2PF_COUNTER(mbx));
> > +		hw->mbx.fw_req = v & GENMASK(15, 0);
> > +		hw->mbx.fw_ack = (v >> 16) & GENMASK(15, 0);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, FW2PF_MBOX_VEC(mbx), nr_vec);
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), GENMASK(31, 16));
> > +	} else {
> > +		mbx_wr32(hw, FW_PF_MBOX_MASK(mbx), 0xfffffffe);
> > +		mbx_wr32(hw, PF2FW_MBOX_CTRL(mbx), 0);
> > +		mbx_wr32(hw, RNPGBE_DMA_DUMY, 0);
> > +	}
> > +}
> > +
> > +/**
> > + * mucse_init_mbx_params_pf - Set initial values for pf mailbox
> > + * @hw: pointer to the HW structure
> > + *
> > + * Initializes the hw->mbx struct to correct values for pf mailbox
> > + */
> > +static void mucse_init_mbx_params_pf(struct mucse_hw *hw)
> > +{
> > +	struct mucse_mbx_info *mbx = &hw->mbx;
> > +
> > +	mbx->usec_delay = 100;
> > +	mbx->timeout = (4 * 1000 * 1000) / mbx->usec_delay;
> 
> Use appropriate constants like USEC_PER_SEC instead of hardcoded values.
> 

Ok, I will update it.

> > +	mbx->stats.msgs_tx = 0;
> > +	mbx->stats.msgs_rx = 0;
> 
> 
> -- 
> Thanks and Regards,
> Md Danish Anwar
> 
> 

Thanks for your feedback.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ