| 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
| ||
|
Message-ID: <a8483822-fa75-4f73-882b-ce7b69a98752@roeck-us.net>
Date: Thu, 3 Oct 2024 11:05:17 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Sanman Pradhan <sanman.p211993@...il.com>, netdev@...r.kernel.org
Cc: alexanderduyck@...com, kuba@...nel.org, kernel-team@...a.com,
davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
jdelvare@...e.com, horms@...nel.org, mohsin.bashr@...il.com,
sanmanpradhan@...a.com, andrew@...n.ch, linux-hwmon@...r.kernel.org
Subject: Re: [PATCH net-next v2] eth: fbnic: Add hardware monitoring support
via HWMON interface
On 10/3/24 10:36, Sanman Pradhan wrote:
> From: Sanman Pradhan <sanmanpradhan@...a.com>
>
> This patch adds support for hardware monitoring to the fbnic driver,
> allowing for temperature and voltage sensor data to be exposed to
> userspace via the HWMON interface. The driver registers a HWMON device
> and provides callbacks for reading sensor data, enabling system
> admins to monitor the health and operating conditions of fbnic.
>
> Signed-off-by: Sanman Pradhan <sanmanpradhan@...a.com>
>
> ---
> v2:
> - Refined error handling in hwmon registration
> - Improve error handling and logging for hwmon device registration failures
>
> v1: https://lore.kernel.org/netdev/153c5be4-158e-421a-83a5-5632a9263e87@roeck-us.net/T/
>
> ---
> drivers/net/ethernet/meta/fbnic/Makefile | 1 +
> drivers/net/ethernet/meta/fbnic/fbnic.h | 4 +
> drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c | 80 +++++++++++++++++++
> drivers/net/ethernet/meta/fbnic/fbnic_mac.h | 7 ++
> drivers/net/ethernet/meta/fbnic/fbnic_pci.c | 8 +-
> 5 files changed, 99 insertions(+), 1 deletion(-)
> create mode 100644 drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
>
> diff --git a/drivers/net/ethernet/meta/fbnic/Makefile b/drivers/net/ethernet/meta/fbnic/Makefile
> index ed4533a73c57..41494022792a 100644
> --- a/drivers/net/ethernet/meta/fbnic/Makefile
> +++ b/drivers/net/ethernet/meta/fbnic/Makefile
> @@ -11,6 +11,7 @@ fbnic-y := fbnic_devlink.o \
> fbnic_ethtool.o \
> fbnic_fw.o \
> fbnic_hw_stats.o \
> + fbnic_hwmon.o \
> fbnic_irq.o \
> fbnic_mac.o \
> fbnic_netdev.o \
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index 0f9e8d79461c..2d3aa20bc876 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -18,6 +18,7 @@
> struct fbnic_dev {
> struct device *dev;
> struct net_device *netdev;
> + struct device *hwmon;
>
> u32 __iomem *uc_addr0;
> u32 __iomem *uc_addr4;
> @@ -127,6 +128,9 @@ void fbnic_devlink_unregister(struct fbnic_dev *fbd);
> int fbnic_fw_enable_mbx(struct fbnic_dev *fbd);
> void fbnic_fw_disable_mbx(struct fbnic_dev *fbd);
>
> +void fbnic_hwmon_register(struct fbnic_dev *fbd);
> +void fbnic_hwmon_unregister(struct fbnic_dev *fbd);
> +
> int fbnic_pcs_irq_enable(struct fbnic_dev *fbd);
> void fbnic_pcs_irq_disable(struct fbnic_dev *fbd);
>
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> new file mode 100644
> index 000000000000..0ff9c85f08eb
> --- /dev/null
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_hwmon.c
> @@ -0,0 +1,80 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) Meta Platforms, Inc. and affiliates. */
> +
> +#include <linux/hwmon.h>
> +
> +#include "fbnic.h"
> +#include "fbnic_mac.h"
> +
> +static int fbnic_hwmon_sensor_id(enum hwmon_sensor_types type)
> +{
> + if (type == hwmon_temp)
> + return FBNIC_SENSOR_TEMP;
> + if (type == hwmon_in)
> + return FBNIC_SENSOR_VOLTAGE;
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static umode_t fbnic_hwmon_is_visible(const void *drvdata,
> + enum hwmon_sensor_types type,
> + u32 attr, int channel)
> +{
> + if (type == hwmon_temp && attr == hwmon_temp_input)
> + return 0444;
> + if (type == hwmon_in && attr == hwmon_in_input)
> + return 0444;
> +
> + return 0;
> +}
> +
> +static int fbnic_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
> + u32 attr, int channel, long *val)
> +{
> + struct fbnic_dev *fbd = dev_get_drvdata(dev);
> + const struct fbnic_mac *mac = fbd->mac;
> + int id;
> +
> + return id < 0 ? id : mac->get_sensor(fbd, id, val);
How does this work ? Unless I am missing something, "id" is not initialized.
Guenter
> +}
> +
> +static const struct hwmon_ops fbnic_hwmon_ops = {
> + .is_visible = fbnic_hwmon_is_visible,
> + .read = fbnic_hwmon_read,
> +};
> +
> +static const struct hwmon_channel_info *fbnic_hwmon_info[] = {
> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
> + HWMON_CHANNEL_INFO(in, HWMON_I_INPUT),
> + NULL
> +};
> +
> +static const struct hwmon_chip_info fbnic_chip_info = {
> + .ops = &fbnic_hwmon_ops,
> + .info = fbnic_hwmon_info,
> +};
> +
> +void fbnic_hwmon_register(struct fbnic_dev *fbd)
> +{
> + if (!IS_REACHABLE(CONFIG_HWMON))
> + return;
> +
> + fbd->hwmon = hwmon_device_register_with_info(fbd->dev, "fbnic",
> + fbd, &fbnic_chip_info,
> + NULL);
> + if (IS_ERR(fbd->hwmon)) {
> + dev_notice(fbd->dev,
> + "Failed to register hwmon device %pe\n",
> + fbd->hwmon);
> + fbd->hwmon = NULL;
> + }
> +}
> +
> +void fbnic_hwmon_unregister(struct fbnic_dev *fbd)
> +{
> + if (!IS_REACHABLE(CONFIG_HWMON) || !fbd->hwmon)
> + return;
> +
> + hwmon_device_unregister(fbd->hwmon);
> + fbd->hwmon = NULL;
> +}
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> index 476239a9d381..05a591653e09 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_mac.h
> @@ -47,6 +47,11 @@ enum {
> #define FBNIC_LINK_MODE_PAM4 (FBNIC_LINK_50R1)
> #define FBNIC_LINK_MODE_MASK (FBNIC_LINK_AUTO - 1)
>
> +enum fbnic_sensor_id {
> + FBNIC_SENSOR_TEMP, /* Temp in millidegrees Centigrade */
> + FBNIC_SENSOR_VOLTAGE, /* Voltage in millivolts */
> +};
> +
> /* This structure defines the interface hooks for the MAC. The MAC hooks
> * will be configured as a const struct provided with a set of function
> * pointers.
> @@ -83,6 +88,8 @@ struct fbnic_mac {
>
> void (*link_down)(struct fbnic_dev *fbd);
> void (*link_up)(struct fbnic_dev *fbd, bool tx_pause, bool rx_pause);
> +
> + int (*get_sensor)(struct fbnic_dev *fbd, int id, long *val);
> };
>
> int fbnic_mac_init(struct fbnic_dev *fbd);
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
> index a4809fe0fc24..633a9aa39fe2 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_pci.c
> @@ -289,6 +289,8 @@ static int fbnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> fbnic_devlink_register(fbd);
>
> + fbnic_hwmon_register(fbd);
> +
> if (!fbd->dsn) {
> dev_warn(&pdev->dev, "Reading serial number failed\n");
> goto init_failure_mode;
> @@ -297,7 +299,7 @@ static int fbnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> netdev = fbnic_netdev_alloc(fbd);
> if (!netdev) {
> dev_err(&pdev->dev, "Netdev allocation failed\n");
> - goto init_failure_mode;
> + goto ifm_hwmon_unregister;
> }
>
> err = fbnic_netdev_register(netdev);
> @@ -308,6 +310,8 @@ static int fbnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> return 0;
>
> +ifm_hwmon_unregister:
> + fbnic_hwmon_unregister(fbd);
> ifm_free_netdev:
> fbnic_netdev_free(fbd);
> init_failure_mode:
> @@ -345,6 +349,7 @@ static void fbnic_remove(struct pci_dev *pdev)
> fbnic_netdev_free(fbd);
> }
>
> + fbnic_hwmon_unregister(fbd);
> fbnic_devlink_unregister(fbd);
> fbnic_fw_disable_mbx(fbd);
> fbnic_free_irqs(fbd);
> @@ -428,6 +433,7 @@ static int __fbnic_pm_resume(struct device *dev)
> rtnl_unlock();
>
> return 0;
> +
> err_disable_mbx:
> rtnl_unlock();
> fbnic_fw_disable_mbx(fbd);
> --
> 2.43.5
Powered by blists - more mailing lists