Re: [PATCH 01/15] net: hbl_cn: add habanalabs Core Network driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/13/24 10:21, Omer Shpigelman wrote:
Add the hbl_cn driver which will serve both Ethernet and InfiniBand
drivers.
hbl_cn is the layer which is used by the satellite drivers for many shared
operations that are needed by both EN and IB subsystems like QPs, CQs etc.
The CN driver is initialized via auxiliary bus by the habanalabs driver.

Signed-off-by: Omer Shpigelman <oshpigelman@xxxxxxxxx>
Co-developed-by: Abhilash K V <kvabhilash@xxxxxxxxx>
Signed-off-by: Abhilash K V <kvabhilash@xxxxxxxxx>
Co-developed-by: Andrey Agranovich <aagranovich@xxxxxxxxx>
Signed-off-by: Andrey Agranovich <aagranovich@xxxxxxxxx>
Co-developed-by: Bharat Jauhari <bjauhari@xxxxxxxxx>
Signed-off-by: Bharat Jauhari <bjauhari@xxxxxxxxx>
Co-developed-by: David Meriin <dmeriin@xxxxxxxxx>
Signed-off-by: David Meriin <dmeriin@xxxxxxxxx>
Co-developed-by: Sagiv Ozeri <sozeri@xxxxxxxxx>
Signed-off-by: Sagiv Ozeri <sozeri@xxxxxxxxx>
Co-developed-by: Zvika Yehudai <zyehudai@xxxxxxxxx>
Signed-off-by: Zvika Yehudai <zyehudai@xxxxxxxxx>
---
  .../device_drivers/ethernet/index.rst         |    1 +
  .../device_drivers/ethernet/intel/hbl.rst     |   82 +
  MAINTAINERS                                   |   11 +
  drivers/net/ethernet/intel/Kconfig            |   20 +
  drivers/net/ethernet/intel/Makefile           |    1 +
  drivers/net/ethernet/intel/hbl_cn/Makefile    |    9 +
  .../net/ethernet/intel/hbl_cn/common/Makefile |    3 +
  .../net/ethernet/intel/hbl_cn/common/hbl_cn.c | 5954 +++++++++++++++++
  .../net/ethernet/intel/hbl_cn/common/hbl_cn.h | 1627 +++++
  .../ethernet/intel/hbl_cn/common/hbl_cn_drv.c |  220 +
  .../intel/hbl_cn/common/hbl_cn_memory.c       |   40 +
  .../ethernet/intel/hbl_cn/common/hbl_cn_phy.c |   33 +
  .../ethernet/intel/hbl_cn/common/hbl_cn_qp.c  |   13 +
  include/linux/habanalabs/cpucp_if.h           |  125 +-
  include/linux/habanalabs/hl_boot_if.h         |    9 +-
  include/linux/net/intel/cn.h                  |  474 ++
  include/linux/net/intel/cn_aux.h              |  298 +
  include/linux/net/intel/cni.h                 |  636 ++
  18 files changed, 9545 insertions(+), 11 deletions(-)

this is a very big patch, it asks for a split; what's worse, it's
proportional to the size of this series:
 146 files changed, 148514 insertions(+), 70 deletions(-)
which is just too big

[...]

+Support
+=======
+For general information, go to the Intel support website at:
+https://www.intel.com/support/
+
+If an issue is identified with the released source code on a supported kernel
+with a supported adapter, email the specific information related to the issue
+to intel-wired-lan@xxxxxxxxxxxxxxxx.

I'm welcoming you to post next version of the driver to the IWL mailing
list, and before that, to go through our Intel path for ethernet
subsystem (rdma and a few smaller ones also go through that)
(that starts internally, I will PM you the details)

[...]

+++ b/drivers/net/ethernet/intel/hbl_cn/common/hbl_cn.c
@@ -0,0 +1,5954 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright 2020-2024 HabanaLabs, Ltd.
+ * Copyright (C) 2023-2024, Intel Corporation.
+ * All Rights Reserved.
+ */
+
+#include "hbl_cn.h"
+
+#include <linux/file.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+
+#define NIC_MIN_WQS_PER_PORT		2
+
+#define NIC_SEQ_RESETS_TIMEOUT_MS	15000 /* 15 seconds */
+#define NIC_MAX_SEQ_RESETS		3
+
+#define HBL_CN_IPV4_PROTOCOL_UDP	17
+
+/* SOB mask is not expected to change across ASIC. Hence common defines. */
+#define NIC_SOB_INC_MASK		0x80000000
+#define NIC_SOB_VAL_MASK		0x7fff
+
+#define NIC_DUMP_QP_SZ			SZ_4K
+
+#define HBL_AUX2NIC(aux_dev)	\
+	({ \
+		struct hbl_aux_dev *__aux_dev = (aux_dev); \
+		((__aux_dev)->type == HBL_AUX_DEV_ETH) ? \
+		container_of(__aux_dev, struct hbl_cn_device, en_aux_dev) : \
+		container_of(__aux_dev, struct hbl_cn_device, ib_aux_dev); \
+	})

this should be a function

+
+#define RAND_STAT_CNT(cnt) \
+	do { \
+		u32 __cnt = get_random_u32(); \
+		(cnt) = __cnt; \
+		dev_info(hdev->dev, "port %d, %s: %u\n", port, #cnt, __cnt); \

no way for such message, ditto for the function

+	} while (0)
+
+struct hbl_cn_stat hbl_cn_mac_fec_stats[] = {
+	{"correctable_errors", 0x2, 0x3},
+	{"uncorrectable_errors", 0x4, 0x5}
+};
+
+struct hbl_cn_stat hbl_cn_mac_stats_rx[] = {
+	{"Octets", 0x0},
+	{"OctetsReceivedOK", 0x4},
+	{"aAlignmentErrors", 0x8},
+	{"aPAUSEMACCtrlFramesReceived", 0xC},
+	{"aFrameTooLongErrors", 0x10},
+	{"aInRangeLengthErrors", 0x14},
+	{"aFramesReceivedOK", 0x18},
+	{"aFrameCheckSequenceErrors", 0x1C},
+	{"VLANReceivedOK", 0x20},
+	{"ifInErrors", 0x24},
+	{"ifInUcastPkts", 0x28},
+	{"ifInMulticastPkts", 0x2C},
+	{"ifInBroadcastPkts", 0x30},
+	{"DropEvents", 0x34},
+	{"Pkts", 0x38},
+	{"UndersizePkts", 0x3C},
+	{"Pkts64Octets", 0x40},
+	{"Pkts65to127Octets", 0x44},
+	{"Pkts128to255Octets", 0x48},
+	{"Pkts256to511Octets", 0x4C},
+	{"Pkts512to1023Octets", 0x50},
+	{"Pkts1024to1518Octets", 0x54},
+	{"Pkts1519toMaxOctets", 0x58},
+	{"OversizePkts", 0x5C},
+	{"Jabbers", 0x60},
+	{"Fragments", 0x64},
+	{"aCBFCPAUSERx0", 0x68},
+	{"aCBFCPAUSERx1", 0x6C},
+	{"aCBFCPAUSERx2", 0x70},
+	{"aCBFCPAUSERx3", 0x74},
+	{"aCBFCPAUSERx4", 0x78},
+	{"aCBFCPAUSERx5", 0x7C},
+	{"aCBFCPAUSERx6", 0x80},
+	{"aCBFCPAUSERx7", 0x84},
+	{"aMACControlFramesReceived", 0x88}
+};
+
+struct hbl_cn_stat hbl_cn_mac_stats_tx[] = {
+	{"Octets", 0x0},
+	{"OctetsTransmittedOK", 0x4},
+	{"aPAUSEMACCtrlFramesTransmitted", 0x8},
+	{"aFramesTransmittedOK", 0xC},
+	{"VLANTransmittedOK", 0x10},
+	{"ifOutErrors", 0x14},
+	{"ifOutUcastPkts", 0x18},
+	{"ifOutMulticastPkts", 0x1C},
+	{"ifOutBroadcastPkts", 0x20},
+	{"Pkts64Octets", 0x24},
+	{"Pkts65to127Octets", 0x28},
+	{"Pkts128to255Octets", 0x2C},
+	{"Pkts256to511Octets", 0x30},
+	{"Pkts512to1023Octets", 0x34},
+	{"Pkts1024to1518Octets", 0x38},
+	{"Pkts1519toMaxOctets", 0x3C},
+	{"aCBFCPAUSETx0", 0x40},
+	{"aCBFCPAUSETx1", 0x44},
+	{"aCBFCPAUSETx2", 0x48},
+	{"aCBFCPAUSETx3", 0x4C},
+	{"aCBFCPAUSETx4", 0x50},
+	{"aCBFCPAUSETx5", 0x54},
+	{"aCBFCPAUSETx6", 0x58},
+	{"aCBFCPAUSETx7", 0x5C},
+	{"aMACControlFramesTx", 0x60},
+	{"Pkts", 0x64}
+};
+
+static const char pcs_counters_str[][ETH_GSTRING_LEN] = {
+	{"pcs_local_faults"},
+	{"pcs_remote_faults"},
+	{"pcs_remote_fault_reconfig"},
+	{"pcs_link_restores"},
+	{"pcs_link_toggles"},
+};
+
+static size_t pcs_counters_str_len = ARRAY_SIZE(pcs_counters_str);
+size_t hbl_cn_mac_fec_stats_len = ARRAY_SIZE(hbl_cn_mac_fec_stats);
+size_t hbl_cn_mac_stats_rx_len = ARRAY_SIZE(hbl_cn_mac_stats_rx);
+size_t hbl_cn_mac_stats_tx_len = ARRAY_SIZE(hbl_cn_mac_stats_tx);

why those are not const?

+
+static void qps_stop(struct hbl_cn_device *hdev);
+static void qp_destroy_work(struct work_struct *work);
+static int __user_wq_arr_unset(struct hbl_cn_ctx *ctx, struct hbl_cn_port *cn_port, u32 type);
+static void user_cq_destroy(struct kref *kref);
+static void set_app_params_clear(struct hbl_cn_device *hdev);
+static int hbl_cn_ib_cmd_ctrl(struct hbl_aux_dev *aux_dev, void *cn_ib_ctx, u32 op, void *input,
+			      void *output);
+static int hbl_cn_ib_query_mem_handle(struct hbl_aux_dev *ib_aux_dev, u64 mem_handle,
+				      struct hbl_ib_mem_info *info);
+
+static void hbl_cn_reset_stats_counters_port(struct hbl_cn_device *hdev, u32 port);
+static void hbl_cn_late_init(struct hbl_cn_device *hdev);
+static void hbl_cn_late_fini(struct hbl_cn_device *hdev);
+static int hbl_cn_sw_init(struct hbl_cn_device *hdev);
+static void hbl_cn_sw_fini(struct hbl_cn_device *hdev);
+static void hbl_cn_spmu_init(struct hbl_cn_port *cn_port, bool full);
+static int hbl_cn_cmd_port_check(struct hbl_cn_device *hdev, u32 port, u32 flags);
+static void hbl_cn_qps_stop(struct hbl_cn_port *cn_port);
+
+static int hbl_cn_request_irqs(struct hbl_cn_device *hdev)
+{
+	struct hbl_cn_asic_funcs *asic_funcs = hdev->asic_funcs;
+
+	return asic_funcs->request_irqs(hdev);
+}
+
+static void hbl_cn_free_irqs(struct hbl_cn_device *hdev)
+{
+	struct hbl_cn_asic_funcs *asic_funcs =  hdev->asic_funcs;
+
+	asic_funcs->free_irqs(hdev);
+}
+
+static void hbl_cn_synchronize_irqs(struct hbl_aux_dev *cn_aux_dev)
+{
+	struct hbl_cn_device *hdev = cn_aux_dev->priv;
+	struct hbl_cn_asic_funcs *asic_funcs;
+
+	asic_funcs = hdev->asic_funcs;
+
+	asic_funcs->synchronize_irqs(hdev);
+}
+
+void hbl_cn_get_frac_info(u64 numerator, u64 denominator, u64 *integer, u64 *exp)
+{
+	u64 high_digit_n, high_digit_d, integer_tmp, exp_tmp;
+	u8 num_digits_n, num_digits_d;
+	int i;
+
+	num_digits_d = hbl_cn_get_num_of_digits(denominator);
+	high_digit_d = denominator;
+	for (i = 0; i < num_digits_d - 1; i++)
+		high_digit_d /= 10;
+
+	integer_tmp = 0;
+	exp_tmp = 0;
+
+	if (numerator) {
+		num_digits_n = hbl_cn_get_num_of_digits(numerator);
+		high_digit_n = numerator;
+		for (i = 0; i < num_digits_n - 1; i++)
+			high_digit_n /= 10;
+
+		exp_tmp = num_digits_d - num_digits_n;
+
+		if (high_digit_n < high_digit_d) {
+			high_digit_n *= 10;
+			exp_tmp++;
+		}
+
+		integer_tmp = div_u64(high_digit_n, high_digit_d);
+	}
+
+	*integer = integer_tmp;
+	*exp = exp_tmp;
+}

this function sounds suspicious for a network driver, what do you need
it for?

+
+int hbl_cn_read_spmu_counters(struct hbl_cn_port *cn_port, u64 out_data[], u32 *num_out_data)
+{
+	struct hbl_cn_device *hdev = cn_port->hdev;
+	struct hbl_cn_asic_port_funcs *port_funcs;
+	struct hbl_cn_stat *ignore;
+	int rc;
+
+	port_funcs = hdev->asic_funcs->port_funcs;
+
+	port_funcs->spmu_get_stats_info(cn_port, &ignore, num_out_data);

hard to ignore that you deref uninitialized pointer...

please consider going one step back and start with our internal mailing
lists, thank you
Przemek

[...]




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux