| 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: Mon, 26 Aug 2019 21:24:04 -0700
From: Jakub Kicinski <jakub.kicinski@...ronome.com>
To: Shannon Nelson <snelson@...sando.io>
Cc: netdev@...r.kernel.org, davem@...emloft.net
Subject: Re: [PATCH v5 net-next 02/18] ionic: Add hardware init and device
commands
On Mon, 26 Aug 2019 14:33:23 -0700, Shannon Nelson wrote:
> The ionic device has a small set of PCI registers, including a
> device control and data space, and a large set of message
> commands.
>
> Signed-off-by: Shannon Nelson <snelson@...sando.io>
> diff --git a/drivers/net/ethernet/pensando/ionic/Makefile b/drivers/net/ethernet/pensando/ionic/Makefile
> index f174e8f7bce1..a23d58519c63 100644
> --- a/drivers/net/ethernet/pensando/ionic/Makefile
> +++ b/drivers/net/ethernet/pensando/ionic/Makefile
> @@ -3,4 +3,5 @@
>
> obj-$(CONFIG_IONIC) := ionic.o
>
> -ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o
> +ionic-y := ionic_main.o ionic_bus_pci.o ionic_devlink.o ionic_dev.o \
> + ionic_debugfs.o
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic.h b/drivers/net/ethernet/pensando/ionic/ionic.h
> index d40077161214..1f3c4a916849 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic.h
> @@ -4,6 +4,10 @@
> #ifndef _IONIC_H_
> #define _IONIC_H_
>
> +#include "ionic_if.h"
> +#include "ionic_dev.h"
> +#include "ionic_devlink.h"
> +
> #define IONIC_DRV_NAME "ionic"
> #define IONIC_DRV_DESCRIPTION "Pensando Ethernet NIC Driver"
> #define IONIC_DRV_VERSION "0.15.0-k"
> @@ -17,10 +21,27 @@
> #define IONIC_SUBDEV_ID_NAPLES_100_4 0x4001
> #define IONIC_SUBDEV_ID_NAPLES_100_8 0x4002
>
> +#define devcmd_timeout 10
nit: upper case?
> struct ionic {
> struct pci_dev *pdev;
> struct device *dev;
> struct devlink *dl;
> + struct devlink_port dl_port;
devlink_port is not used in this patch
> + struct ionic_dev idev;
> + struct mutex dev_cmd_lock; /* lock for dev_cmd operations */
> + struct dentry *dentry;
> + struct ionic_dev_bar bars[IONIC_BARS_MAX];
> + unsigned int num_bars;
> + struct ionic_identity ident;
> };
> + err = ionic_init(ionic);
> + if (err) {
> + dev_err(dev, "Cannot init device: %d, aborting\n", err);
> + goto err_out_teardown;
> + }
> +
> + err = ionic_devlink_register(ionic);
> + if (err)
> + dev_err(dev, "Cannot register devlink: %d\n", err);
>
> return 0;
> +
> +err_out_teardown:
> + ionic_dev_teardown(ionic);
> +err_out_unmap_bars:
> + ionic_unmap_bars(ionic);
> + pci_release_regions(pdev);
> +err_out_pci_clear_master:
> + pci_clear_master(pdev);
> +err_out_pci_disable_device:
> + pci_disable_device(pdev);
> +err_out_debugfs_del_dev:
> + ionic_debugfs_del_dev(ionic);
> +err_out_clear_drvdata:
> + mutex_destroy(&ionic->dev_cmd_lock);
> + ionic_devlink_free(ionic);
> +
> + return err;
> }
>
> static void ionic_remove(struct pci_dev *pdev)
> {
> struct ionic *ionic = pci_get_drvdata(pdev);
>
> + if (!ionic)
How can this be NULL? Usually if this is NULL that means probe()
failed but 'err' was not set properly. Perhaps WARN_ON() here?
> + return;
> +
> + ionic_devlink_unregister(ionic);
> + ionic_reset(ionic);
> + ionic_dev_teardown(ionic);
> + ionic_unmap_bars(ionic);
> + pci_release_regions(pdev);
> + pci_clear_master(pdev);
> + pci_disable_device(pdev);
> + ionic_debugfs_del_dev(ionic);
> + mutex_destroy(&ionic->dev_cmd_lock);
> ionic_devlink_free(ionic);
> }
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> new file mode 100644
> index 000000000000..30a5206bba4e
> --- /dev/null
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -0,0 +1,143 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
> +
> +#ifndef _IONIC_DEV_H_
> +#define _IONIC_DEV_H_
> +
> +#include <linux/mutex.h>
> +#include <linux/workqueue.h>
> +
> +#include "ionic_if.h"
> +#include "ionic_regs.h"
> +
> +struct ionic_dev_bar {
> + void __iomem *vaddr;
> + phys_addr_t bus_addr;
> + unsigned long len;
> + int res_index;
> +};
> +
> +static inline void ionic_struct_size_checks(void)
> +{
> + /* Registers */
> + BUILD_BUG_ON(sizeof(struct ionic_intr) != 32);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_doorbell) != 8);
> + BUILD_BUG_ON(sizeof(struct ionic_intr_status) != 8);
> +
> + BUILD_BUG_ON(sizeof(union ionic_dev_regs) != 4096);
> + BUILD_BUG_ON(sizeof(union ionic_dev_info_regs) != 2048);
> + BUILD_BUG_ON(sizeof(union ionic_dev_cmd_regs) != 2048);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_lif_stats) != 1024);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_admin_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_admin_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_nop_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_nop_comp) != 16);
> +
> + /* Device commands */
> + BUILD_BUG_ON(sizeof(struct ionic_dev_identify_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_identify_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_init_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_init_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_reset_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_reset_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_getattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_getattr_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_setattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_dev_setattr_comp) != 16);
> +
> + /* Port commands */
> + BUILD_BUG_ON(sizeof(struct ionic_port_identify_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_port_identify_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_port_init_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_port_init_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_port_reset_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_port_reset_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_port_getattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_port_getattr_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_port_setattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_port_setattr_comp) != 16);
> +
> + /* LIF commands */
> + BUILD_BUG_ON(sizeof(struct ionic_lif_init_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_init_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_reset_cmd) != 64);
> + BUILD_BUG_ON(sizeof(ionic_lif_reset_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_getattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_getattr_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_setattr_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_lif_setattr_comp) != 16);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_q_init_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_q_init_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_q_control_cmd) != 64);
> + BUILD_BUG_ON(sizeof(ionic_q_control_comp) != 16);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_rx_mode_set_cmd) != 64);
> + BUILD_BUG_ON(sizeof(ionic_rx_mode_set_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_rx_filter_add_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_rx_filter_add_comp) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_rx_filter_del_cmd) != 64);
> + BUILD_BUG_ON(sizeof(ionic_rx_filter_del_comp) != 16);
> +
> + /* RDMA commands */
> + BUILD_BUG_ON(sizeof(struct ionic_rdma_reset_cmd) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_rdma_queue_cmd) != 64);
> +
> + /* Events */
> + BUILD_BUG_ON(sizeof(struct ionic_notifyq_cmd) != 4);
> + BUILD_BUG_ON(sizeof(union ionic_notifyq_comp) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_notifyq_event) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_link_change_event) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_reset_event) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_heartbeat_event) != 64);
> + BUILD_BUG_ON(sizeof(struct ionic_log_event) != 64);
> +
> + /* I/O */
> + BUILD_BUG_ON(sizeof(struct ionic_txq_desc) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_txq_sg_desc) != 128);
> + BUILD_BUG_ON(sizeof(struct ionic_txq_comp) != 16);
> +
> + BUILD_BUG_ON(sizeof(struct ionic_rxq_desc) != 16);
> + BUILD_BUG_ON(sizeof(struct ionic_rxq_sg_desc) != 128);
> + BUILD_BUG_ON(sizeof(struct ionic_rxq_comp) != 16);
static_assert() for all of those? That way you don't need this fake
function.
> +}
> +
> +struct ionic_devinfo {
> + u8 asic_type;
> + u8 asic_rev;
> + char fw_version[IONIC_DEVINFO_FWVERS_BUFLEN + 1];
> + char serial_num[IONIC_DEVINFO_SERIAL_BUFLEN + 1];
> +};
> +
> +struct ionic_dev {
> + union ionic_dev_info_regs __iomem *dev_info_regs;
> + union ionic_dev_cmd_regs __iomem *dev_cmd_regs;
> +
> + u64 __iomem *db_pages;
> + dma_addr_t phy_db_pages;
> +
> + struct ionic_intr __iomem *intr_ctrl;
> + u64 __iomem *intr_status;
> +
> + struct ionic_devinfo dev_info;
> +};
> +
> +struct ionic;
> +
> +void ionic_init_devinfo(struct ionic *ionic);
> +int ionic_dev_setup(struct ionic *ionic);
> +void ionic_dev_teardown(struct ionic *ionic);
> +
> +void ionic_dev_cmd_go(struct ionic_dev *idev, union ionic_dev_cmd *cmd);
> +u8 ionic_dev_cmd_status(struct ionic_dev *idev);
> +bool ionic_dev_cmd_done(struct ionic_dev *idev);
> +void ionic_dev_cmd_comp(struct ionic_dev *idev, union ionic_dev_cmd_comp *comp);
> +
> +void ionic_dev_cmd_identify(struct ionic_dev *idev, u8 ver);
> +void ionic_dev_cmd_init(struct ionic_dev *idev);
> +void ionic_dev_cmd_reset(struct ionic_dev *idev);
> +
> +#endif /* _IONIC_DEV_H_ */
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> index e24ef6971cd5..1ca1e33cca04 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
> @@ -11,8 +11,28 @@
> static int ionic_dl_info_get(struct devlink *dl, struct devlink_info_req *req,
> struct netlink_ext_ack *extack)
> {
> + struct ionic *ionic = devlink_priv(dl);
> + struct ionic_dev *idev = &ionic->idev;
> + char buf[16];
> +
> devlink_info_driver_name_put(req, IONIC_DRV_NAME);
>
> + devlink_info_version_running_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_FW_MGMT,
> + idev->dev_info.fw_version);
Are you sure this is not the FW that controls the data path?
> + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_type);
> + devlink_info_version_fixed_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
> + buf);
Board ID is not ASIC. This is for identifying a board version with all
its components which surround the main ASIC.
> + snprintf(buf, sizeof(buf), "0x%x", idev->dev_info.asic_rev);
> + devlink_info_version_fixed_put(req,
> + DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
> + buf);
ditto
> + devlink_info_serial_number_put(req, idev->dev_info.serial_num);
> +
> return 0;
> }
>
> @@ -41,3 +61,22 @@ void ionic_devlink_free(struct ionic *ionic)
> {
> devlink_free(ionic->dl);
> }
> +
> +int ionic_devlink_register(struct ionic *ionic)
> +{
> + int err;
> +
> + err = devlink_register(ionic->dl, ionic->dev);
> + if (err)
> + dev_warn(ionic->dev, "devlink_register failed: %d\n", err);
> +
> + return err;
> +}
> +
> +void ionic_devlink_unregister(struct ionic *ionic)
> +{
> + if (!ionic || !ionic->dl)
> + return;
Impossiblu
> + devlink_unregister(ionic->dl);
> +}
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
> index 1df50874260a..0690172fc57a 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.h
> @@ -8,5 +8,7 @@
>
> struct ionic *ionic_devlink_alloc(struct device *dev);
> void ionic_devlink_free(struct ionic *ionic);
> +int ionic_devlink_register(struct ionic *ionic);
> +void ionic_devlink_unregister(struct ionic *ionic);
>
> #endif /* _IONIC_DEVLINK_H_ */
Powered by blists - more mailing lists