| 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
| ||
|
Date: Thu, 19 Jan 2023 16:29:37 +0000
From: "Lucero Palau, Alejandro" <alejandro.lucero-palau@....com>
To: "Lucero Palau, Alejandro" <alejandro.lucero-palau@....com>,
Jiri Pirko <jiri@...nulli.us>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-net-drivers (AMD-Xilinx)" <linux-net-drivers@....com>,
"davem@...emloft.net" <davem@...emloft.net>,
"kuba@...nel.org" <kuba@...nel.org>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"habetsm@...il.com" <habetsm@...il.com>,
"ecree.xilinx@...il.com" <ecree.xilinx@...il.com>
Subject: Re: [PATCH net-next 1/7] sfc: add devlink support for ef100
On 1/19/23 14:51, Lucero Palau, Alejandro wrote:
> On 1/19/23 12:14, Jiri Pirko wrote:
>> Thu, Jan 19, 2023 at 12:31:34PM CET, alejandro.lucero-palau@....com wrote:
>>> From: Alejandro Lucero <alejandro.lucero-palau@....com>
>>>
>>> Basic support for devlink info command.
>>>
>>> Signed-off-by: Alejandro Lucero <alejandro.lucero-palau@....com>
>>> ---
>>> drivers/net/ethernet/sfc/Kconfig | 1 +
>>> drivers/net/ethernet/sfc/Makefile | 3 +-
>>> drivers/net/ethernet/sfc/ef100_nic.c | 6 +
>>> drivers/net/ethernet/sfc/efx_devlink.c | 427 +++++++++++++++++++++++++
>>> drivers/net/ethernet/sfc/efx_devlink.h | 20 ++
>>> drivers/net/ethernet/sfc/mcdi.c | 72 +++++
>>> drivers/net/ethernet/sfc/mcdi.h | 3 +
>>> drivers/net/ethernet/sfc/net_driver.h | 2 +
>>> 8 files changed, 533 insertions(+), 1 deletion(-)
>>> create mode 100644 drivers/net/ethernet/sfc/efx_devlink.c
>>> create mode 100644 drivers/net/ethernet/sfc/efx_devlink.h
>> Could you please split?
>>
>> At least the devlink introduction part and info implementation.
>>
> Sure.
Just for being sure, would it be fine to have a simple info
implementation first along with the devlink infrastructure then a second
patch adding the more complex devlink info support? I guess I need at
least one operation supported initially.
>>> diff --git a/drivers/net/ethernet/sfc/Kconfig b/drivers/net/ethernet/sfc/Kconfig
>>> index 0950e6b0508f..4af36ba8906b 100644
>>> --- a/drivers/net/ethernet/sfc/Kconfig
>>> +++ b/drivers/net/ethernet/sfc/Kconfig
>>> @@ -22,6 +22,7 @@ config SFC
>>> depends on PTP_1588_CLOCK_OPTIONAL
>>> select MDIO
>>> select CRC32
>>> + select NET_DEVLINK
>>> help
>>> This driver supports 10/40-gigabit Ethernet cards based on
>>> the Solarflare SFC9100-family controllers.
>>> diff --git a/drivers/net/ethernet/sfc/Makefile b/drivers/net/ethernet/sfc/Makefile
>>> index 712a48d00069..55b9c73cd8ef 100644
>>> --- a/drivers/net/ethernet/sfc/Makefile
>>> +++ b/drivers/net/ethernet/sfc/Makefile
>>> @@ -6,7 +6,8 @@ sfc-y += efx.o efx_common.o efx_channels.o nic.o \
>>> mcdi.o mcdi_port.o mcdi_port_common.o \
>>> mcdi_functions.o mcdi_filters.o mcdi_mon.o \
>>> ef100.o ef100_nic.o ef100_netdev.o \
>>> - ef100_ethtool.o ef100_rx.o ef100_tx.o
>>> + ef100_ethtool.o ef100_rx.o ef100_tx.o \
>>> + efx_devlink.o
>>> sfc-$(CONFIG_SFC_MTD) += mtd.o
>>> sfc-$(CONFIG_SFC_SRIOV) += sriov.o ef10_sriov.o ef100_sriov.o ef100_rep.o \
>>> mae.o tc.o tc_bindings.o tc_counters.o
>>> diff --git a/drivers/net/ethernet/sfc/ef100_nic.c b/drivers/net/ethernet/sfc/ef100_nic.c
>>> index ad686c671ab8..14af8f314b83 100644
>>> --- a/drivers/net/ethernet/sfc/ef100_nic.c
>>> +++ b/drivers/net/ethernet/sfc/ef100_nic.c
>>> @@ -27,6 +27,7 @@
>>> #include "tc.h"
>>> #include "mae.h"
>>> #include "rx_common.h"
>>> +#include "efx_devlink.h"
>>>
>>> #define EF100_MAX_VIS 4096
>>> #define EF100_NUM_MCDI_BUFFERS 1
>>> @@ -1124,6 +1125,10 @@ int ef100_probe_netdev_pf(struct efx_nic *efx)
>>> netif_warn(efx, probe, net_dev,
>>> "Failed to probe base mport rc %d; representors will not function\n",
>>> rc);
>>> + } else {
>>> + if (efx_probe_devlink(efx))
>> I don't understand why the devlink register call is here.
>> 1) You can registers it for PF and VF. I don't follow why you do it only
>> for PF.
> We discuss the possibility of creating the devlink interface for VFs,
> but we decided not to. Arguably, this should not be available for VFs
> owned by VMs/containers, or at least in our case, since the interface is
> being used for getting information about the whole device or for
> getting/setting the MAC address. We plan to support more devlink
> functionalities in the near future, so maybe we add such support then if
> we consider it is needed.
>
>> 2) It should be done as the first thing in the probe flow. Then devlink
>> port register, only after that netdev. It makes sense from the
>> perspective of object hierarchy.
> I can not see a problem here. It is not the first thing done in the
> probe function, but I can not see any dependency for being so. And it is
> done before the devlink port registration and before the netdev
> registration what seems to be what other drivers do. What am I missing
> here?
>
>> It is also usual to have the devlink priv as the "main" driver
>> structure storage, see how that is done in mlx5 of mlxsw for example.
> I will check the way others drivers use the devlink priv.
Interestingly, there are 26 calls to devlink_alloc with half of them
using the main driver structure and the other half using a specific
devlink private struct. This is a huge responsibility now for me!
I'm afraid, for the shake of having this merged as soon as possible,
I'll keep the current allocation. I did not suffer any problem with this
approach while implementing the current support. If, for the further
devlink support we plan to add, it turns out to be an issue, we will to
the changes then. I would say it is a main change now or then, and
current work does not justify it now.
>>
>>> + netif_warn(efx, probe, net_dev,
>>> + "Failed to register devlink\n");
>>> }
>>>
>>> rc = efx_init_tc(efx);
>>> @@ -1157,6 +1162,7 @@ void ef100_remove(struct efx_nic *efx)
>>> {
>>> struct ef100_nic_data *nic_data = efx->nic_data;
>>>
>>> + efx_fini_devlink(efx);
>>> efx_mcdi_detach(efx);
>>> efx_mcdi_fini(efx);
>>> if (nic_data)
>>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.c b/drivers/net/ethernet/sfc/efx_devlink.c
>>> new file mode 100644
>>> index 000000000000..c506f8f35d25
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/sfc/efx_devlink.c
>>> @@ -0,0 +1,427 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/****************************************************************************
>>> + * Driver for AMD network controllers and boards
>>> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published
>>> + * by the Free Software Foundation, incorporated herein by reference.
>>> + */
>>> +
>>> +#include <linux/rtc.h>
>>> +#include "net_driver.h"
>>> +#include "ef100_nic.h"
>>> +#include "efx_devlink.h"
>>> +#include "nic.h"
>>> +#include "mcdi.h"
>>> +#include "mcdi_functions.h"
>>> +#include "mcdi_pcol.h"
>>> +
>>> +/* Custom devlink-info version object names for details that do not map to the
>>> + * generic standardized names.
>>> + */
>>> +#define EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC "fw.mgmt.suc"
>>> +#define EFX_DEVLINK_INFO_VERSION_FW_MGMT_CMC "fw.mgmt.cmc"
>>> +#define EFX_DEVLINK_INFO_VERSION_FPGA_REV "fpga.rev"
>>> +#define EFX_DEVLINK_INFO_VERSION_DATAPATH_HW "fpga.app"
>>> +#define EFX_DEVLINK_INFO_VERSION_DATAPATH_FW DEVLINK_INFO_VERSION_GENERIC_FW_APP
>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_BOOT "coproc.boot"
>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_UBOOT "coproc.uboot"
>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_MAIN "coproc.main"
>>> +#define EFX_DEVLINK_INFO_VERSION_SOC_RECOVERY "coproc.recovery"
>>> +#define EFX_DEVLINK_INFO_VERSION_FW_EXPROM "fw.exprom"
>>> +#define EFX_DEVLINK_INFO_VERSION_FW_UEFI "fw.uefi"
>>> +
>>> +#define EFX_MAX_VERSION_INFO_LEN 64
>>> +
>>> +static int efx_devlink_info_nvram_partition(struct efx_nic *efx,
>>> + struct devlink_info_req *req,
>>> + unsigned int partition_type,
>>> + const char *version_name)
>>> +{
>>> + char buf[EFX_MAX_VERSION_INFO_LEN];
>>> + u16 version[4];
>>> + int rc;
>>> +
>>> + rc = efx_mcdi_nvram_metadata(efx, partition_type, NULL, version, NULL,
>>> + 0);
>>> + if (rc)
>>> + return rc;
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u", version[0],
>>> + version[1], version[2], version[3]);
>>> + devlink_info_version_stored_put(req, version_name, buf);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static void efx_devlink_info_stored_versions(struct efx_nic *efx,
>>> + struct devlink_info_req *req)
>>> +{
>>> + efx_devlink_info_nvram_partition(efx, req, NVRAM_PARTITION_TYPE_BUNDLE,
>>> + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID);
>>> + efx_devlink_info_nvram_partition(efx, req,
>>> + NVRAM_PARTITION_TYPE_MC_FIRMWARE,
>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT);
>>> + efx_devlink_info_nvram_partition(efx, req,
>>> + NVRAM_PARTITION_TYPE_SUC_FIRMWARE,
>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC);
>>> + efx_devlink_info_nvram_partition(efx, req,
>>> + NVRAM_PARTITION_TYPE_EXPANSION_ROM,
>>> + EFX_DEVLINK_INFO_VERSION_FW_EXPROM);
>>> + efx_devlink_info_nvram_partition(efx, req,
>>> + NVRAM_PARTITION_TYPE_EXPANSION_UEFI,
>>> + EFX_DEVLINK_INFO_VERSION_FW_UEFI);
>>> +}
>>> +
>>> +#define EFX_MAX_SERIALNUM_LEN (ETH_ALEN * 2 + 1)
>>> +
>>> +static void efx_devlink_info_board_cfg(struct efx_nic *efx,
>>> + struct devlink_info_req *req)
>>> +{
>>> + char sn[EFX_MAX_SERIALNUM_LEN];
>>> + u8 mac_address[ETH_ALEN];
>>> + int rc;
>>> +
>>> + rc = efx_mcdi_get_board_cfg(efx, (u8 *)mac_address, NULL, NULL);
>>> + if (!rc) {
>>> + snprintf(sn, EFX_MAX_SERIALNUM_LEN, "%pm", mac_address);
>>> + devlink_info_serial_number_put(req, sn);
>>> + }
>>> +}
>>> +
>>> +#define EFX_VER_FLAG(_f) \
>>> + (MC_CMD_GET_VERSION_V5_OUT_ ## _f ## _PRESENT_LBN)
>>> +
>>> +static void efx_devlink_info_running_versions(struct efx_nic *efx,
>>> + struct devlink_info_req *req)
>>> +{
>>> + MCDI_DECLARE_BUF(outbuf, MC_CMD_GET_VERSION_V5_OUT_LEN);
>>> + MCDI_DECLARE_BUF(inbuf, MC_CMD_GET_VERSION_EXT_IN_LEN);
>>> + char buf[EFX_MAX_VERSION_INFO_LEN];
>>> + unsigned int flags, build_id;
>>> + union {
>>> + const __le32 *dwords;
>>> + const __le16 *words;
>>> + const char *str;
>>> + } ver;
>>> + struct rtc_time build_date;
>>> + size_t outlength, offset;
>>> + u64 tstamp;
>>> + int rc;
>>> +
>>> + rc = efx_mcdi_rpc(efx, MC_CMD_GET_VERSION, inbuf, sizeof(inbuf),
>>> + outbuf, sizeof(outbuf), &outlength);
>>> + if (rc || outlength < MC_CMD_GET_VERSION_OUT_LEN)
>>> + return;
>>> +
>>> + /* Handle previous output */
>>> + if (outlength < MC_CMD_GET_VERSION_V2_OUT_LEN) {
>>> + ver.words = (__le16 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_EXT_OUT_VERSION);
>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le16_to_cpu(ver.words[0]),
>>> + le16_to_cpu(ver.words[1]),
>>> + le16_to_cpu(ver.words[2]),
>>> + le16_to_cpu(ver.words[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT,
>>> + buf);
>>> + return;
>>> + }
>>> +
>>> + /* Handle V2 additions */
>>> + flags = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_FLAGS);
>>> + if (flags & BIT(EFX_VER_FLAG(BOARD_EXT_INFO))) {
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%s",
>>> + MCDI_PTR(outbuf, GET_VERSION_V2_OUT_BOARD_NAME));
>>> + devlink_info_version_fixed_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
>>> + buf);
>>> +
>>> + /* Favour full board version if present (in V5 or later) */
>>> + if (~flags & BIT(EFX_VER_FLAG(BOARD_VERSION))) {
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u",
>>> + MCDI_DWORD(outbuf,
>>> + GET_VERSION_V2_OUT_BOARD_REVISION));
>>> + devlink_info_version_fixed_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
>>> + buf);
>>> + }
>>> +
>>> + ver.str = MCDI_PTR(outbuf, GET_VERSION_V2_OUT_BOARD_SERIAL);
>>> + if (ver.str[0])
>>> + devlink_info_board_serial_number_put(req, ver.str);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(FPGA_EXT_INFO))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V2_OUT_FPGA_VERSION);
>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u_%c%u",
>>> + le32_to_cpu(ver.dwords[0]),
>>> + 'A' + le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]));
>>> +
>>> + ver.str = MCDI_PTR(outbuf, GET_VERSION_V2_OUT_FPGA_EXTRA);
>>> + if (ver.str[0])
>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset,
>>> + " (%s)", ver.str);
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_FPGA_REV,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(CMC_EXT_INFO))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V2_OUT_CMCFW_VERSION);
>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]),
>>> + le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + tstamp = MCDI_QWORD(outbuf,
>>> + GET_VERSION_V2_OUT_CMCFW_BUILD_DATE);
>>> + if (tstamp) {
>>> + rtc_time64_to_tm(tstamp, &build_date);
>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset,
>>> + " (%ptRd)", &build_date);
>>> + }
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_CMC,
>>> + buf);
>>> + }
>>> +
>>> + ver.words = (__le16 *)MCDI_PTR(outbuf, GET_VERSION_V2_OUT_VERSION);
>>> + offset = snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le16_to_cpu(ver.words[0]), le16_to_cpu(ver.words[1]),
>>> + le16_to_cpu(ver.words[2]), le16_to_cpu(ver.words[3]));
>>> + if (flags & BIT(EFX_VER_FLAG(MCFW_EXT_INFO))) {
>>> + build_id = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_MCFW_BUILD_ID);
>>> + snprintf(&buf[offset], EFX_MAX_VERSION_INFO_LEN - offset,
>>> + " (%x) %s", build_id,
>>> + MCDI_PTR(outbuf, GET_VERSION_V2_OUT_MCFW_BUILD_NAME));
>>> + }
>>> + devlink_info_version_running_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT,
>>> + buf);
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(SUCFW_EXT_INFO))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V2_OUT_SUCFW_VERSION);
>>> + tstamp = MCDI_QWORD(outbuf,
>>> + GET_VERSION_V2_OUT_SUCFW_BUILD_DATE);
>>> + rtc_time64_to_tm(tstamp, &build_date);
>>> + build_id = MCDI_DWORD(outbuf, GET_VERSION_V2_OUT_SUCFW_CHIP_ID);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN,
>>> + "%u.%u.%u.%u type %x (%ptRd)",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]), le32_to_cpu(ver.dwords[3]),
>>> + build_id, &build_date);
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC,
>>> + buf);
>>> + }
>>> +
>>> + if (outlength < MC_CMD_GET_VERSION_V3_OUT_LEN)
>>> + return;
>>> +
>>> + /* Handle V3 additions */
>>> + if (flags & BIT(EFX_VER_FLAG(DATAPATH_HW_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V3_OUT_DATAPATH_HW_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_DATAPATH_HW,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(DATAPATH_FW_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V3_OUT_DATAPATH_FW_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_DATAPATH_FW,
>>> + buf);
>>> + }
>>> +
>>> + if (outlength < MC_CMD_GET_VERSION_V4_OUT_LEN)
>>> + return;
>>> +
>>> + /* Handle V4 additions */
>>> + if (flags & BIT(EFX_VER_FLAG(SOC_BOOT_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V4_OUT_SOC_BOOT_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_SOC_BOOT,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(SOC_UBOOT_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V4_OUT_SOC_UBOOT_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_SOC_UBOOT,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(SOC_MAIN_ROOTFS_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V4_OUT_SOC_MAIN_ROOTFS_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_SOC_MAIN,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(SOC_RECOVERY_BUILDROOT_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V4_OUT_SOC_RECOVERY_BUILDROOT_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_SOC_RECOVERY,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(SUCFW_VERSION)) &&
>>> + ~flags & BIT(EFX_VER_FLAG(SUCFW_EXT_INFO))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V4_OUT_SUCFW_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + EFX_DEVLINK_INFO_VERSION_FW_MGMT_SUC,
>>> + buf);
>>> + }
>>> +
>>> + if (outlength < MC_CMD_GET_VERSION_V5_OUT_LEN)
>>> + return;
>>> +
>>> + /* Handle V5 additions */
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(BOARD_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V5_OUT_BOARD_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
>>> + buf);
>>> + }
>>> +
>>> + if (flags & BIT(EFX_VER_FLAG(BUNDLE_VERSION))) {
>>> + ver.dwords = (__le32 *)MCDI_PTR(outbuf,
>>> + GET_VERSION_V5_OUT_BUNDLE_VERSION);
>>> +
>>> + snprintf(buf, EFX_MAX_VERSION_INFO_LEN, "%u.%u.%u.%u",
>>> + le32_to_cpu(ver.dwords[0]), le32_to_cpu(ver.dwords[1]),
>>> + le32_to_cpu(ver.dwords[2]),
>>> + le32_to_cpu(ver.dwords[3]));
>>> +
>>> + devlink_info_version_running_put(req,
>>> + DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID,
>>> + buf);
>>> + }
>>> +}
>>> +
>>> +#undef EFX_VER_FLAG
>>> +
>>> +static void efx_devlink_info_query_all(struct efx_nic *efx,
>>> + struct devlink_info_req *req)
>>> +{
>>> + efx_devlink_info_board_cfg(efx, req);
>>> + efx_devlink_info_stored_versions(efx, req);
>>> + efx_devlink_info_running_versions(efx, req);
>>> +}
>>> +
>>> +struct efx_devlink {
>>> + struct efx_nic *efx;
>>> +};
>>> +
>>> +static int efx_devlink_info_get(struct devlink *devlink,
>>> + struct devlink_info_req *req,
>>> + struct netlink_ext_ack *extack)
>>> +{
>>> + struct efx_devlink *devlink_private = devlink_priv(devlink);
>>> + struct efx_nic *efx = devlink_private->efx;
>>> +
>>> + efx_devlink_info_query_all(efx, req);
>> I don't understand the reason for having efx_devlink_info_query_all() as
>> a separate function in compare to inline its code here.
>>
> Yes, it could be do as you say. I'll do.
>
> Thanks
>
>>> + return 0;
>>> +}
>>> +
>>> +static const struct devlink_ops sfc_devlink_ops = {
>>> + .info_get = efx_devlink_info_get,
>>> +};
>>> +
>>> +void efx_fini_devlink(struct efx_nic *efx)
>>> +{
>>> + if (efx->devlink) {
>>> + struct efx_devlink *devlink_private;
>>> +
>>> + devlink_private = devlink_priv(efx->devlink);
>>> +
>>> + devlink_unregister(efx->devlink);
>>> + devlink_free(efx->devlink);
>>> + efx->devlink = NULL;
>>> + }
>>> +}
>>> +
>>> +int efx_probe_devlink(struct efx_nic *efx)
>>> +{
>>> + struct efx_devlink *devlink_private;
>>> +
>>> + efx->devlink = devlink_alloc(&sfc_devlink_ops,
>>> + sizeof(struct efx_devlink),
>>> + &efx->pci_dev->dev);
>>> + if (!efx->devlink)
>>> + return -ENOMEM;
>>> + devlink_private = devlink_priv(efx->devlink);
>>> + devlink_private->efx = efx;
>>> +
>>> + devlink_register(efx->devlink);
>>> +
>>> + return 0;
>>> +}
>>> diff --git a/drivers/net/ethernet/sfc/efx_devlink.h b/drivers/net/ethernet/sfc/efx_devlink.h
>>> new file mode 100644
>>> index 000000000000..997f878aea93
>>> --- /dev/null
>>> +++ b/drivers/net/ethernet/sfc/efx_devlink.h
>>> @@ -0,0 +1,20 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/****************************************************************************
>>> + * Driver for AMD network controllers and boards
>>> + * Copyright (C) 2023, Advanced Micro Devices, Inc.
>>> + *
>>> + * This program is free software; you can redistribute it and/or modify it
>>> + * under the terms of the GNU General Public License version 2 as published
>>> + * by the Free Software Foundation, incorporated herein by reference.
>>> + */
>>> +
>>> +#ifndef _EFX_DEVLINK_H
>>> +#define _EFX_DEVLINK_H
>>> +
>>> +#include "net_driver.h"
>>> +#include <net/devlink.h>
>>> +
>>> +int efx_probe_devlink(struct efx_nic *efx);
>>> +void efx_fini_devlink(struct efx_nic *efx);
>>> +
>>> +#endif /* _EFX_DEVLINK_H */
>>> diff --git a/drivers/net/ethernet/sfc/mcdi.c b/drivers/net/ethernet/sfc/mcdi.c
>>> index af338208eae9..328cae82a7d8 100644
>>> --- a/drivers/net/ethernet/sfc/mcdi.c
>>> +++ b/drivers/net/ethernet/sfc/mcdi.c
>>> @@ -2308,6 +2308,78 @@ static int efx_mcdi_nvram_update_finish(struct efx_nic *efx, unsigned int type)
>>> return rc;
>>> }
>>>
>>> +int efx_mcdi_nvram_metadata(struct efx_nic *efx, unsigned int type,
>>> + u32 *subtype, u16 version[4], char *desc,
>>> + size_t descsize)
>>> +{
>>> + MCDI_DECLARE_BUF(inbuf, MC_CMD_NVRAM_METADATA_IN_LEN);
>>> + efx_dword_t *outbuf;
>>> + size_t outlen;
>>> + u32 flags;
>>> + int rc;
>>> +
>>> + outbuf = kzalloc(MC_CMD_NVRAM_METADATA_OUT_LENMAX_MCDI2, GFP_KERNEL);
>>> + if (!outbuf)
>>> + return -ENOMEM;
>>> +
>>> + MCDI_SET_DWORD(inbuf, NVRAM_METADATA_IN_TYPE, type);
>>> +
>>> + rc = efx_mcdi_rpc_quiet(efx, MC_CMD_NVRAM_METADATA, inbuf,
>>> + sizeof(inbuf), outbuf,
>>> + MC_CMD_NVRAM_METADATA_OUT_LENMAX_MCDI2,
>>> + &outlen);
>>> + if (rc)
>>> + goto out_free;
>>> + if (outlen < MC_CMD_NVRAM_METADATA_OUT_LENMIN) {
>>> + rc = -EIO;
>>> + goto out_free;
>>> + }
>>> +
>>> + flags = MCDI_DWORD(outbuf, NVRAM_METADATA_OUT_FLAGS);
>>> +
>>> + if (desc && descsize > 0) {
>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_VALID_LBN)) {
>>> + if (descsize <=
>>> + MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen)) {
>>> + rc = -E2BIG;
>>> + goto out_free;
>>> + }
>>> +
>>> + strncpy(desc,
>>> + MCDI_PTR(outbuf, NVRAM_METADATA_OUT_DESCRIPTION),
>>> + MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen));
>>> + desc[MC_CMD_NVRAM_METADATA_OUT_DESCRIPTION_NUM(outlen)] = '\0';
>>> + } else {
>>> + desc[0] = '\0';
>>> + }
>>> + }
>>> +
>>> + if (subtype) {
>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_SUBTYPE_VALID_LBN))
>>> + *subtype = MCDI_DWORD(outbuf, NVRAM_METADATA_OUT_SUBTYPE);
>>> + else
>>> + *subtype = 0;
>>> + }
>>> +
>>> + if (version) {
>>> + if (flags & BIT(MC_CMD_NVRAM_METADATA_OUT_VERSION_VALID_LBN)) {
>>> + version[0] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_W);
>>> + version[1] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_X);
>>> + version[2] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_Y);
>>> + version[3] = MCDI_WORD(outbuf, NVRAM_METADATA_OUT_VERSION_Z);
>>> + } else {
>>> + version[0] = 0;
>>> + version[1] = 0;
>>> + version[2] = 0;
>>> + version[3] = 0;
>>> + }
>>> + }
>>> +
>>> +out_free:
>>> + kfree(outbuf);
>>> + return rc;
>>> +}
>>> +
>>> int efx_mcdi_mtd_read(struct mtd_info *mtd, loff_t start,
>>> size_t len, size_t *retlen, u8 *buffer)
>>> {
>>> diff --git a/drivers/net/ethernet/sfc/mcdi.h b/drivers/net/ethernet/sfc/mcdi.h
>>> index 7e35fec9da35..63b090587f7a 100644
>>> --- a/drivers/net/ethernet/sfc/mcdi.h
>>> +++ b/drivers/net/ethernet/sfc/mcdi.h
>>> @@ -379,6 +379,9 @@ int efx_mcdi_nvram_info(struct efx_nic *efx, unsigned int type,
>>> bool *protected_out);
>>> int efx_new_mcdi_nvram_test_all(struct efx_nic *efx);
>>> int efx_mcdi_nvram_test_all(struct efx_nic *efx);
>>> +int efx_mcdi_nvram_metadata(struct efx_nic *efx, unsigned int type,
>>> + u32 *subtype, u16 version[4], char *desc,
>>> + size_t descsize);
>>> int efx_mcdi_handle_assertion(struct efx_nic *efx);
>>> int efx_mcdi_set_id_led(struct efx_nic *efx, enum efx_led_mode mode);
>>> int efx_mcdi_wol_filter_set_magic(struct efx_nic *efx, const u8 *mac,
>>> diff --git a/drivers/net/ethernet/sfc/net_driver.h b/drivers/net/ethernet/sfc/net_driver.h
>>> index 3b49e216768b..d036641dc043 100644
>>> --- a/drivers/net/ethernet/sfc/net_driver.h
>>> +++ b/drivers/net/ethernet/sfc/net_driver.h
>>> @@ -994,6 +994,7 @@ enum efx_xdp_tx_queues_mode {
>>> * xdp_rxq_info structures?
>>> * @netdev_notifier: Netdevice notifier.
>>> * @tc: state for TC offload (EF100).
>>> + * @devlink: reference to devlink structure owned by this device
>>> * @mem_bar: The BAR that is mapped into membase.
>>> * @reg_base: Offset from the start of the bar to the function control window.
>>> * @monitor_work: Hardware monitor workitem
>>> @@ -1179,6 +1180,7 @@ struct efx_nic {
>>> struct notifier_block netdev_notifier;
>>> struct efx_tc_state *tc;
>>>
>>> + struct devlink *devlink;
>>> unsigned int mem_bar;
>>> u32 reg_base;
>>>
>>> --
>>> 2.17.1
>>>
Powered by blists - more mailing lists