| 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: <d63a0509-404a-4abd-90b9-d5ebb408ce98@intel.com>
Date: Mon, 10 Mar 2025 10:33:45 -0700
From: Dave Jiang <dave.jiang@...el.com>
To: Shannon Nelson <shannon.nelson@....com>, jgg@...dia.com,
andrew.gospodarek@...adcom.com, aron.silverton@...cle.com,
dan.j.williams@...el.com, daniel.vetter@...ll.ch, dsahern@...nel.org,
gregkh@...uxfoundation.org, hch@...radead.org, itayavr@...dia.com,
jiri@...dia.com, Jonathan.Cameron@...wei.com, kuba@...nel.org,
lbloch@...dia.com, leonro@...dia.com, linux-cxl@...r.kernel.org,
linux-rdma@...r.kernel.org, netdev@...r.kernel.org, saeedm@...dia.com
Cc: brett.creeley@....com
Subject: Re: [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device
On 3/7/25 11:53 AM, Shannon Nelson wrote:
> Add support for a new fwctl-based auxiliary_device for creating a
> channel for fwctl support into the AMD/Pensando DSC.
>
> Reviewed-by: Leon Romanovsky <leonro@...dia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
Reviewed-by: Dave Jiang <dave.jiang@...el.com>
minor comment below
> ---
> drivers/net/ethernet/amd/pds_core/auxbus.c | 4 ++--
> drivers/net/ethernet/amd/pds_core/core.c | 7 +++++++
> drivers/net/ethernet/amd/pds_core/core.h | 1 +
> drivers/net/ethernet/amd/pds_core/main.c | 14 +++++++++++++-
> include/linux/pds/pds_common.h | 2 ++
> 5 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 563de9e7ce0a..c9aeb56e8174 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -224,8 +224,8 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
> }
>
> /* Verify that the type is supported and enabled. It is not
> - * an error if there is no auxbus device support for this
> - * VF, it just means something else needs to happen with it.
> + * an error if the firmware doesn't support the feature, we
> + * just won't set up an auxiliary_device for it.
s/, we just/; the driver/
DJ
> */
> vt_support = !!le16_to_cpu(pf->dev_ident.vif_types[vt]);
> if (!(vt_support &&
> diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
> index 536635e57727..1eb0d92786f7 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.c
> +++ b/drivers/net/ethernet/amd/pds_core/core.c
> @@ -402,6 +402,9 @@ static int pdsc_core_init(struct pdsc *pdsc)
> }
>
> static struct pdsc_viftype pdsc_viftype_defaults[] = {
> + [PDS_DEV_TYPE_FWCTL] = { .name = PDS_DEV_TYPE_FWCTL_STR,
> + .vif_id = PDS_DEV_TYPE_FWCTL,
> + .dl_id = -1 },
> [PDS_DEV_TYPE_VDPA] = { .name = PDS_DEV_TYPE_VDPA_STR,
> .vif_id = PDS_DEV_TYPE_VDPA,
> .dl_id = DEVLINK_PARAM_GENERIC_ID_ENABLE_VNET },
> @@ -428,6 +431,10 @@ static int pdsc_viftypes_init(struct pdsc *pdsc)
>
> /* See what the Core device has for support */
> vt_support = !!le16_to_cpu(pdsc->dev_ident.vif_types[vt]);
> +
> + if (vt == PDS_DEV_TYPE_FWCTL)
> + pdsc->viftype_status[vt].enabled = true;
> +
> dev_dbg(pdsc->dev, "VIF %s is %ssupported\n",
> pdsc->viftype_status[vt].name,
> vt_support ? "" : "not ");
> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
> index f075e68c64db..0bf320c43083 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.h
> +++ b/drivers/net/ethernet/amd/pds_core/core.h
> @@ -156,6 +156,7 @@ struct pdsc {
> struct dentry *dentry;
> struct device *dev;
> struct pdsc_dev_bar bars[PDS_CORE_BARS_MAX];
> + struct pds_auxiliary_dev *padev;
> struct pdsc_vf *vfs;
> int num_vfs;
> int vf_id;
> diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
> index a3a68889137b..4843f9249a31 100644
> --- a/drivers/net/ethernet/amd/pds_core/main.c
> +++ b/drivers/net/ethernet/amd/pds_core/main.c
> @@ -265,6 +265,10 @@ static int pdsc_init_pf(struct pdsc *pdsc)
>
> mutex_unlock(&pdsc->config_lock);
>
> + err = pdsc_auxbus_dev_add(pdsc, pdsc, PDS_DEV_TYPE_FWCTL, &pdsc->padev);
> + if (err)
> + goto err_out_stop;
> +
> dl = priv_to_devlink(pdsc);
> devl_lock(dl);
> err = devl_params_register(dl, pdsc_dl_params,
> @@ -273,7 +277,7 @@ static int pdsc_init_pf(struct pdsc *pdsc)
> devl_unlock(dl);
> dev_warn(pdsc->dev, "Failed to register devlink params: %pe\n",
> ERR_PTR(err));
> - goto err_out_stop;
> + goto err_out_del_dev;
> }
>
> hr = devl_health_reporter_create(dl, &pdsc_fw_reporter_ops, 0, pdsc);
> @@ -296,6 +300,8 @@ static int pdsc_init_pf(struct pdsc *pdsc)
> err_out_unreg_params:
> devlink_params_unregister(dl, pdsc_dl_params,
> ARRAY_SIZE(pdsc_dl_params));
> +err_out_del_dev:
> + pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev);
> err_out_stop:
> pdsc_stop(pdsc);
> err_out_teardown:
> @@ -427,6 +433,7 @@ static void pdsc_remove(struct pci_dev *pdev)
> * shut themselves down.
> */
> pdsc_sriov_configure(pdev, 0);
> + pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev);
>
> timer_shutdown_sync(&pdsc->wdtimer);
> if (pdsc->wq)
> @@ -485,6 +492,8 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
> if (!IS_ERR(pf))
> pdsc_auxbus_dev_del(pdsc, pf,
> &pf->vfs[pdsc->vf_id].padev);
> + } else {
> + pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev);
> }
>
> pdsc_unmap_bars(pdsc);
> @@ -531,6 +540,9 @@ static void pdsc_reset_done(struct pci_dev *pdev)
> if (!IS_ERR(pf))
> pdsc_auxbus_dev_add(pdsc, pf, PDS_DEV_TYPE_VDPA,
> &pf->vfs[pdsc->vf_id].padev);
> + } else {
> + pdsc_auxbus_dev_add(pdsc, pdsc, PDS_DEV_TYPE_FWCTL,
> + &pdsc->padev);
> }
> }
>
> diff --git a/include/linux/pds/pds_common.h b/include/linux/pds/pds_common.h
> index 5802e1deef24..b193adbe7cc3 100644
> --- a/include/linux/pds/pds_common.h
> +++ b/include/linux/pds/pds_common.h
> @@ -29,6 +29,7 @@ enum pds_core_vif_types {
> PDS_DEV_TYPE_ETH = 3,
> PDS_DEV_TYPE_RDMA = 4,
> PDS_DEV_TYPE_LM = 5,
> + PDS_DEV_TYPE_FWCTL = 6,
>
> /* new ones added before this line */
> PDS_DEV_TYPE_MAX = 16 /* don't change - used in struct size */
> @@ -40,6 +41,7 @@ enum pds_core_vif_types {
> #define PDS_DEV_TYPE_ETH_STR "Eth"
> #define PDS_DEV_TYPE_RDMA_STR "RDMA"
> #define PDS_DEV_TYPE_LM_STR "LM"
> +#define PDS_DEV_TYPE_FWCTL_STR "fwctl"
>
> #define PDS_VDPA_DEV_NAME PDS_CORE_DRV_NAME "." PDS_DEV_TYPE_VDPA_STR
> #define PDS_VFIO_LM_DEV_NAME PDS_CORE_DRV_NAME "." PDS_DEV_TYPE_LM_STR "." PDS_DEV_TYPE_VFIO_STR
Powered by blists - more mailing lists