| 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: <02b934ee-edd9-08f1-3571-5efe7687b546@amd.com>
Date: Wed, 8 Mar 2023 18:05:13 -0800
From: Shannon Nelson <shannon.nelson@....com>
To: Jiri Pirko <jiri@...nulli.us>
Cc: brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
kuba@...nel.org, drivers@...sando.io, leon@...nel.org
Subject: Re: [PATCH RFC v4 net-next 02/13] pds_core: add devcmd device
interfaces
On 3/8/23 1:37 AM, Jiri Pirko wrote:
> Wed, Mar 08, 2023 at 06:12:59AM CET, shannon.nelson@....com wrote:
>> The devcmd interface is the basic connection to the device through the
>> PCI BAR for low level identification and command services. This does
>> the early device initialization and finds the identity data, and adds
>> devcmd routines to be used by later driver bits.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
>> ---
>> drivers/net/ethernet/amd/pds_core/Makefile | 4 +-
>> drivers/net/ethernet/amd/pds_core/core.c | 36 ++
>> drivers/net/ethernet/amd/pds_core/debugfs.c | 67 ++++
>> drivers/net/ethernet/amd/pds_core/dev.c | 350 ++++++++++++++++++++
>> drivers/net/ethernet/amd/pds_core/main.c | 35 +-
>> include/linux/pds/pds_common.h | 63 ++++
>> include/linux/pds/pds_core.h | 52 +++
>> include/linux/pds/pds_intr.h | 161 +++++++++
>> 8 files changed, 764 insertions(+), 4 deletions(-)
>> create mode 100644 drivers/net/ethernet/amd/pds_core/core.c
>> create mode 100644 drivers/net/ethernet/amd/pds_core/dev.c
>> create mode 100644 include/linux/pds/pds_intr.h
>>
>> diff --git a/drivers/net/ethernet/amd/pds_core/Makefile b/drivers/net/ethernet/amd/pds_core/Makefile
>> index b4cc4b242e44..eaca8557ba66 100644
>> --- a/drivers/net/ethernet/amd/pds_core/Makefile
>> +++ b/drivers/net/ethernet/amd/pds_core/Makefile
>> @@ -4,6 +4,8 @@
>> obj-$(CONFIG_PDS_CORE) := pds_core.o
>>
>> pds_core-y := main.o \
>> - devlink.o
>> + devlink.o \
>> + dev.o \
>> + core.o
>>
>> pds_core-$(CONFIG_DEBUG_FS) += debugfs.o
>> diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
>> new file mode 100644
>> index 000000000000..88a6aa42cc28
>> --- /dev/null
>> +++ b/drivers/net/ethernet/amd/pds_core/core.c
>> @@ -0,0 +1,36 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright(c) 2023 Advanced Micro Devices, Inc */
>> +
>> +#include <linux/pds/pds_core.h>
>> +
>> +int pdsc_setup(struct pdsc *pdsc, bool init)
>> +{
>> + int err = 0;
>> +
>> + if (init)
>> + err = pdsc_dev_init(pdsc);
>> + else
>> + err = pdsc_dev_reinit(pdsc);
>> + if (err)
>> + return err;
>> +
>> + clear_bit(PDSC_S_FW_DEAD, &pdsc->state);
>> + return 0;
>> +}
>> +
>> +void pdsc_teardown(struct pdsc *pdsc, bool removing)
>> +{
>> + pdsc_devcmd_reset(pdsc);
>> +
>> + if (removing) {
>> + kfree(pdsc->intr_info);
>> + pdsc->intr_info = NULL;
>> + }
>> +
>> + if (pdsc->kern_dbpage) {
>> + iounmap(pdsc->kern_dbpage);
>> + pdsc->kern_dbpage = NULL;
>> + }
>> +
>> + set_bit(PDSC_S_FW_DEAD, &pdsc->state);
>> +}
>> diff --git a/drivers/net/ethernet/amd/pds_core/debugfs.c b/drivers/net/ethernet/amd/pds_core/debugfs.c
>> index c5cf01ca7853..75376c9f77cf 100644
>> --- a/drivers/net/ethernet/amd/pds_core/debugfs.c
>> +++ b/drivers/net/ethernet/amd/pds_core/debugfs.c
>> @@ -44,4 +44,71 @@ void pdsc_debugfs_del_dev(struct pdsc *pdsc)
>> debugfs_remove_recursive(pdsc->dentry);
>> pdsc->dentry = NULL;
>> }
>> +
>> +static int identity_show(struct seq_file *seq, void *v)
>> +{
>> + struct pdsc *pdsc = seq->private;
>> + struct pds_core_dev_identity *ident;
>> + int vt;
>> +
>> + ident = &pdsc->dev_ident;
>> +
>> + seq_printf(seq, "asic_type: 0x%x\n", pdsc->dev_info.asic_type);
>> + seq_printf(seq, "asic_rev: 0x%x\n", pdsc->dev_info.asic_rev);
>> + seq_printf(seq, "serial_num: %s\n", pdsc->dev_info.serial_num);
>> + seq_printf(seq, "fw_version: %s\n", pdsc->dev_info.fw_version);
>
> What is the exact reason of exposing this here and not trought well
> defined devlink info interface?
These do show up in devlink dev info eventually, but that isn't for
another couple of patches. This gives us info here for debugging the
earlier patches if needed.
>
>
>> + seq_printf(seq, "fw_status: 0x%x\n",
>> + ioread8(&pdsc->info_regs->fw_status));
>> + seq_printf(seq, "fw_heartbeat: 0x%x\n",
>> + ioread32(&pdsc->info_regs->fw_heartbeat));
>> +
>> + seq_printf(seq, "nlifs: %d\n", le32_to_cpu(ident->nlifs));
>> + seq_printf(seq, "nintrs: %d\n", le32_to_cpu(ident->nintrs));
>> + seq_printf(seq, "ndbpgs_per_lif: %d\n", le32_to_cpu(ident->ndbpgs_per_lif));
>> + seq_printf(seq, "intr_coal_mult: %d\n", le32_to_cpu(ident->intr_coal_mult));
>> + seq_printf(seq, "intr_coal_div: %d\n", le32_to_cpu(ident->intr_coal_div));
>> +
>> + seq_puts(seq, "vif_types: ");
>> + for (vt = 0; vt < PDS_DEV_TYPE_MAX; vt++)
>> + seq_printf(seq, "%d ", le16_to_cpu(pdsc->dev_ident.vif_types[vt]));
>> + seq_puts(seq, "\n");
>> +
>> + return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(identity);
>> +
>> +void pdsc_debugfs_add_ident(struct pdsc *pdsc)
>> +{
>> + debugfs_create_file("identity", 0400, pdsc->dentry, pdsc, &identity_fops);
>> +}
>> +
>> +static int irqs_show(struct seq_file *seq, void *v)
>> +{
>> + struct pdsc *pdsc = seq->private;
>> + struct pdsc_intr_info *intr_info;
>> + int i;
>> +
>> + seq_printf(seq, "index vector name (nintrs %d)\n", pdsc->nintrs);
>> +
>> + if (!pdsc->intr_info)
>> + return 0;
>> +
>> + for (i = 0; i < pdsc->nintrs; i++) {
>> + intr_info = &pdsc->intr_info[i];
>> + if (!intr_info->vector)
>> + continue;
>> +
>> + seq_printf(seq, "% 3d % 3d %s\n",
>> + i, intr_info->vector, intr_info->name);
>> + }
>> +
>> + return 0;
>> +}
>> +DEFINE_SHOW_ATTRIBUTE(irqs);
>> +
>> +void pdsc_debugfs_add_irqs(struct pdsc *pdsc)
>> +{
>> + debugfs_create_file("irqs", 0400, pdsc->dentry, pdsc, &irqs_fops);
>> +}
>> +
>> #endif /* CONFIG_DEBUG_FS */
>> diff --git a/drivers/net/ethernet/amd/pds_core/dev.c b/drivers/net/ethernet/amd/pds_core/dev.c
>> new file mode 100644
>> index 000000000000..7a9db1505c1f
>> --- /dev/null
>> +++ b/drivers/net/ethernet/amd/pds_core/dev.c
>> @@ -0,0 +1,350 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright(c) 2023 Advanced Micro Devices, Inc */
>> +
>> +#include <linux/version.h>
>> +#include <linux/errno.h>
>> +#include <linux/pci.h>
>> +#include <linux/utsname.h>
>> +
>> +#include <linux/pds/pds_core.h>
>> +
>> +int pdsc_err_to_errno(enum pds_core_status_code code)
>> +{
>> + switch (code) {
>> + case PDS_RC_SUCCESS:
>> + return 0;
>> + case PDS_RC_EVERSION:
>> + case PDS_RC_EQTYPE:
>> + case PDS_RC_EQID:
>> + case PDS_RC_EINVAL:
>> + case PDS_RC_ENOSUPP:
>> + return -EINVAL;
>> + case PDS_RC_EPERM:
>> + return -EPERM;
>> + case PDS_RC_ENOENT:
>> + return -ENOENT;
>> + case PDS_RC_EAGAIN:
>> + return -EAGAIN;
>> + case PDS_RC_ENOMEM:
>> + return -ENOMEM;
>> + case PDS_RC_EFAULT:
>> + return -EFAULT;
>> + case PDS_RC_EBUSY:
>> + return -EBUSY;
>> + case PDS_RC_EEXIST:
>> + return -EEXIST;
>> + case PDS_RC_EVFID:
>> + return -ENODEV;
>> + case PDS_RC_ECLIENT:
>> + return -ECHILD;
>> + case PDS_RC_ENOSPC:
>> + return -ENOSPC;
>> + case PDS_RC_ERANGE:
>> + return -ERANGE;
>> + case PDS_RC_BAD_ADDR:
>> + return -EFAULT;
>> + case PDS_RC_EOPCODE:
>> + case PDS_RC_EINTR:
>> + case PDS_RC_DEV_CMD:
>> + case PDS_RC_ERROR:
>> + case PDS_RC_ERDMA:
>> + case PDS_RC_EIO:
>> + default:
>> + return -EIO;
>> + }
>> +}
>> +
>> +bool pdsc_is_fw_running(struct pdsc *pdsc)
>> +{
>> + pdsc->fw_status = ioread8(&pdsc->info_regs->fw_status);
>> + pdsc->last_fw_time = jiffies;
>> + pdsc->last_hb = ioread32(&pdsc->info_regs->fw_heartbeat);
>> +
>> + /* Firmware is useful only if the running bit is set and
>> + * fw_status != 0xff (bad PCI read)
>> + */
>> + return (pdsc->fw_status != 0xff) &&
>> + (pdsc->fw_status & PDS_CORE_FW_STS_F_RUNNING);
>> +}
>> +
>> +bool pdsc_is_fw_good(struct pdsc *pdsc)
>> +{
>> + return pdsc_is_fw_running(pdsc) &&
>> + (pdsc->fw_status & PDS_CORE_FW_STS_F_GENERATION) == pdsc->fw_generation;
>> +}
>> +
>> +static u8 pdsc_devcmd_status(struct pdsc *pdsc)
>> +{
>> + return ioread8(&pdsc->cmd_regs->comp.status);
>> +}
>> +
>> +static bool pdsc_devcmd_done(struct pdsc *pdsc)
>> +{
>> + return ioread32(&pdsc->cmd_regs->done) & PDS_CORE_DEV_CMD_DONE;
>> +}
>> +
>> +static void pdsc_devcmd_dbell(struct pdsc *pdsc)
>> +{
>> + iowrite32(0, &pdsc->cmd_regs->done);
>> + iowrite32(1, &pdsc->cmd_regs->doorbell);
>> +}
>> +
>> +static void pdsc_devcmd_clean(struct pdsc *pdsc)
>> +{
>> + iowrite32(0, &pdsc->cmd_regs->doorbell);
>> + memset_io(&pdsc->cmd_regs->cmd, 0, sizeof(pdsc->cmd_regs->cmd));
>> +}
>> +
>> +static const char *pdsc_devcmd_str(int opcode)
>> +{
>> + switch (opcode) {
>> + case PDS_CORE_CMD_NOP:
>> + return "PDS_CORE_CMD_NOP";
>> + case PDS_CORE_CMD_IDENTIFY:
>> + return "PDS_CORE_CMD_IDENTIFY";
>> + case PDS_CORE_CMD_RESET:
>> + return "PDS_CORE_CMD_RESET";
>> + case PDS_CORE_CMD_INIT:
>> + return "PDS_CORE_CMD_INIT";
>> + case PDS_CORE_CMD_FW_DOWNLOAD:
>> + return "PDS_CORE_CMD_FW_DOWNLOAD";
>> + case PDS_CORE_CMD_FW_CONTROL:
>> + return "PDS_CORE_CMD_FW_CONTROL";
>> + default:
>> + return "PDS_CORE_CMD_UNKNOWN";
>> + }
>> +}
>> +
>> +static int pdsc_devcmd_wait(struct pdsc *pdsc, int max_seconds)
>> +{
>> + struct device *dev = pdsc->dev;
>> + unsigned long start_time;
>> + unsigned long max_wait;
>> + unsigned long duration;
>> + int timeout = 0;
>> + int status = 0;
>> + int done = 0;
>> + int err = 0;
>> + int opcode;
>> +
>> + opcode = ioread8(&pdsc->cmd_regs->cmd.opcode);
>> +
>> + start_time = jiffies;
>> + max_wait = start_time + (max_seconds * HZ);
>> +
>> + while (!done && !timeout) {
>> + done = pdsc_devcmd_done(pdsc);
>> + if (done)
>> + break;
>> +
>> + timeout = time_after(jiffies, max_wait);
>> + if (timeout)
>> + break;
>> +
>> + usleep_range(100, 200);
>> + }
>> + duration = jiffies - start_time;
>> +
>> + if (done && duration > HZ)
>> + dev_dbg(dev, "DEVCMD %d %s after %ld secs\n",
>> + opcode, pdsc_devcmd_str(opcode), duration / HZ);
>> +
>> + if (!done || timeout) {
>> + dev_err(dev, "DEVCMD %d %s timeout, done %d timeout %d max_seconds=%d\n",
>> + opcode, pdsc_devcmd_str(opcode), done, timeout,
>> + max_seconds);
>> + err = -ETIMEDOUT;
>> + pdsc_devcmd_clean(pdsc);
>> + }
>> +
>> + status = pdsc_devcmd_status(pdsc);
>> + err = pdsc_err_to_errno(status);
>> + if (status != PDS_RC_SUCCESS && status != PDS_RC_EAGAIN)
>> + dev_err(dev, "DEVCMD %d %s failed, status=%d err %d %pe\n",
>> + opcode, pdsc_devcmd_str(opcode), status, err,
>> + ERR_PTR(err));
>> +
>> + return err;
>> +}
>> +
>> +int pdsc_devcmd_locked(struct pdsc *pdsc, union pds_core_dev_cmd *cmd,
>> + union pds_core_dev_comp *comp, int max_seconds)
>> +{
>> + int err;
>> +
>> + memcpy_toio(&pdsc->cmd_regs->cmd, cmd, sizeof(*cmd));
>> + pdsc_devcmd_dbell(pdsc);
>> + err = pdsc_devcmd_wait(pdsc, max_seconds);
>> + memcpy_fromio(comp, &pdsc->cmd_regs->comp, sizeof(*comp));
>> +
>> + return err;
>> +}
>> +
>> +int pdsc_devcmd(struct pdsc *pdsc, union pds_core_dev_cmd *cmd,
>> + union pds_core_dev_comp *comp, int max_seconds)
>> +{
>> + int err;
>> +
>> + mutex_lock(&pdsc->devcmd_lock);
>> + err = pdsc_devcmd_locked(pdsc, cmd, comp, max_seconds);
>> + mutex_unlock(&pdsc->devcmd_lock);
>> +
>> + return err;
>> +}
>> +
>> +int pdsc_devcmd_init(struct pdsc *pdsc)
>> +{
>> + union pds_core_dev_comp comp = { 0 };
>> + union pds_core_dev_cmd cmd = {
>> + .opcode = PDS_CORE_CMD_INIT,
>> + };
>> +
>> + return pdsc_devcmd(pdsc, &cmd, &comp, pdsc->devcmd_timeout);
>> +}
>> +
>> +int pdsc_devcmd_reset(struct pdsc *pdsc)
>> +{
>> + union pds_core_dev_comp comp = { 0 };
>> + union pds_core_dev_cmd cmd = {
>> + .reset.opcode = PDS_CORE_CMD_RESET,
>> + };
>> +
>> + return pdsc_devcmd(pdsc, &cmd, &comp, pdsc->devcmd_timeout);
>> +}
>> +
>> +static int pdsc_devcmd_identify_locked(struct pdsc *pdsc)
>> +{
>> + union pds_core_dev_comp comp = { 0 };
>> + union pds_core_dev_cmd cmd = {
>> + .identify.opcode = PDS_CORE_CMD_IDENTIFY,
>> + .identify.ver = PDS_CORE_IDENTITY_VERSION_1,
>> + };
>> +
>> + return pdsc_devcmd_locked(pdsc, &cmd, &comp, pdsc->devcmd_timeout);
>> +}
>> +
>> +static void pdsc_init_devinfo(struct pdsc *pdsc)
>> +{
>> + pdsc->dev_info.asic_type = ioread8(&pdsc->info_regs->asic_type);
>> + pdsc->dev_info.asic_rev = ioread8(&pdsc->info_regs->asic_rev);
>> + pdsc->fw_generation = PDS_CORE_FW_STS_F_GENERATION &
>> + ioread8(&pdsc->info_regs->fw_status);
>> +
>> + memcpy_fromio(pdsc->dev_info.fw_version,
>> + pdsc->info_regs->fw_version,
>> + PDS_CORE_DEVINFO_FWVERS_BUFLEN);
>> + pdsc->dev_info.fw_version[PDS_CORE_DEVINFO_FWVERS_BUFLEN] = 0;
>> +
>> + memcpy_fromio(pdsc->dev_info.serial_num,
>> + pdsc->info_regs->serial_num,
>> + PDS_CORE_DEVINFO_SERIAL_BUFLEN);
>> + pdsc->dev_info.serial_num[PDS_CORE_DEVINFO_SERIAL_BUFLEN] = 0;
>> +
>> + dev_dbg(pdsc->dev, "fw_version %s\n", pdsc->dev_info.fw_version);
>> +}
>> +
>> +static int pdsc_identify(struct pdsc *pdsc)
>> +{
>> + struct pds_core_drv_identity drv = { 0 };
>> + size_t sz;
>> + int err;
>> +
>> + drv.drv_type = cpu_to_le32(PDS_DRIVER_LINUX);
>> + drv.kernel_ver = cpu_to_le32(LINUX_VERSION_CODE);
>> + snprintf(drv.kernel_ver_str, sizeof(drv.kernel_ver_str),
>> + "%s %s", utsname()->release, utsname()->version);
>> + snprintf(drv.driver_ver_str, sizeof(drv.driver_ver_str),
>> + "%s %s", PDS_CORE_DRV_NAME, utsname()->release);
>
> Why exactly are you doing this? Looks very wrong.
This helps us when debugging on the DSC side - we can see which host
driver is using the interface (PXE, Linux, etc). This is modeled from
what we have in our out-of-tree ionic driver interface, but I need to
trim it down to be less snoopy.
sln
Powered by blists - more mailing lists