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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <74b2aab0-25a5-4bb0-ab98-a70fb25f7c54@intel.com>
Date: Wed, 5 Jun 2024 07:03:11 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Mengyuan Lou <mengyuanlou@...-swift.com>, <netdev@...r.kernel.org>
CC: <jiawenwu@...stnetic.com>, <duanqiangwen@...-swift.com>
Subject: Re: [PATCH net-next v4 1/6] net: libwx: Add malibox api for wangxun
 pf drivers

On 6/4/24 17:57, Mengyuan Lou wrote:
> Implements the mailbox interfaces for wangxun pf drivers
> ngbe and txgbe.
> 
> Signed-off-by: Mengyuan Lou <mengyuanlou@...-swift.com>
> ---
>   drivers/net/ethernet/wangxun/libwx/Makefile  |   2 +-
>   drivers/net/ethernet/wangxun/libwx/wx_mbx.c  | 189 +++++++++++++++++++
>   drivers/net/ethernet/wangxun/libwx/wx_mbx.h  |  32 ++++
>   drivers/net/ethernet/wangxun/libwx/wx_type.h |   5 +
>   4 files changed, 227 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_mbx.c
>   create mode 100644 drivers/net/ethernet/wangxun/libwx/wx_mbx.h
> 

Please change your future submissions to have cover letter message-id
linked with actual patches, instead of two separate threads. Please also
post URLs to previous versions.

> diff --git a/drivers/net/ethernet/wangxun/libwx/Makefile b/drivers/net/ethernet/wangxun/libwx/Makefile
> index 42ccd6e4052e..913a978c9032 100644
> --- a/drivers/net/ethernet/wangxun/libwx/Makefile
> +++ b/drivers/net/ethernet/wangxun/libwx/Makefile
> @@ -4,4 +4,4 @@
>   
>   obj-$(CONFIG_LIBWX) += libwx.o
>   
> -libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o
> +libwx-objs := wx_hw.o wx_lib.o wx_ethtool.o wx_mbx.o
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_mbx.c b/drivers/net/ethernet/wangxun/libwx/wx_mbx.c
> new file mode 100644
> index 000000000000..e7d7178a1f13
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_mbx.c
> @@ -0,0 +1,189 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
> +#include <linux/pci.h>
> +#include "wx_type.h"
> +#include "wx_mbx.h"
> +
> +/**
> + *  wx_obtain_mbx_lock_pf - obtain mailbox lock
> + *  @wx: pointer to the HW structure
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if we obtained the mailbox lock
> + **/
> +static int wx_obtain_mbx_lock_pf(struct wx *wx, u16 vf)
> +{
> +	int ret = -EBUSY;
> +	int count = 5;
> +	u32 mailbox;
> +
> +	while (count--) {
> +		/* Take ownership of the buffer */
> +		wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_PFU);
> +
> +		/* reserve mailbox for vf use */
> +		mailbox = rd32(wx, WX_PXMAILBOX(vf));
> +		if (mailbox & WX_PXMAILBOX_PFU) {
> +			ret = 0;
> +			break;
> +		}
> +		udelay(10);

not needed delay on the last loop step

> +	}
> +
> +	if (ret)
> +		wx_err(wx, "Failed to obtain mailbox lock for PF%d", vf);
> +
> +	return ret;
> +}
> +
> +static int wx_check_for_bit_pf(struct wx *wx, u32 mask, int index)
> +{
> +	u32 mbvficr = rd32(wx, WX_MBVFICR(index));
> +	int ret = -EBUSY;

please invert the flow (and use this as general rule), so you will have:
	if (err) {
		error_handling();
		return -ESTH;
	}
	normal_code();
	return 0;

> +
> +	if (mbvficr & mask) {
> +		ret = 0;
> +		wr32(wx, WX_MBVFICR(index), mask);

you are checking if any bit of mask is set, then writing
this value into the register; not an error per-se, but also not an 
obvious thing

> +	}
> +
> +	return ret;
> +}
> +
> +/**
> + *  wx_check_for_ack_pf - checks to see if the VF has ACKed
> + *  @wx: pointer to the HW structure
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_ack_pf(struct wx *wx, u16 vf)
> +{
> +	u32 index = vf / 16, vf_bit = vf % 16;
> +
> +	return wx_check_for_bit_pf(wx,
> +				   FIELD_PREP(WX_MBVFICR_VFACK_MASK, BIT(vf_bit)),
> +				   index);
> +}
> +EXPORT_SYMBOL(wx_check_for_ack_pf);
> +
> +/**
> + *  wx_check_for_msg_pf - checks to see if the VF has sent mail
> + *  @wx: pointer to the HW structure
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_msg_pf(struct wx *wx, u16 vf)
> +{
> +	u32 index = vf / 16, vf_bit = vf % 16;
> +
> +	return wx_check_for_bit_pf(wx,
> +				   FIELD_PREP(WX_MBVFICR_VFREQ_MASK, BIT(vf_bit)),
> +				   index);
> +}
> +EXPORT_SYMBOL(wx_check_for_msg_pf);
> +
> +/**
> + *  wx_write_mbx_pf - Places a message in the mailbox
> + *  @wx: pointer to the HW structure
> + *  @msg: The message buffer
> + *  @size: Length of buffer
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if it successfully copied message into the buffer

s/return:/Return:/
s/SUCCESS/0/ ;)

> + **/
> +int wx_write_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf)
> +{
> +	struct wx_mbx_info *mbx = &wx->mbx;
> +	int ret, i;
> +
> +	if (size > mbx->size) {
> +		wx_err(wx, "Invalid mailbox message size %d", size);
> +		ret = -EINVAL;
> +		goto out_no_write;
> +	}
> +
> +	/* lock the mailbox to prevent pf/vf race condition */
> +	ret = wx_obtain_mbx_lock_pf(wx, vf);
> +	if (ret)
> +		goto out_no_write;
> +
> +	/* flush msg and acks as we are overwriting the message buffer */

how many messages does fit into mbox?

> +	wx_check_for_msg_pf(wx, vf);
> +	wx_check_for_ack_pf(wx, vf);
> +
> +	/* copy the caller specified message to the mailbox memory buffer */
> +	for (i = 0; i < size; i++)
> +		wr32a(wx, WX_PXMBMEM(vf), i, msg[i]);
> +
> +	/* Interrupt VF to tell it a message has been sent and release buffer*/
> +	/* set mirrored mailbox flags */
> +	wr32a(wx, WX_PXMBMEM(vf), WX_VXMAILBOX_SIZE, WX_PXMAILBOX_STS);
> +	wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_STS);
> +
> +out_no_write:
> +	return ret;
> +}
> +EXPORT_SYMBOL(wx_write_mbx_pf);
> +
> +/**
> + *  wx_read_mbx_pf - Read a message from the mailbox
> + *  @wx: pointer to the HW structure
> + *  @msg: The message buffer
> + *  @size: Length of buffer
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if VF copy a message from the mailbox buffer.
> + **/
> +int wx_read_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf)
> +{
> +	struct wx_mbx_info *mbx = &wx->mbx;
> +	int ret;
> +	u16 i;
> +
> +	/* limit read to size of mailbox */
> +	if (size > mbx->size)
> +		size = mbx->size;
> +
> +	/* lock the mailbox to prevent pf/vf race condition */
> +	ret = wx_obtain_mbx_lock_pf(wx, vf);
> +	if (ret)
> +		goto out_no_read;
> +
> +	/* copy the message to the mailbox memory buffer */
> +	for (i = 0; i < size; i++)
> +		msg[i] = rd32a(wx, WX_PXMBMEM(vf), i);
> +
> +	/* Acknowledge the message and release buffer */
> +	/* set mirrored mailbox flags */
> +	wr32a(wx, WX_PXMBMEM(vf), WX_VXMAILBOX_SIZE, WX_PXMAILBOX_ACK);
> +	wr32(wx, WX_PXMAILBOX(vf), WX_PXMAILBOX_ACK);
> +out_no_read:
> +	return ret;
> +}
> +EXPORT_SYMBOL(wx_read_mbx_pf);
> +
> +/**
> + *  wx_check_for_rst_pf - checks to see if the VF has reset
> + *  @wx: pointer to the HW structure
> + *  @vf: the VF index
> + *
> + *  return: return SUCCESS if the VF has set the Status bit or else -EBUSY
> + **/
> +int wx_check_for_rst_pf(struct wx *wx, u16 vf)
> +{
> +	u32 reg_offset = vf / 32;
> +	u32 vf_shift = vf % 32;
> +	int ret = -EBUSY;
> +	u32 vflre = 0;
> +
> +	vflre = rd32(wx, WX_VFLRE(reg_offset));
> +
> +	if (vflre & BIT(vf_shift)) {

ditto error vs normal flow, please apply to whole series

> +		ret = 0;
> +		wr32(wx, WX_VFLREC(reg_offset), BIT(vf_shift));
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(wx_check_for_rst_pf);
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_mbx.h b/drivers/net/ethernet/wangxun/libwx/wx_mbx.h
> new file mode 100644
> index 000000000000..1579096fb6ad
> --- /dev/null
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_mbx.h
> @@ -0,0 +1,32 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright (c) 2015 - 2024 Beijing WangXun Technology Co., Ltd. */
> +#ifndef _WX_MBX_H_
> +#define _WX_MBX_H_
> +
> +#define WX_VXMAILBOX_SIZE    15
> +
> +/* PF Registers */
> +#define WX_PXMAILBOX(i)      (0x600 + (4 * (i))) /* i=[0,63] */
> +#define WX_PXMAILBOX_STS     BIT(0) /* Initiate message send to VF */
> +#define WX_PXMAILBOX_ACK     BIT(1) /* Ack message recv'd from VF */
> +#define WX_PXMAILBOX_PFU     BIT(3) /* PF owns the mailbox buffer */
> +
> +#define WX_PXMBMEM(i)        (0x5000 + (64 * (i))) /* i=[0,63] */
> +
> +#define WX_VFLRE(i)          (0x4A0 + (4 * (i))) /* i=[0,1] */
> +#define WX_VFLREC(i)         (0x4A8 + (4 * (i))) /* i=[0,1] */
> +
> +/* SR-IOV specific macros */
> +#define WX_MBVFICR(i)         (0x480 + (4 * (i))) /* i=[0,3] */
> +#define WX_MBVFICR_VFREQ_MASK GENMASK(15, 0)
> +#define WX_MBVFICR_VFACK_MASK GENMASK(31, 16)
> +
> +#define WX_VT_MSGINFO_MASK    GENMASK(23, 16)
> +
> +int wx_write_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf);
> +int wx_read_mbx_pf(struct wx *wx, u32 *msg, u16 size, u16 vf);
> +int wx_check_for_rst_pf(struct wx *wx, u16 mbx_id);
> +int wx_check_for_msg_pf(struct wx *wx, u16 mbx_id);
> +int wx_check_for_ack_pf(struct wx *wx, u16 mbx_id);
> +
> +#endif /* _WX_MBX_H_ */
> diff --git a/drivers/net/ethernet/wangxun/libwx/wx_type.h b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> index 5aaf7b1fa2db..caa2f4157834 100644
> --- a/drivers/net/ethernet/wangxun/libwx/wx_type.h
> +++ b/drivers/net/ethernet/wangxun/libwx/wx_type.h
> @@ -674,6 +674,10 @@ struct wx_bus_info {
>   	u16 device;
>   };
>   
> +struct wx_mbx_info {
> +	u16 size;
> +};
> +
>   struct wx_thermal_sensor_data {
>   	s16 temp;
>   	s16 alarm_thresh;
> @@ -995,6 +999,7 @@ struct wx {
>   	struct pci_dev *pdev;
>   	struct net_device *netdev;
>   	struct wx_bus_info bus;
> +	struct wx_mbx_info mbx;
>   	struct wx_mac_info mac;
>   	enum em_mac_type mac_type;
>   	enum sp_media_type media_type;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ