* [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices
@ 2025-03-07 18:53 Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
Following along from Jason's work [1] we have our initial patchset
for pds_fwctl - an auxiliary_bus driver for supporting fwctl through the
pds_core driver and its PDS core device.
The PDS core is PCI device that is separate and distinct from the
ionic Eth device and from the other PCI devices that can be supported
by the AMD/Pensando DSC. It is used by pds_vdpa and pds_vfio_pci to
coordinate/communicate with the FW for setting up their services.
Until now the DSC's basic configurations for defining what devices to
support and for getting low-level device debug information have been
through internal commands not available from the host side, requiring
access into the DSC's own OS. Adding the fwctl service allows us to start
bringing these capabilities up into the host, but they don't replace the
existing function-specific tools. For example, these are things that make
the Eth PCI device appear on the PCI bus, while the tuning of the specific
Eth features still go through the standard ethtool/devlink/ip/etc tools.
The first two patches are a bit of clean up for pds_core's add and delete
of auxiliary_devices. Those are followed by a patch to add the new
pds_core.fwctl auxiliary_device. This is only supported by the pds_core
PF and not on any VFs.
The remaining patches add the pds_fwctl driver framework and then fill
in the details for the fwctl services.
[1] https://lore.kernel.org/netdev/0-v5-642aa0c94070+4447f-fwctl_jgg@nvidia.com/
v3: Lots of little cleaning tweaks (no changes to patch 1 and 2)
- rebase on Jason's tree
- better comment on "an error if there is no auxbus device support" (Jonathan)
- don't call pdsc_auxbus_dev_del() if pdsc_auxbus_dev_add() failed (Jonathan)
- struct pdsc *pdsc; is unnecessary in struct pdsfc_dev (Jonathan)
- remove unused UID field in pdsfc_info (Jonathan, Jason)
- change "Word boundary padding" to "Reserved" (Jonathan)
- remove the remaining use of cleanup pattern
- don't lose err from pds_client_adminq_cmd() in pdsfc_identify() (Jason)
- comment on pds_fwctl_ident.features (Jonathan)
- change noisy dev_errs to dev_dbg and remove a couple unnecessary ones (Jonathan, Jason)
- no cast needed on struct fwctl_rpc_pds *rpc = (struct fwctl_rpc_pds *)in (Jonathan)
- more labels and reverse order for cleanup in pdsfc_fw_rpc (Jonathan)
- __counted_by_le() on struct pds_fwctl_query_data (Jonathan)
- other comment comments on new structs (Jonathan)
- use FIELD_GET (Jason)
- Use __aligned_u64 in the uapi headers (Jason)
- call fwctl a subsystem in Docs (Jonathan)
- clean up text around endpoints in Docs (Jonathan)
v2: https://lore.kernel.org/netdev/20250301013554.49511-1-shannon.nelson@amd.com/
- removed the RFC tag
- add a patch to make pdsc_auxbus_dev_del() a void type (Jonathan)
- fix up error handling if pdsc_auxbus_dev_add() fails in probe (Jonathan)
- fix auxiliary spelling in commit subject header (Jonathan)
- clean up of code around use of __free() gizmo (Jonathan, David)
- removed extra whitespace and dev_xxx() calls (Leon)
- copy ident info from DMA and release DMA memory in probe (Jonathan)
- use dev_err_probe() (Jonathan)
- add counted_by_le(num_entries) (Jonathan, David)
- convert num_entries from __le32 to host in get_endpoints() (Jonathan)
- remove unnecessary variable inits (Jonathan, Leon)
v1: https://lore.kernel.org/netdev/20250211234854.52277-1-shannon.nelson@amd.com/
Brett Creeley (1):
pds_fwctl: add rpc and query support
Shannon Nelson (5):
pds_core: make pdsc_auxbus_dev_del() void
pds_core: specify auxiliary_device to be created
pds_core: add new fwctl auxiliary_device
pds_fwctl: initial driver framework
pds_fwctl: add Documentation entries
Documentation/userspace-api/fwctl/fwctl.rst | 1 +
Documentation/userspace-api/fwctl/index.rst | 1 +
.../userspace-api/fwctl/pds_fwctl.rst | 40 ++
MAINTAINERS | 7 +
drivers/fwctl/Kconfig | 10 +
drivers/fwctl/Makefile | 1 +
drivers/fwctl/pds/Makefile | 4 +
drivers/fwctl/pds/main.c | 506 ++++++++++++++++++
drivers/net/ethernet/amd/pds_core/auxbus.c | 44 +-
drivers/net/ethernet/amd/pds_core/core.c | 7 +
drivers/net/ethernet/amd/pds_core/core.h | 8 +-
drivers/net/ethernet/amd/pds_core/devlink.c | 7 +-
drivers/net/ethernet/amd/pds_core/main.c | 25 +-
include/linux/pds/pds_adminq.h | 268 ++++++++++
include/linux/pds/pds_common.h | 2 +
include/uapi/fwctl/fwctl.h | 1 +
include/uapi/fwctl/pds.h | 42 ++
17 files changed, 940 insertions(+), 34 deletions(-)
create mode 100644 Documentation/userspace-api/fwctl/pds_fwctl.rst
create mode 100644 drivers/fwctl/pds/Makefile
create mode 100644 drivers/fwctl/pds/main.c
create mode 100644 include/uapi/fwctl/pds.h
--
2.17.1
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-10 16:58 ` Dave Jiang
2025-03-07 18:53 ` [PATCH v3 2/6] pds_core: specify auxiliary_device to be created Shannon Nelson
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
Since there really is no useful return, advertising a return value
is rather misleading. Make pdsc_auxbus_dev_del() a void function.
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/net/ethernet/amd/pds_core/auxbus.c | 7 +------
drivers/net/ethernet/amd/pds_core/core.h | 2 +-
drivers/net/ethernet/amd/pds_core/devlink.c | 6 ++++--
3 files changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
index 2babea110991..78fba368e797 100644
--- a/drivers/net/ethernet/amd/pds_core/auxbus.c
+++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
@@ -175,13 +175,9 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
return padev;
}
-int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
+void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
{
struct pds_auxiliary_dev *padev;
- int err = 0;
-
- if (!cf)
- return -ENODEV;
mutex_lock(&pf->config_lock);
@@ -195,7 +191,6 @@ int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
pf->vfs[cf->vf_id].padev = NULL;
mutex_unlock(&pf->config_lock);
- return err;
}
int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
index 14522d6d5f86..631a59cfdd7e 100644
--- a/drivers/net/ethernet/amd/pds_core/core.h
+++ b/drivers/net/ethernet/amd/pds_core/core.h
@@ -304,7 +304,7 @@ int pdsc_register_notify(struct notifier_block *nb);
void pdsc_unregister_notify(struct notifier_block *nb);
void pdsc_notify(unsigned long event, void *data);
int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
-int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
+void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
void pdsc_process_adminq(struct pdsc_qcq *qcq);
void pdsc_work_thread(struct work_struct *work);
diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index 44971e71991f..4e2b92ddef6f 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -56,8 +56,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
for (vf_id = 0; vf_id < pdsc->num_vfs; vf_id++) {
struct pdsc *vf = pdsc->vfs[vf_id].vf;
- err = ctx->val.vbool ? pdsc_auxbus_dev_add(vf, pdsc) :
- pdsc_auxbus_dev_del(vf, pdsc);
+ if (ctx->val.vbool)
+ err = pdsc_auxbus_dev_add(vf, pdsc);
+ else
+ pdsc_auxbus_dev_del(vf, pdsc);
}
return err;
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 2/6] pds_core: specify auxiliary_device to be created
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-10 17:26 ` Dave Jiang
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
In preparation for adding a new auxiliary_device for the
PF, make the vif type an argument to pdsc_auxbus_dev_add().
We also now pass in the address to where we'll keep the new
padev pointer so that the caller can specify where to save it
but we can still change it under the mutex and keep the mutex
usage within the function.
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/net/ethernet/amd/pds_core/auxbus.c | 37 ++++++++++-----------
drivers/net/ethernet/amd/pds_core/core.h | 7 ++--
drivers/net/ethernet/amd/pds_core/devlink.c | 5 +--
drivers/net/ethernet/amd/pds_core/main.c | 11 +++---
4 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
index 78fba368e797..563de9e7ce0a 100644
--- a/drivers/net/ethernet/amd/pds_core/auxbus.c
+++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
@@ -175,29 +175,32 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
return padev;
}
-void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
+void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
+ struct pds_auxiliary_dev **pd_ptr)
{
struct pds_auxiliary_dev *padev;
+ if (!*pd_ptr)
+ return;
+
mutex_lock(&pf->config_lock);
- padev = pf->vfs[cf->vf_id].padev;
- if (padev) {
- pds_client_unregister(pf, padev->client_id);
- auxiliary_device_delete(&padev->aux_dev);
- auxiliary_device_uninit(&padev->aux_dev);
- padev->client_id = 0;
- }
- pf->vfs[cf->vf_id].padev = NULL;
+ padev = *pd_ptr;
+ pds_client_unregister(pf, padev->client_id);
+ auxiliary_device_delete(&padev->aux_dev);
+ auxiliary_device_uninit(&padev->aux_dev);
+ padev->client_id = 0;
+ *pd_ptr = NULL;
mutex_unlock(&pf->config_lock);
}
-int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
+int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
+ enum pds_core_vif_types vt,
+ struct pds_auxiliary_dev **pd_ptr)
{
struct pds_auxiliary_dev *padev;
char devname[PDS_DEVNAME_LEN];
- enum pds_core_vif_types vt;
unsigned long mask;
u16 vt_support;
int client_id;
@@ -206,6 +209,9 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
if (!cf)
return -ENODEV;
+ if (vt >= PDS_DEV_TYPE_MAX)
+ return -EINVAL;
+
mutex_lock(&pf->config_lock);
mask = BIT_ULL(PDSC_S_FW_DEAD) |
@@ -217,17 +223,10 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
goto out_unlock;
}
- /* We only support vDPA so far, so it is the only one to
- * be verified that it is available in the Core device and
- * enabled in the devlink param. In the future this might
- * become a loop for several VIF types.
- */
-
/* 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.
*/
- vt = PDS_DEV_TYPE_VDPA;
vt_support = !!le16_to_cpu(pf->dev_ident.vif_types[vt]);
if (!(vt_support &&
pf->viftype_status[vt].supported &&
@@ -253,7 +252,7 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
err = PTR_ERR(padev);
goto out_unlock;
}
- pf->vfs[cf->vf_id].padev = padev;
+ *pd_ptr = padev;
out_unlock:
mutex_unlock(&pf->config_lock);
diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
index 631a59cfdd7e..f075e68c64db 100644
--- a/drivers/net/ethernet/amd/pds_core/core.h
+++ b/drivers/net/ethernet/amd/pds_core/core.h
@@ -303,8 +303,11 @@ void pdsc_health_thread(struct work_struct *work);
int pdsc_register_notify(struct notifier_block *nb);
void pdsc_unregister_notify(struct notifier_block *nb);
void pdsc_notify(unsigned long event, void *data);
-int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
-void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
+int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
+ enum pds_core_vif_types vt,
+ struct pds_auxiliary_dev **pd_ptr);
+void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
+ struct pds_auxiliary_dev **pd_ptr);
void pdsc_process_adminq(struct pdsc_qcq *qcq);
void pdsc_work_thread(struct work_struct *work);
diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
index 4e2b92ddef6f..c5c787df61a4 100644
--- a/drivers/net/ethernet/amd/pds_core/devlink.c
+++ b/drivers/net/ethernet/amd/pds_core/devlink.c
@@ -57,9 +57,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
struct pdsc *vf = pdsc->vfs[vf_id].vf;
if (ctx->val.vbool)
- err = pdsc_auxbus_dev_add(vf, pdsc);
+ err = pdsc_auxbus_dev_add(vf, pdsc, vt_entry->vif_id,
+ &pdsc->vfs[vf_id].padev);
else
- pdsc_auxbus_dev_del(vf, pdsc);
+ pdsc_auxbus_dev_del(vf, pdsc, &pdsc->vfs[vf_id].padev);
}
return err;
diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
index 660268ff9562..a3a68889137b 100644
--- a/drivers/net/ethernet/amd/pds_core/main.c
+++ b/drivers/net/ethernet/amd/pds_core/main.c
@@ -190,7 +190,8 @@ static int pdsc_init_vf(struct pdsc *vf)
devl_unlock(dl);
pf->vfs[vf->vf_id].vf = vf;
- err = pdsc_auxbus_dev_add(vf, pf);
+ err = pdsc_auxbus_dev_add(vf, pf, PDS_DEV_TYPE_VDPA,
+ &pf->vfs[vf->vf_id].padev);
if (err) {
devl_lock(dl);
devl_unregister(dl);
@@ -417,7 +418,7 @@ static void pdsc_remove(struct pci_dev *pdev)
pf = pdsc_get_pf_struct(pdsc->pdev);
if (!IS_ERR(pf)) {
- pdsc_auxbus_dev_del(pdsc, pf);
+ pdsc_auxbus_dev_del(pdsc, pf, &pf->vfs[pdsc->vf_id].padev);
pf->vfs[pdsc->vf_id].vf = NULL;
}
} else {
@@ -482,7 +483,8 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
pf = pdsc_get_pf_struct(pdsc->pdev);
if (!IS_ERR(pf))
- pdsc_auxbus_dev_del(pdsc, pf);
+ pdsc_auxbus_dev_del(pdsc, pf,
+ &pf->vfs[pdsc->vf_id].padev);
}
pdsc_unmap_bars(pdsc);
@@ -527,7 +529,8 @@ static void pdsc_reset_done(struct pci_dev *pdev)
pf = pdsc_get_pf_struct(pdsc->pdev);
if (!IS_ERR(pf))
- pdsc_auxbus_dev_add(pdsc, pf);
+ pdsc_auxbus_dev_add(pdsc, pf, PDS_DEV_TYPE_VDPA,
+ &pf->vfs[pdsc->vf_id].padev);
}
}
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 2/6] pds_core: specify auxiliary_device to be created Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-10 17:33 ` Dave Jiang
2025-03-12 18:22 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
` (2 subsequent siblings)
5 siblings, 2 replies; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
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@nvidia.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
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.
*/
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
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 4/6] pds_fwctl: initial driver framework
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
` (2 preceding siblings ...)
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-10 18:28 ` Dave Jiang
2025-03-12 18:25 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 6/6] pds_fwctl: add Documentation entries Shannon Nelson
5 siblings, 2 replies; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
Initial files for adding a new fwctl driver for the AMD/Pensando PDS
devices. This sets up a simple auxiliary_bus driver that registers
with fwctl subsystem. It expects that a pds_core device has set up
the auxiliary_device pds_core.fwctl
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
MAINTAINERS | 7 ++
drivers/fwctl/Kconfig | 10 ++
drivers/fwctl/Makefile | 1 +
drivers/fwctl/pds/Makefile | 4 +
drivers/fwctl/pds/main.c | 169 +++++++++++++++++++++++++++++++++
include/linux/pds/pds_adminq.h | 83 ++++++++++++++++
include/uapi/fwctl/fwctl.h | 1 +
include/uapi/fwctl/pds.h | 26 +++++
8 files changed, 301 insertions(+)
create mode 100644 drivers/fwctl/pds/Makefile
create mode 100644 drivers/fwctl/pds/main.c
create mode 100644 include/uapi/fwctl/pds.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 3381e41dcf37..c63fd76a3684 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9576,6 +9576,13 @@ L: linux-kernel@vger.kernel.org
S: Maintained
F: drivers/fwctl/mlx5/
+FWCTL PDS DRIVER
+M: Brett Creeley <brett.creeley@amd.com>
+R: Shannon Nelson <shannon.nelson@amd.com>
+L: linux-kernel@vger.kernel.org
+S: Maintained
+F: drivers/fwctl/pds/
+
GALAXYCORE GC0308 CAMERA SENSOR DRIVER
M: Sebastian Reichel <sre@kernel.org>
L: linux-media@vger.kernel.org
diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
index f802cf5d4951..b5583b12a011 100644
--- a/drivers/fwctl/Kconfig
+++ b/drivers/fwctl/Kconfig
@@ -19,5 +19,15 @@ config FWCTL_MLX5
This will allow configuration and debug tools to work out of the box on
mainstream kernel.
+ If you don't know what to do here, say N.
+
+config FWCTL_PDS
+ tristate "AMD/Pensando pds fwctl driver"
+ depends on PDS_CORE
+ help
+ The pds_fwctl driver provides an fwctl interface for a user process
+ to access the debug and configuration information of the AMD/Pensando
+ DSC hardware family.
+
If you don't know what to do here, say N.
endif
diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
index 1c535f694d7f..c093b5f661d6 100644
--- a/drivers/fwctl/Makefile
+++ b/drivers/fwctl/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_FWCTL) += fwctl.o
obj-$(CONFIG_FWCTL_MLX5) += mlx5/
+obj-$(CONFIG_FWCTL_PDS) += pds/
fwctl-y += main.o
diff --git a/drivers/fwctl/pds/Makefile b/drivers/fwctl/pds/Makefile
new file mode 100644
index 000000000000..cc2317c07be1
--- /dev/null
+++ b/drivers/fwctl/pds/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_FWCTL_PDS) += pds_fwctl.o
+
+pds_fwctl-y += main.o
diff --git a/drivers/fwctl/pds/main.c b/drivers/fwctl/pds/main.c
new file mode 100644
index 000000000000..27942315a602
--- /dev/null
+++ b/drivers/fwctl/pds/main.c
@@ -0,0 +1,169 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) Advanced Micro Devices, Inc */
+
+#include <linux/module.h>
+#include <linux/auxiliary_bus.h>
+#include <linux/pci.h>
+#include <linux/vmalloc.h>
+
+#include <uapi/fwctl/fwctl.h>
+#include <uapi/fwctl/pds.h>
+#include <linux/fwctl.h>
+
+#include <linux/pds/pds_common.h>
+#include <linux/pds/pds_core_if.h>
+#include <linux/pds/pds_adminq.h>
+#include <linux/pds/pds_auxbus.h>
+
+struct pdsfc_uctx {
+ struct fwctl_uctx uctx;
+ u32 uctx_caps;
+};
+
+struct pdsfc_dev {
+ struct fwctl_device fwctl;
+ struct pds_auxiliary_dev *padev;
+ u32 caps;
+ struct pds_fwctl_ident ident;
+};
+
+static int pdsfc_open_uctx(struct fwctl_uctx *uctx)
+{
+ struct pdsfc_dev *pdsfc = container_of(uctx->fwctl, struct pdsfc_dev, fwctl);
+ struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
+
+ pdsfc_uctx->uctx_caps = pdsfc->caps;
+
+ return 0;
+}
+
+static void pdsfc_close_uctx(struct fwctl_uctx *uctx)
+{
+}
+
+static void *pdsfc_info(struct fwctl_uctx *uctx, size_t *length)
+{
+ struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
+ struct fwctl_info_pds *info;
+
+ info = kzalloc(sizeof(*info), GFP_KERNEL);
+ if (!info)
+ return ERR_PTR(-ENOMEM);
+
+ info->uctx_caps = pdsfc_uctx->uctx_caps;
+
+ return info;
+}
+
+static int pdsfc_identify(struct pdsfc_dev *pdsfc)
+{
+ struct device *dev = &pdsfc->fwctl.dev;
+ union pds_core_adminq_comp comp = {0};
+ union pds_core_adminq_cmd cmd;
+ struct pds_fwctl_ident *ident;
+ dma_addr_t ident_pa;
+ int err;
+
+ ident = dma_alloc_coherent(dev->parent, sizeof(*ident), &ident_pa, GFP_KERNEL);
+ err = dma_mapping_error(dev->parent, ident_pa);
+ if (err) {
+ dev_err(dev, "Failed to map ident buffer\n");
+ return err;
+ }
+
+ cmd = (union pds_core_adminq_cmd) {
+ .fwctl_ident = {
+ .opcode = PDS_FWCTL_CMD_IDENT,
+ .version = 0,
+ .len = cpu_to_le32(sizeof(*ident)),
+ .ident_pa = cpu_to_le64(ident_pa),
+ }
+ };
+
+ err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
+ if (err)
+ dev_err(dev, "Failed to send adminq cmd opcode: %u err: %d\n",
+ cmd.fwctl_ident.opcode, err);
+ else
+ pdsfc->ident = *ident;
+
+ dma_free_coherent(dev->parent, sizeof(*ident), ident, ident_pa);
+
+ return err;
+}
+
+static void *pdsfc_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
+ void *in, size_t in_len, size_t *out_len)
+{
+ return NULL;
+}
+
+static const struct fwctl_ops pdsfc_ops = {
+ .device_type = FWCTL_DEVICE_TYPE_PDS,
+ .uctx_size = sizeof(struct pdsfc_uctx),
+ .open_uctx = pdsfc_open_uctx,
+ .close_uctx = pdsfc_close_uctx,
+ .info = pdsfc_info,
+ .fw_rpc = pdsfc_fw_rpc,
+};
+
+static int pdsfc_probe(struct auxiliary_device *adev,
+ const struct auxiliary_device_id *id)
+{
+ struct pds_auxiliary_dev *padev =
+ container_of(adev, struct pds_auxiliary_dev, aux_dev);
+ struct device *dev = &adev->dev;
+ struct pdsfc_dev *pdsfc;
+ int err;
+
+ pdsfc = fwctl_alloc_device(&padev->vf_pdev->dev, &pdsfc_ops,
+ struct pdsfc_dev, fwctl);
+ if (!pdsfc)
+ return dev_err_probe(dev, -ENOMEM, "Failed to allocate fwctl device struct\n");
+ pdsfc->padev = padev;
+
+ err = pdsfc_identify(pdsfc);
+ if (err) {
+ fwctl_put(&pdsfc->fwctl);
+ return dev_err_probe(dev, err, "Failed to identify device\n");
+ }
+
+ err = fwctl_register(&pdsfc->fwctl);
+ if (err) {
+ fwctl_put(&pdsfc->fwctl);
+ return dev_err_probe(dev, err, "Failed to register device\n");
+ }
+
+ auxiliary_set_drvdata(adev, pdsfc);
+
+ return 0;
+}
+
+static void pdsfc_remove(struct auxiliary_device *adev)
+{
+ struct pdsfc_dev *pdsfc = auxiliary_get_drvdata(adev);
+
+ fwctl_unregister(&pdsfc->fwctl);
+ fwctl_put(&pdsfc->fwctl);
+}
+
+static const struct auxiliary_device_id pdsfc_id_table[] = {
+ {.name = PDS_CORE_DRV_NAME "." PDS_DEV_TYPE_FWCTL_STR },
+ {}
+};
+MODULE_DEVICE_TABLE(auxiliary, pdsfc_id_table);
+
+static struct auxiliary_driver pdsfc_driver = {
+ .name = "pds_fwctl",
+ .probe = pdsfc_probe,
+ .remove = pdsfc_remove,
+ .id_table = pdsfc_id_table,
+};
+
+module_auxiliary_driver(pdsfc_driver);
+
+MODULE_IMPORT_NS("FWCTL");
+MODULE_DESCRIPTION("pds fwctl driver");
+MODULE_AUTHOR("Shannon Nelson <shannon.nelson@amd.com>");
+MODULE_AUTHOR("Brett Creeley <brett.creeley@amd.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/pds/pds_adminq.h b/include/linux/pds/pds_adminq.h
index 4b4e9a98b37b..22c6d77b3dcb 100644
--- a/include/linux/pds/pds_adminq.h
+++ b/include/linux/pds/pds_adminq.h
@@ -1179,6 +1179,84 @@ struct pds_lm_host_vf_status_cmd {
u8 status;
};
+enum pds_fwctl_cmd_opcode {
+ PDS_FWCTL_CMD_IDENT = 70,
+};
+
+/**
+ * struct pds_fwctl_cmd - Firmware control command structure
+ * @opcode: Opcode
+ * @rsvd: Reserved
+ * @ep: Endpoint identifier
+ * @op: Operation identifier
+ */
+struct pds_fwctl_cmd {
+ u8 opcode;
+ u8 rsvd[3];
+ __le32 ep;
+ __le32 op;
+} __packed;
+
+/**
+ * struct pds_fwctl_comp - Firmware control completion structure
+ * @status: Status of the firmware control operation
+ * @rsvd: Reserved
+ * @comp_index: Completion index in little-endian format
+ * @rsvd2: Reserved
+ * @color: Color bit indicating the state of the completion
+ */
+struct pds_fwctl_comp {
+ u8 status;
+ u8 rsvd;
+ __le16 comp_index;
+ u8 rsvd2[11];
+ u8 color;
+} __packed;
+
+/**
+ * struct pds_fwctl_ident_cmd - Firmware control identification command structure
+ * @opcode: Operation code for the command
+ * @rsvd: Reserved
+ * @version: Interface version
+ * @rsvd2: Reserved
+ * @len: Length of the identification data
+ * @ident_pa: Physical address of the identification data
+ */
+struct pds_fwctl_ident_cmd {
+ u8 opcode;
+ u8 rsvd;
+ u8 version;
+ u8 rsvd2;
+ __le32 len;
+ __le64 ident_pa;
+} __packed;
+
+/* future feature bits here
+ * enum pds_fwctl_features {
+ * };
+ * (compilers don't like empty enums)
+ */
+
+/**
+ * struct pds_fwctl_ident - Firmware control identification structure
+ * @features: Supported features (enum pds_fwctl_features)
+ * @version: Interface version
+ * @rsvd: Reserved
+ * @max_req_sz: Maximum request size
+ * @max_resp_sz: Maximum response size
+ * @max_req_sg_elems: Maximum number of request SGs
+ * @max_resp_sg_elems: Maximum number of response SGs
+ */
+struct pds_fwctl_ident {
+ __le64 features;
+ u8 version;
+ u8 rsvd[3];
+ __le32 max_req_sz;
+ __le32 max_resp_sz;
+ u8 max_req_sg_elems;
+ u8 max_resp_sg_elems;
+} __packed;
+
union pds_core_adminq_cmd {
u8 opcode;
u8 bytes[64];
@@ -1216,6 +1294,9 @@ union pds_core_adminq_cmd {
struct pds_lm_dirty_enable_cmd lm_dirty_enable;
struct pds_lm_dirty_disable_cmd lm_dirty_disable;
struct pds_lm_dirty_seq_ack_cmd lm_dirty_seq_ack;
+
+ struct pds_fwctl_cmd fwctl;
+ struct pds_fwctl_ident_cmd fwctl_ident;
};
union pds_core_adminq_comp {
@@ -1243,6 +1324,8 @@ union pds_core_adminq_comp {
struct pds_lm_state_size_comp lm_state_size;
struct pds_lm_dirty_status_comp lm_dirty_status;
+
+ struct pds_fwctl_comp fwctl;
};
#ifndef __CHECKER__
diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
index c2d5abc5a726..716ac0eee42d 100644
--- a/include/uapi/fwctl/fwctl.h
+++ b/include/uapi/fwctl/fwctl.h
@@ -44,6 +44,7 @@ enum fwctl_device_type {
FWCTL_DEVICE_TYPE_ERROR = 0,
FWCTL_DEVICE_TYPE_MLX5 = 1,
FWCTL_DEVICE_TYPE_CXL = 2,
+ FWCTL_DEVICE_TYPE_PDS = 4,
};
/**
diff --git a/include/uapi/fwctl/pds.h b/include/uapi/fwctl/pds.h
new file mode 100644
index 000000000000..558e030b7583
--- /dev/null
+++ b/include/uapi/fwctl/pds.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/* Copyright(c) Advanced Micro Devices, Inc */
+
+/*
+ * fwctl interface info for pds_fwctl
+ */
+
+#ifndef _UAPI_FWCTL_PDS_H_
+#define _UAPI_FWCTL_PDS_H_
+
+#include <linux/types.h>
+
+/*
+ * struct fwctl_info_pds
+ *
+ * Return basic information about the FW interface available.
+ */
+struct fwctl_info_pds {
+ __u32 uctx_caps;
+};
+
+enum pds_fwctl_capabilities {
+ PDS_FWCTL_QUERY_CAP = 0,
+ PDS_FWCTL_SEND_CAP,
+};
+#endif /* _UAPI_FWCTL_PDS_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 5/6] pds_fwctl: add rpc and query support
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
` (3 preceding siblings ...)
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-07 23:38 ` Jason Gunthorpe
2025-03-12 18:29 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 6/6] pds_fwctl: add Documentation entries Shannon Nelson
5 siblings, 2 replies; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
From: Brett Creeley <brett.creeley@amd.com>
The pds_fwctl driver doesn't know what RPC operations are available
in the firmware, so also doesn't know what scope they might have. The
userland utility supplies the firmware "endpoint" and "operation" id values
and this driver queries the firmware for endpoints and their available
operations. The operation descriptions include the scope information
which the driver uses for scope testing.
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
drivers/fwctl/pds/main.c | 339 ++++++++++++++++++++++++++++++++-
include/linux/pds/pds_adminq.h | 185 ++++++++++++++++++
include/uapi/fwctl/pds.h | 16 ++
3 files changed, 539 insertions(+), 1 deletion(-)
diff --git a/drivers/fwctl/pds/main.c b/drivers/fwctl/pds/main.c
index 27942315a602..5b5e99ab1297 100644
--- a/drivers/fwctl/pds/main.c
+++ b/drivers/fwctl/pds/main.c
@@ -20,11 +20,21 @@ struct pdsfc_uctx {
u32 uctx_caps;
};
+struct pdsfc_rpc_endpoint_info {
+ u32 endpoint;
+ dma_addr_t operations_pa;
+ struct pds_fwctl_query_data *operations;
+ struct mutex lock; /* lock for endpoint info management */
+};
+
struct pdsfc_dev {
struct fwctl_device fwctl;
struct pds_auxiliary_dev *padev;
u32 caps;
struct pds_fwctl_ident ident;
+ dma_addr_t endpoints_pa;
+ struct pds_fwctl_query_data *endpoints;
+ struct pdsfc_rpc_endpoint_info *endpoint_info;
};
static int pdsfc_open_uctx(struct fwctl_uctx *uctx)
@@ -92,10 +102,327 @@ static int pdsfc_identify(struct pdsfc_dev *pdsfc)
return err;
}
+static void pdsfc_free_endpoints(struct pdsfc_dev *pdsfc)
+{
+ struct device *dev = &pdsfc->fwctl.dev;
+ int i;
+
+ if (!pdsfc->endpoints)
+ return;
+
+ for (i = 0; pdsfc->endpoint_info && i < pdsfc->endpoints->num_entries; i++)
+ mutex_destroy(&pdsfc->endpoint_info[i].lock);
+ vfree(pdsfc->endpoint_info);
+ pdsfc->endpoint_info = NULL;
+ dma_free_coherent(dev->parent, PAGE_SIZE,
+ pdsfc->endpoints, pdsfc->endpoints_pa);
+ pdsfc->endpoints = NULL;
+ pdsfc->endpoints_pa = DMA_MAPPING_ERROR;
+}
+
+static void pdsfc_free_operations(struct pdsfc_dev *pdsfc)
+{
+ struct device *dev = &pdsfc->fwctl.dev;
+ u32 num_endpoints;
+ int i;
+
+ num_endpoints = le32_to_cpu(pdsfc->endpoints->num_entries);
+ for (i = 0; i < num_endpoints; i++) {
+ struct pdsfc_rpc_endpoint_info *ei = &pdsfc->endpoint_info[i];
+
+ if (ei->operations) {
+ dma_free_coherent(dev->parent, PAGE_SIZE,
+ ei->operations, ei->operations_pa);
+ ei->operations = NULL;
+ ei->operations_pa = DMA_MAPPING_ERROR;
+ }
+ }
+}
+
+static struct pds_fwctl_query_data *pdsfc_get_endpoints(struct pdsfc_dev *pdsfc,
+ dma_addr_t *pa)
+{
+ struct device *dev = &pdsfc->fwctl.dev;
+ union pds_core_adminq_comp comp = {0};
+ struct pds_fwctl_query_data *data;
+ union pds_core_adminq_cmd cmd;
+ dma_addr_t data_pa;
+ int err;
+
+ data = dma_alloc_coherent(dev->parent, PAGE_SIZE, &data_pa, GFP_KERNEL);
+ err = dma_mapping_error(dev, data_pa);
+ if (err) {
+ dev_err(dev, "Failed to map endpoint list\n");
+ return ERR_PTR(err);
+ }
+
+ cmd = (union pds_core_adminq_cmd) {
+ .fwctl_query = {
+ .opcode = PDS_FWCTL_CMD_QUERY,
+ .entity = PDS_FWCTL_RPC_ROOT,
+ .version = 0,
+ .query_data_buf_len = cpu_to_le32(PAGE_SIZE),
+ .query_data_buf_pa = cpu_to_le64(data_pa),
+ }
+ };
+
+ err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
+ if (err) {
+ dev_err(dev, "Failed to send adminq cmd opcode: %u entity: %u err: %d\n",
+ cmd.fwctl_query.opcode, cmd.fwctl_query.entity, err);
+ dma_free_coherent(dev->parent, PAGE_SIZE, data, data_pa);
+ return ERR_PTR(err);
+ }
+
+ *pa = data_pa;
+
+ return data;
+}
+
+static int pdsfc_init_endpoints(struct pdsfc_dev *pdsfc)
+{
+ struct pds_fwctl_query_data_endpoint *ep_entry;
+ u32 num_endpoints;
+ int i;
+
+ pdsfc->endpoints = pdsfc_get_endpoints(pdsfc, &pdsfc->endpoints_pa);
+ if (IS_ERR(pdsfc->endpoints))
+ return PTR_ERR(pdsfc->endpoints);
+
+ num_endpoints = le32_to_cpu(pdsfc->endpoints->num_entries);
+ pdsfc->endpoint_info = vcalloc(num_endpoints,
+ sizeof(*pdsfc->endpoint_info));
+ if (!pdsfc->endpoint_info) {
+ pdsfc_free_endpoints(pdsfc);
+ return -ENOMEM;
+ }
+
+ ep_entry = (struct pds_fwctl_query_data_endpoint *)pdsfc->endpoints->entries;
+ for (i = 0; i < num_endpoints; i++) {
+ mutex_init(&pdsfc->endpoint_info[i].lock);
+ pdsfc->endpoint_info[i].endpoint = ep_entry[i].id;
+ }
+
+ return 0;
+}
+
+static struct pds_fwctl_query_data *pdsfc_get_operations(struct pdsfc_dev *pdsfc,
+ dma_addr_t *pa, u32 ep)
+{
+ struct device *dev = &pdsfc->fwctl.dev;
+ union pds_core_adminq_comp comp = {0};
+ struct pds_fwctl_query_data *data;
+ union pds_core_adminq_cmd cmd;
+ dma_addr_t data_pa;
+ int err;
+
+ /* Query the operations list for the given endpoint */
+ data = dma_alloc_coherent(dev->parent, PAGE_SIZE, &data_pa, GFP_KERNEL);
+ err = dma_mapping_error(dev->parent, data_pa);
+ if (err) {
+ dev_err(dev, "Failed to map operations list\n");
+ return ERR_PTR(err);
+ }
+
+ cmd = (union pds_core_adminq_cmd) {
+ .fwctl_query = {
+ .opcode = PDS_FWCTL_CMD_QUERY,
+ .entity = PDS_FWCTL_RPC_ENDPOINT,
+ .version = 0,
+ .query_data_buf_len = cpu_to_le32(PAGE_SIZE),
+ .query_data_buf_pa = cpu_to_le64(data_pa),
+ .ep = cpu_to_le32(ep),
+ }
+ };
+
+ err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
+ if (err) {
+ dev_err(dev, "Failed to send adminq cmd opcode: %u entity: %u err: %d\n",
+ cmd.fwctl_query.opcode, cmd.fwctl_query.entity, err);
+ dma_free_coherent(dev->parent, PAGE_SIZE, data, data_pa);
+ return ERR_PTR(err);
+ }
+
+ *pa = data_pa;
+
+ return data;
+}
+
+static int pdsfc_validate_rpc(struct pdsfc_dev *pdsfc,
+ struct fwctl_rpc_pds *rpc,
+ enum fwctl_rpc_scope scope)
+{
+ struct pds_fwctl_query_data_operation *op_entry;
+ struct pdsfc_rpc_endpoint_info *ep_info = NULL;
+ struct device *dev = &pdsfc->fwctl.dev;
+ int i;
+
+ /* validate rpc in_len & out_len based
+ * on ident.max_req_sz & max_resp_sz
+ */
+ if (rpc->in.len > pdsfc->ident.max_req_sz) {
+ dev_dbg(dev, "Invalid request size %u, max %u\n",
+ rpc->in.len, pdsfc->ident.max_req_sz);
+ return -EINVAL;
+ }
+
+ if (rpc->out.len > pdsfc->ident.max_resp_sz) {
+ dev_dbg(dev, "Invalid response size %u, max %u\n",
+ rpc->out.len, pdsfc->ident.max_resp_sz);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < pdsfc->endpoints->num_entries; i++) {
+ if (pdsfc->endpoint_info[i].endpoint == rpc->in.ep) {
+ ep_info = &pdsfc->endpoint_info[i];
+ break;
+ }
+ }
+ if (!ep_info) {
+ dev_dbg(dev, "Invalid endpoint %d\n", rpc->in.ep);
+ return -EINVAL;
+ }
+
+ /* query and cache this endpoint's operations */
+ mutex_lock(&ep_info->lock);
+ if (!ep_info->operations) {
+ ep_info->operations = pdsfc_get_operations(pdsfc,
+ &ep_info->operations_pa,
+ rpc->in.ep);
+ if (!ep_info->operations) {
+ mutex_unlock(&ep_info->lock);
+ return -ENOMEM;
+ }
+ }
+ mutex_unlock(&ep_info->lock);
+
+ /* reject unsupported and/or out of scope commands */
+ op_entry = (struct pds_fwctl_query_data_operation *)ep_info->operations->entries;
+ for (i = 0; i < ep_info->operations->num_entries; i++) {
+ if (PDS_FWCTL_RPC_OPCODE_CMP(rpc->in.op, op_entry[i].id)) {
+ if (scope < op_entry[i].scope)
+ return -EPERM;
+ return 0;
+ }
+ }
+
+ dev_dbg(dev, "Invalid operation %d for endpoint %d\n", rpc->in.op, rpc->in.ep);
+
+ return -EINVAL;
+}
+
static void *pdsfc_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
void *in, size_t in_len, size_t *out_len)
{
- return NULL;
+ struct pdsfc_dev *pdsfc = container_of(uctx->fwctl, struct pdsfc_dev, fwctl);
+ struct device *dev = &uctx->fwctl->dev;
+ union pds_core_adminq_comp comp = {0};
+ dma_addr_t out_payload_dma_addr = 0;
+ dma_addr_t in_payload_dma_addr = 0;
+ struct fwctl_rpc_pds *rpc = in;
+ union pds_core_adminq_cmd cmd;
+ void *out_payload = NULL;
+ void *in_payload = NULL;
+ void *out = NULL;
+ int err;
+
+ err = pdsfc_validate_rpc(pdsfc, rpc, scope);
+ if (err)
+ return ERR_PTR(err);
+
+ if (rpc->in.len > 0) {
+ in_payload = kzalloc(rpc->in.len, GFP_KERNEL);
+ if (!in_payload) {
+ dev_err(dev, "Failed to allocate in_payload\n");
+ err = -ENOMEM;
+ goto err_out;
+ }
+
+ if (copy_from_user(in_payload, u64_to_user_ptr(rpc->in.payload),
+ rpc->in.len)) {
+ dev_dbg(dev, "Failed to copy in_payload from user\n");
+ err = -EFAULT;
+ goto err_in_payload;
+ }
+
+ in_payload_dma_addr = dma_map_single(dev->parent, in_payload,
+ rpc->in.len, DMA_TO_DEVICE);
+ err = dma_mapping_error(dev->parent, in_payload_dma_addr);
+ if (err) {
+ dev_dbg(dev, "Failed to map in_payload\n");
+ goto err_in_payload;
+ }
+ }
+
+ if (rpc->out.len > 0) {
+ out_payload = kzalloc(rpc->out.len, GFP_KERNEL);
+ if (!out_payload) {
+ dev_dbg(dev, "Failed to allocate out_payload\n");
+ err = -ENOMEM;
+ goto err_out_payload;
+ }
+
+ out_payload_dma_addr = dma_map_single(dev->parent, out_payload,
+ rpc->out.len, DMA_FROM_DEVICE);
+ err = dma_mapping_error(dev->parent, out_payload_dma_addr);
+ if (err) {
+ dev_dbg(dev, "Failed to map out_payload\n");
+ goto err_out_payload;
+ }
+ }
+
+ cmd = (union pds_core_adminq_cmd) {
+ .fwctl_rpc = {
+ .opcode = PDS_FWCTL_CMD_RPC,
+ .flags = PDS_FWCTL_RPC_IND_REQ | PDS_FWCTL_RPC_IND_RESP,
+ .ep = cpu_to_le32(rpc->in.ep),
+ .op = cpu_to_le32(rpc->in.op),
+ .req_pa = cpu_to_le64(in_payload_dma_addr),
+ .req_sz = cpu_to_le32(rpc->in.len),
+ .resp_pa = cpu_to_le64(out_payload_dma_addr),
+ .resp_sz = cpu_to_le32(rpc->out.len),
+ }
+ };
+
+ err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
+ if (err) {
+ dev_dbg(dev, "%s: ep %d op %x req_pa %llx req_sz %d req_sg %d resp_pa %llx resp_sz %d resp_sg %d err %d\n",
+ __func__, rpc->in.ep, rpc->in.op,
+ cmd.fwctl_rpc.req_pa, cmd.fwctl_rpc.req_sz, cmd.fwctl_rpc.req_sg_elems,
+ cmd.fwctl_rpc.resp_pa, cmd.fwctl_rpc.resp_sz, cmd.fwctl_rpc.resp_sg_elems,
+ err);
+ goto done;
+ }
+
+ dynamic_hex_dump("out ", DUMP_PREFIX_OFFSET, 16, 1, out_payload, rpc->out.len, true);
+
+ if (copy_to_user(u64_to_user_ptr(rpc->out.payload), out_payload, rpc->out.len)) {
+ dev_dbg(dev, "Failed to copy out_payload to user\n");
+ out = ERR_PTR(-EFAULT);
+ goto done;
+ }
+
+ rpc->out.retval = le32_to_cpu(comp.fwctl_rpc.err);
+ *out_len = in_len;
+ out = in;
+
+done:
+ if (out_payload_dma_addr)
+ dma_unmap_single(dev->parent, out_payload_dma_addr,
+ rpc->out.len, DMA_FROM_DEVICE);
+err_out_payload:
+ kfree(out_payload);
+
+ if (in_payload_dma_addr)
+ dma_unmap_single(dev->parent, in_payload_dma_addr,
+ rpc->in.len, DMA_TO_DEVICE);
+err_in_payload:
+ kfree(in_payload);
+err_out:
+ if (err)
+ return ERR_PTR(err);
+
+ return out;
}
static const struct fwctl_ops pdsfc_ops = {
@@ -128,8 +455,15 @@ static int pdsfc_probe(struct auxiliary_device *adev,
return dev_err_probe(dev, err, "Failed to identify device\n");
}
+ err = pdsfc_init_endpoints(pdsfc);
+ if (err) {
+ fwctl_put(&pdsfc->fwctl);
+ return dev_err_probe(dev, err, "Failed to init endpoints\n");
+ }
+
err = fwctl_register(&pdsfc->fwctl);
if (err) {
+ pdsfc_free_endpoints(pdsfc);
fwctl_put(&pdsfc->fwctl);
return dev_err_probe(dev, err, "Failed to register device\n");
}
@@ -144,6 +478,9 @@ static void pdsfc_remove(struct auxiliary_device *adev)
struct pdsfc_dev *pdsfc = auxiliary_get_drvdata(adev);
fwctl_unregister(&pdsfc->fwctl);
+ pdsfc_free_operations(pdsfc);
+ pdsfc_free_endpoints(pdsfc);
+
fwctl_put(&pdsfc->fwctl);
}
diff --git a/include/linux/pds/pds_adminq.h b/include/linux/pds/pds_adminq.h
index 22c6d77b3dcb..b7f6a1c04957 100644
--- a/include/linux/pds/pds_adminq.h
+++ b/include/linux/pds/pds_adminq.h
@@ -1181,6 +1181,8 @@ struct pds_lm_host_vf_status_cmd {
enum pds_fwctl_cmd_opcode {
PDS_FWCTL_CMD_IDENT = 70,
+ PDS_FWCTL_CMD_RPC = 71,
+ PDS_FWCTL_CMD_QUERY = 72,
};
/**
@@ -1257,6 +1259,185 @@ struct pds_fwctl_ident {
u8 max_resp_sg_elems;
} __packed;
+enum pds_fwctl_query_entity {
+ PDS_FWCTL_RPC_ROOT = 0,
+ PDS_FWCTL_RPC_ENDPOINT = 1,
+ PDS_FWCTL_RPC_OPERATION = 2,
+};
+
+#define PDS_FWCTL_RPC_OPCODE_CMD_SHIFT 0
+#define PDS_FWCTL_RPC_OPCODE_CMD_MASK GENMASK(15, PDS_FWCTL_RPC_OPCODE_CMD_SHIFT)
+#define PDS_FWCTL_RPC_OPCODE_VER_SHIFT 16
+#define PDS_FWCTL_RPC_OPCODE_VER_MASK GENMASK(23, PDS_FWCTL_RPC_OPCODE_VER_SHIFT)
+
+#define PDS_FWCTL_RPC_OPCODE_GET_CMD(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_CMD_MASK, op)
+#define PDS_FWCTL_RPC_OPCODE_GET_VER(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_VER_MASK, op)
+
+#define PDS_FWCTL_RPC_OPCODE_CMP(op1, op2) \
+ (PDS_FWCTL_RPC_OPCODE_GET_CMD(op1) == PDS_FWCTL_RPC_OPCODE_GET_CMD(op2) && \
+ PDS_FWCTL_RPC_OPCODE_GET_VER(op1) <= PDS_FWCTL_RPC_OPCODE_GET_VER(op2))
+
+/**
+ * struct pds_fwctl_query_cmd - Firmware control query command structure
+ * @opcode: Operation code for the command
+ * @entity: Entity type to query (enum pds_fwctl_query_entity)
+ * @version: Version of the query data structure supported by the driver
+ * @rsvd: Reserved
+ * @query_data_buf_len: Length of the query data buffer
+ * @query_data_buf_pa: Physical address of the query data buffer
+ * @ep: Endpoint identifier to query (when entity is PDS_FWCTL_RPC_ENDPOINT)
+ * @op: Operation identifier to query (when entity is PDS_FWCTL_RPC_OPERATION)
+ *
+ * This structure is used to send a query command to the firmware control
+ * interface. The structure is packed to ensure there is no padding between
+ * the fields.
+ */
+struct pds_fwctl_query_cmd {
+ u8 opcode;
+ u8 entity;
+ u8 version;
+ u8 rsvd;
+ __le32 query_data_buf_len;
+ __le64 query_data_buf_pa;
+ union {
+ __le32 ep;
+ __le32 op;
+ };
+} __packed;
+
+/**
+ * struct pds_fwctl_query_comp - Firmware control query completion structure
+ * @status: Status of the query command
+ * @rsvd: Reserved
+ * @comp_index: Completion index in little-endian format
+ * @version: Version of the query data structure returned by firmware. This
+ * should be less than or equal to the version supported by the driver
+ * @rsvd2: Reserved
+ * @color: Color bit indicating the state of the completion
+ */
+struct pds_fwctl_query_comp {
+ u8 status;
+ u8 rsvd;
+ __le16 comp_index;
+ u8 version;
+ u8 rsvd2[2];
+ u8 color;
+} __packed;
+
+/**
+ * struct pds_fwctl_query_data_endpoint - query data for entity PDS_FWCTL_RPC_ROOT
+ * @id: The identifier for the data endpoint
+ */
+struct pds_fwctl_query_data_endpoint {
+ __le32 id;
+} __packed;
+
+/**
+ * struct pds_fwctl_query_data_operation - query data for entity PDS_FWCTL_RPC_ENDPOINT
+ * @id: Operation identifier
+ * @scope: Scope of the operation (enum fwctl_rpc_scope)
+ * @rsvd: Reserved
+ */
+struct pds_fwctl_query_data_operation {
+ __le32 id;
+ u8 scope;
+ u8 rsvd[3];
+} __packed;
+
+/**
+ * struct pds_fwctl_query_data - query data structure
+ * @version: Version of the query data structure
+ * @rsvd: Reserved
+ * @num_entries: Number of entries in the union
+ * @entries: Array of query data entries, depending on the entity type
+ */
+struct pds_fwctl_query_data {
+ u8 version;
+ u8 rsvd[3];
+ __le32 num_entries;
+ u8 entries[] __counted_by_le(num_entries);
+} __packed;
+
+/**
+ * struct pds_fwctl_rpc_cmd - Firmware control RPC command
+ * @opcode: opcode PDS_FWCTL_CMD_RPC
+ * @rsvd: Reserved
+ * @flags: Indicates indirect request and/or response handling
+ * @ep: Endpoint identifier
+ * @op: Operation identifier
+ * @inline_req0: Buffer for inline request
+ * @inline_req1: Buffer for inline request
+ * @req_pa: Physical address of request data
+ * @req_sz: Size of the request
+ * @req_sg_elems: Number of request SGs
+ * @req_rsvd: Reserved
+ * @inline_req2: Buffer for inline request
+ * @resp_pa: Physical address of response data
+ * @resp_sz: Size of the response
+ * @resp_sg_elems: Number of response SGs
+ * @resp_rsvd: Reserved
+ */
+struct pds_fwctl_rpc_cmd {
+ u8 opcode;
+ u8 rsvd;
+ __le16 flags;
+#define PDS_FWCTL_RPC_IND_REQ 0x1
+#define PDS_FWCTL_RPC_IND_RESP 0x2
+ __le32 ep;
+ __le32 op;
+ u8 inline_req0[16];
+ union {
+ u8 inline_req1[16];
+ struct {
+ __le64 req_pa;
+ __le32 req_sz;
+ u8 req_sg_elems;
+ u8 req_rsvd[3];
+ };
+ };
+ union {
+ u8 inline_req2[16];
+ struct {
+ __le64 resp_pa;
+ __le32 resp_sz;
+ u8 resp_sg_elems;
+ u8 resp_rsvd[3];
+ };
+ };
+} __packed;
+
+/**
+ * struct pds_sg_elem - Transmit scatter-gather (SG) descriptor element
+ * @addr: DMA address of SG element data buffer
+ * @len: Length of SG element data buffer, in bytes
+ * @rsvd: Reserved
+ */
+struct pds_sg_elem {
+ __le64 addr;
+ __le32 len;
+ u8 rsvd[4];
+} __packed;
+
+/**
+ * struct pds_fwctl_rpc_comp - Completion of a firmware control RPC
+ * @status: Status of the command
+ * @rsvd: Reserved
+ * @comp_index: Completion index of the command
+ * @err: Error code, if any, from the RPC
+ * @resp_sz: Size of the response
+ * @rsvd2: Reserved
+ * @color: Color bit indicating the state of the completion
+ */
+struct pds_fwctl_rpc_comp {
+ u8 status;
+ u8 rsvd;
+ __le16 comp_index;
+ __le32 err;
+ __le32 resp_sz;
+ u8 rsvd2[3];
+ u8 color;
+} __packed;
+
union pds_core_adminq_cmd {
u8 opcode;
u8 bytes[64];
@@ -1297,6 +1478,8 @@ union pds_core_adminq_cmd {
struct pds_fwctl_cmd fwctl;
struct pds_fwctl_ident_cmd fwctl_ident;
+ struct pds_fwctl_rpc_cmd fwctl_rpc;
+ struct pds_fwctl_query_cmd fwctl_query;
};
union pds_core_adminq_comp {
@@ -1326,6 +1509,8 @@ union pds_core_adminq_comp {
struct pds_lm_dirty_status_comp lm_dirty_status;
struct pds_fwctl_comp fwctl;
+ struct pds_fwctl_rpc_comp fwctl_rpc;
+ struct pds_fwctl_query_comp fwctl_query;
};
#ifndef __CHECKER__
diff --git a/include/uapi/fwctl/pds.h b/include/uapi/fwctl/pds.h
index 558e030b7583..68e68eeab3ae 100644
--- a/include/uapi/fwctl/pds.h
+++ b/include/uapi/fwctl/pds.h
@@ -23,4 +23,20 @@ enum pds_fwctl_capabilities {
PDS_FWCTL_QUERY_CAP = 0,
PDS_FWCTL_SEND_CAP,
};
+
+struct fwctl_rpc_pds {
+ struct {
+ __u32 op;
+ __u32 ep;
+ __u32 rsvd;
+ __u32 len;
+ __aligned_u64 payload;
+ } in;
+ struct {
+ __u32 retval;
+ __u32 rsvd[2];
+ __u32 len;
+ __aligned_u64 payload;
+ } out;
+};
#endif /* _UAPI_FWCTL_PDS_H_ */
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v3 6/6] pds_fwctl: add Documentation entries
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
` (4 preceding siblings ...)
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
@ 2025-03-07 18:53 ` Shannon Nelson
2025-03-10 20:27 ` Dave Jiang
5 siblings, 1 reply; 21+ messages in thread
From: Shannon Nelson @ 2025-03-07 18:53 UTC (permalink / raw)
To: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl, linux-rdma,
netdev, saeedm
Cc: brett.creeley, Shannon Nelson
Add pds_fwctl to the driver and fwctl documentation pages.
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
---
Documentation/userspace-api/fwctl/fwctl.rst | 1 +
Documentation/userspace-api/fwctl/index.rst | 1 +
.../userspace-api/fwctl/pds_fwctl.rst | 40 +++++++++++++++++++
3 files changed, 42 insertions(+)
create mode 100644 Documentation/userspace-api/fwctl/pds_fwctl.rst
diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
index 04ad78a7cd48..fdcfe418a83f 100644
--- a/Documentation/userspace-api/fwctl/fwctl.rst
+++ b/Documentation/userspace-api/fwctl/fwctl.rst
@@ -150,6 +150,7 @@ fwctl User API
.. kernel-doc:: include/uapi/fwctl/fwctl.h
.. kernel-doc:: include/uapi/fwctl/mlx5.h
+.. kernel-doc:: include/uapi/fwctl/pds.h
sysfs Class
-----------
diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
index d9d40a468a31..316ac456ad3b 100644
--- a/Documentation/userspace-api/fwctl/index.rst
+++ b/Documentation/userspace-api/fwctl/index.rst
@@ -11,3 +11,4 @@ to securely construct and execute RPCs inside device firmware.
fwctl
fwctl-cxl
+ pds_fwctl
diff --git a/Documentation/userspace-api/fwctl/pds_fwctl.rst b/Documentation/userspace-api/fwctl/pds_fwctl.rst
new file mode 100644
index 000000000000..e8a63d4215d0
--- /dev/null
+++ b/Documentation/userspace-api/fwctl/pds_fwctl.rst
@@ -0,0 +1,40 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+================
+fwctl pds driver
+================
+
+:Author: Shannon Nelson
+
+Overview
+========
+
+The PDS Core device makes an fwctl service available through an
+auxiliary_device named pds_core.fwctl.N. The pds_fwctl driver binds to
+this device and registers itself with the fwctl subsystem. The resulting
+userspace interface is used by an application that is a part of the
+AMD/Pensando software package for the Distributed Service Card (DSC).
+
+The pds_fwctl driver has little knowledge of the firmware's internals,
+only knows how to send commands through pds_core's message queue to the
+firmware for fwctl requests. The set of fwctl operations available
+depends on the firmware in the DSC, and the userspace application
+version must match the firmware so that they can talk to each other.
+
+When a connection is created the pds_fwctl driver requests from the
+firmware a list of firmware object endpoints, and for each endpoint the
+driver requests a list of operations for the endpoint. Each operation
+description includes a minimum scope level that the pds_fwctl driver can
+use for filtering privilege levels.
+
+pds_fwctl User API
+==================
+
+.. kernel-doc:: include/uapi/fwctl/pds.h
+
+Each RPC request includes the target endpoint and the operation id, and in
+and out buffer lengths and pointers. The driver verifies the existence
+of the requested endpoint and operations, then checks the current scope
+against the required scope of the operation. The request is then put
+together with the request data and sent through pds_core's message queue
+to the firmware, and the results are returned to the caller.
--
2.17.1
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/6] pds_fwctl: add rpc and query support
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
@ 2025-03-07 23:38 ` Jason Gunthorpe
2025-03-17 22:24 ` Nelson, Shannon
2025-03-12 18:29 ` Jonathan Cameron
1 sibling, 1 reply; 21+ messages in thread
From: Jason Gunthorpe @ 2025-03-07 23:38 UTC (permalink / raw)
To: Shannon Nelson
Cc: andrew.gospodarek, aron.silverton, dan.j.williams, daniel.vetter,
dave.jiang, dsahern, gregkh, hch, itayavr, jiri, Jonathan.Cameron,
kuba, lbloch, leonro, linux-cxl, linux-rdma, netdev, saeedm,
brett.creeley
On Fri, Mar 07, 2025 at 10:53:28AM -0800, Shannon Nelson wrote:
> +#define PDS_FWCTL_RPC_OPCODE_GET_CMD(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_CMD_MASK, op)
> +#define PDS_FWCTL_RPC_OPCODE_GET_VER(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_VER_MASK, op)
../drivers/fwctl/pds/main.c:302:7: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
302 | if (PDS_FWCTL_RPC_OPCODE_CMP(rpc->in.op, op_entry[i].id)) {
Add:
#include <linux/bitfield.h>
Jason
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
@ 2025-03-10 16:58 ` Dave Jiang
0 siblings, 0 replies; 21+ messages in thread
From: Dave Jiang @ 2025-03-10 16:58 UTC (permalink / raw)
To: Shannon Nelson, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/7/25 11:53 AM, Shannon Nelson wrote:
> Since there really is no useful return, advertising a return value
> is rather misleading. Make pdsc_auxbus_dev_del() a void function.
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/net/ethernet/amd/pds_core/auxbus.c | 7 +------
> drivers/net/ethernet/amd/pds_core/core.h | 2 +-
> drivers/net/ethernet/amd/pds_core/devlink.c | 6 ++++--
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 2babea110991..78fba368e797 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -175,13 +175,9 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
> return padev;
> }
>
> -int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> {
> struct pds_auxiliary_dev *padev;
> - int err = 0;
> -
> - if (!cf)
> - return -ENODEV;
>
> mutex_lock(&pf->config_lock);
>
> @@ -195,7 +191,6 @@ int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> pf->vfs[cf->vf_id].padev = NULL;
>
> mutex_unlock(&pf->config_lock);
> - return err;
> }
>
> int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
> index 14522d6d5f86..631a59cfdd7e 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.h
> +++ b/drivers/net/ethernet/amd/pds_core/core.h
> @@ -304,7 +304,7 @@ int pdsc_register_notify(struct notifier_block *nb);
> void pdsc_unregister_notify(struct notifier_block *nb);
> void pdsc_notify(unsigned long event, void *data);
> int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
> -int pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
>
> void pdsc_process_adminq(struct pdsc_qcq *qcq);
> void pdsc_work_thread(struct work_struct *work);
> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
> index 44971e71991f..4e2b92ddef6f 100644
> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
> @@ -56,8 +56,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
> for (vf_id = 0; vf_id < pdsc->num_vfs; vf_id++) {
> struct pdsc *vf = pdsc->vfs[vf_id].vf;
>
> - err = ctx->val.vbool ? pdsc_auxbus_dev_add(vf, pdsc) :
> - pdsc_auxbus_dev_del(vf, pdsc);
> + if (ctx->val.vbool)
> + err = pdsc_auxbus_dev_add(vf, pdsc);
> + else
> + pdsc_auxbus_dev_del(vf, pdsc);
> }
>
> return err;
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/6] pds_core: specify auxiliary_device to be created
2025-03-07 18:53 ` [PATCH v3 2/6] pds_core: specify auxiliary_device to be created Shannon Nelson
@ 2025-03-10 17:26 ` Dave Jiang
2025-03-17 22:30 ` Nelson, Shannon
0 siblings, 1 reply; 21+ messages in thread
From: Dave Jiang @ 2025-03-10 17:26 UTC (permalink / raw)
To: Shannon Nelson, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/7/25 11:53 AM, Shannon Nelson wrote:
> In preparation for adding a new auxiliary_device for the
> PF, make the vif type an argument to pdsc_auxbus_dev_add().
> We also now pass in the address to where we'll keep the new
> padev pointer so that the caller can specify where to save it
> but we can still change it under the mutex and keep the mutex
> usage within the function.
Please consider changing the commit log to use imperative language and avoid using pronouns such as "we".
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes
https://kernelnewbies.org/PatchPhilosophy#:~:text=Please%20read%20that%20whole%20section,it%20into%20the%20git%20history.
Maybe something like:
Pass in the address of the padev pointer so the caller can specify where to save it. ...
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
> drivers/net/ethernet/amd/pds_core/auxbus.c | 37 ++++++++++-----------
> drivers/net/ethernet/amd/pds_core/core.h | 7 ++--
> drivers/net/ethernet/amd/pds_core/devlink.c | 5 +--
> drivers/net/ethernet/amd/pds_core/main.c | 11 +++---
> 4 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
> index 78fba368e797..563de9e7ce0a 100644
> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
> @@ -175,29 +175,32 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
> return padev;
> }
>
> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
> + struct pds_auxiliary_dev **pd_ptr)
> {
> struct pds_auxiliary_dev *padev;
>
> + if (!*pd_ptr)
> + return;
> +
> mutex_lock(&pf->config_lock);
>
> - padev = pf->vfs[cf->vf_id].padev;
> - if (padev) {
> - pds_client_unregister(pf, padev->client_id);
> - auxiliary_device_delete(&padev->aux_dev);
> - auxiliary_device_uninit(&padev->aux_dev);
> - padev->client_id = 0;
> - }
> - pf->vfs[cf->vf_id].padev = NULL;
> + padev = *pd_ptr;
> + pds_client_unregister(pf, padev->client_id);
> + auxiliary_device_delete(&padev->aux_dev);
> + auxiliary_device_uninit(&padev->aux_dev);
> + padev->client_id = 0;
> + *pd_ptr = NULL;
>
> mutex_unlock(&pf->config_lock);
> }
>
> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
> + enum pds_core_vif_types vt,
> + struct pds_auxiliary_dev **pd_ptr)
> {
> struct pds_auxiliary_dev *padev;
> char devname[PDS_DEVNAME_LEN];
> - enum pds_core_vif_types vt;
> unsigned long mask;
> u16 vt_support;
> int client_id;
> @@ -206,6 +209,9 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
> if (!cf)
> return -ENODEV;
>
> + if (vt >= PDS_DEV_TYPE_MAX)
> + return -EINVAL;
> +
> mutex_lock(&pf->config_lock);
>
> mask = BIT_ULL(PDSC_S_FW_DEAD) |
> @@ -217,17 +223,10 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
> goto out_unlock;
> }
>
> - /* We only support vDPA so far, so it is the only one to
> - * be verified that it is available in the Core device and
> - * enabled in the devlink param. In the future this might
> - * become a loop for several VIF types.
> - */
> -
> /* 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.
> */
> - vt = PDS_DEV_TYPE_VDPA;
> vt_support = !!le16_to_cpu(pf->dev_ident.vif_types[vt]);
> if (!(vt_support &&
> pf->viftype_status[vt].supported &&
> @@ -253,7 +252,7 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
> err = PTR_ERR(padev);
> goto out_unlock;
> }
> - pf->vfs[cf->vf_id].padev = padev;
> + *pd_ptr = padev;
>
> out_unlock:
> mutex_unlock(&pf->config_lock);
> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
> index 631a59cfdd7e..f075e68c64db 100644
> --- a/drivers/net/ethernet/amd/pds_core/core.h
> +++ b/drivers/net/ethernet/amd/pds_core/core.h
> @@ -303,8 +303,11 @@ void pdsc_health_thread(struct work_struct *work);
> int pdsc_register_notify(struct notifier_block *nb);
> void pdsc_unregister_notify(struct notifier_block *nb);
> void pdsc_notify(unsigned long event, void *data);
> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
> + enum pds_core_vif_types vt,
> + struct pds_auxiliary_dev **pd_ptr);
> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
> + struct pds_auxiliary_dev **pd_ptr);
>
> void pdsc_process_adminq(struct pdsc_qcq *qcq);
> void pdsc_work_thread(struct work_struct *work);
> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
> index 4e2b92ddef6f..c5c787df61a4 100644
> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
> @@ -57,9 +57,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
> struct pdsc *vf = pdsc->vfs[vf_id].vf;
>
> if (ctx->val.vbool)
> - err = pdsc_auxbus_dev_add(vf, pdsc);
> + err = pdsc_auxbus_dev_add(vf, pdsc, vt_entry->vif_id,
> + &pdsc->vfs[vf_id].padev);
> else
> - pdsc_auxbus_dev_del(vf, pdsc);
> + pdsc_auxbus_dev_del(vf, pdsc, &pdsc->vfs[vf_id].padev);
> }
>
> return err;
> diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
> index 660268ff9562..a3a68889137b 100644
> --- a/drivers/net/ethernet/amd/pds_core/main.c
> +++ b/drivers/net/ethernet/amd/pds_core/main.c
> @@ -190,7 +190,8 @@ static int pdsc_init_vf(struct pdsc *vf)
> devl_unlock(dl);
>
> pf->vfs[vf->vf_id].vf = vf;
> - err = pdsc_auxbus_dev_add(vf, pf);
> + err = pdsc_auxbus_dev_add(vf, pf, PDS_DEV_TYPE_VDPA,
> + &pf->vfs[vf->vf_id].padev);
> if (err) {
> devl_lock(dl);
> devl_unregister(dl);
> @@ -417,7 +418,7 @@ static void pdsc_remove(struct pci_dev *pdev)
>
> pf = pdsc_get_pf_struct(pdsc->pdev);
> if (!IS_ERR(pf)) {
> - pdsc_auxbus_dev_del(pdsc, pf);
> + pdsc_auxbus_dev_del(pdsc, pf, &pf->vfs[pdsc->vf_id].padev);
> pf->vfs[pdsc->vf_id].vf = NULL;
> }
> } else {
> @@ -482,7 +483,8 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
>
> pf = pdsc_get_pf_struct(pdsc->pdev);
> if (!IS_ERR(pf))
> - pdsc_auxbus_dev_del(pdsc, pf);
> + pdsc_auxbus_dev_del(pdsc, pf,
> + &pf->vfs[pdsc->vf_id].padev);
> }
>
> pdsc_unmap_bars(pdsc);
> @@ -527,7 +529,8 @@ static void pdsc_reset_done(struct pci_dev *pdev)
>
> pf = pdsc_get_pf_struct(pdsc->pdev);
> if (!IS_ERR(pf))
> - pdsc_auxbus_dev_add(pdsc, pf);
> + pdsc_auxbus_dev_add(pdsc, pf, PDS_DEV_TYPE_VDPA,
> + &pf->vfs[pdsc->vf_id].padev);
> }
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
@ 2025-03-10 17:33 ` Dave Jiang
2025-03-17 22:37 ` Nelson, Shannon
2025-03-12 18:22 ` Jonathan Cameron
1 sibling, 1 reply; 21+ messages in thread
From: Dave Jiang @ 2025-03-10 17:33 UTC (permalink / raw)
To: Shannon Nelson, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
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@nvidia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.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
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/6] pds_fwctl: initial driver framework
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
@ 2025-03-10 18:28 ` Dave Jiang
2025-03-17 22:40 ` Nelson, Shannon
2025-03-12 18:25 ` Jonathan Cameron
1 sibling, 1 reply; 21+ messages in thread
From: Dave Jiang @ 2025-03-10 18:28 UTC (permalink / raw)
To: Shannon Nelson, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/7/25 11:53 AM, Shannon Nelson wrote:
> Initial files for adding a new fwctl driver for the AMD/Pensando PDS
> devices. This sets up a simple auxiliary_bus driver that registers
> with fwctl subsystem. It expects that a pds_core device has set up
> the auxiliary_device pds_core.fwctl
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
minor comment below.
> ---
> MAINTAINERS | 7 ++
> drivers/fwctl/Kconfig | 10 ++
> drivers/fwctl/Makefile | 1 +
> drivers/fwctl/pds/Makefile | 4 +
> drivers/fwctl/pds/main.c | 169 +++++++++++++++++++++++++++++++++
> include/linux/pds/pds_adminq.h | 83 ++++++++++++++++
> include/uapi/fwctl/fwctl.h | 1 +
> include/uapi/fwctl/pds.h | 26 +++++
> 8 files changed, 301 insertions(+)
> create mode 100644 drivers/fwctl/pds/Makefile
> create mode 100644 drivers/fwctl/pds/main.c
> create mode 100644 include/uapi/fwctl/pds.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3381e41dcf37..c63fd76a3684 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9576,6 +9576,13 @@ L: linux-kernel@vger.kernel.org
> S: Maintained
> F: drivers/fwctl/mlx5/
>
> +FWCTL PDS DRIVER
> +M: Brett Creeley <brett.creeley@amd.com>
> +R: Shannon Nelson <shannon.nelson@amd.com>
> +L: linux-kernel@vger.kernel.org
> +S: Maintained
> +F: drivers/fwctl/pds/
> +
> GALAXYCORE GC0308 CAMERA SENSOR DRIVER
> M: Sebastian Reichel <sre@kernel.org>
> L: linux-media@vger.kernel.org
> diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> index f802cf5d4951..b5583b12a011 100644
> --- a/drivers/fwctl/Kconfig
> +++ b/drivers/fwctl/Kconfig
> @@ -19,5 +19,15 @@ config FWCTL_MLX5
> This will allow configuration and debug tools to work out of the box on
> mainstream kernel.
>
> + If you don't know what to do here, say N.
> +
> +config FWCTL_PDS
> + tristate "AMD/Pensando pds fwctl driver"
> + depends on PDS_CORE
> + help
> + The pds_fwctl driver provides an fwctl interface for a user process
> + to access the debug and configuration information of the AMD/Pensando
> + DSC hardware family.
> +
> If you don't know what to do here, say N.
> endif
> diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
> index 1c535f694d7f..c093b5f661d6 100644
> --- a/drivers/fwctl/Makefile
> +++ b/drivers/fwctl/Makefile
> @@ -1,5 +1,6 @@
> # SPDX-License-Identifier: GPL-2.0
> obj-$(CONFIG_FWCTL) += fwctl.o
> obj-$(CONFIG_FWCTL_MLX5) += mlx5/
> +obj-$(CONFIG_FWCTL_PDS) += pds/
>
> fwctl-y += main.o
> diff --git a/drivers/fwctl/pds/Makefile b/drivers/fwctl/pds/Makefile
> new file mode 100644
> index 000000000000..cc2317c07be1
> --- /dev/null
> +++ b/drivers/fwctl/pds/Makefile
> @@ -0,0 +1,4 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_FWCTL_PDS) += pds_fwctl.o
> +
> +pds_fwctl-y += main.o
> diff --git a/drivers/fwctl/pds/main.c b/drivers/fwctl/pds/main.c
> new file mode 100644
> index 000000000000..27942315a602
> --- /dev/null
> +++ b/drivers/fwctl/pds/main.c
> @@ -0,0 +1,169 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) Advanced Micro Devices, Inc */
> +
> +#include <linux/module.h>
> +#include <linux/auxiliary_bus.h>
> +#include <linux/pci.h>
> +#include <linux/vmalloc.h>
> +
> +#include <uapi/fwctl/fwctl.h>
> +#include <uapi/fwctl/pds.h>
> +#include <linux/fwctl.h>
> +
> +#include <linux/pds/pds_common.h>
> +#include <linux/pds/pds_core_if.h>
> +#include <linux/pds/pds_adminq.h>
> +#include <linux/pds/pds_auxbus.h>
> +
> +struct pdsfc_uctx {
> + struct fwctl_uctx uctx;
> + u32 uctx_caps;
> +};
> +
> +struct pdsfc_dev {
> + struct fwctl_device fwctl;
> + struct pds_auxiliary_dev *padev;
> + u32 caps;
> + struct pds_fwctl_ident ident;
> +};
> +
> +static int pdsfc_open_uctx(struct fwctl_uctx *uctx)
> +{
> + struct pdsfc_dev *pdsfc = container_of(uctx->fwctl, struct pdsfc_dev, fwctl);
> + struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
> +
> + pdsfc_uctx->uctx_caps = pdsfc->caps;
> +
> + return 0;
> +}
> +
> +static void pdsfc_close_uctx(struct fwctl_uctx *uctx)
> +{
> +}
> +
> +static void *pdsfc_info(struct fwctl_uctx *uctx, size_t *length)
> +{
> + struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
> + struct fwctl_info_pds *info;
> +
> + info = kzalloc(sizeof(*info), GFP_KERNEL);
> + if (!info)
> + return ERR_PTR(-ENOMEM);
> +
> + info->uctx_caps = pdsfc_uctx->uctx_caps;
> +
> + return info;
> +}
> +
> +static int pdsfc_identify(struct pdsfc_dev *pdsfc)
> +{
> + struct device *dev = &pdsfc->fwctl.dev;
> + union pds_core_adminq_comp comp = {0};
> + union pds_core_adminq_cmd cmd;
> + struct pds_fwctl_ident *ident;
> + dma_addr_t ident_pa;
> + int err;
> +
> + ident = dma_alloc_coherent(dev->parent, sizeof(*ident), &ident_pa, GFP_KERNEL);
> + err = dma_mapping_error(dev->parent, ident_pa);
> + if (err) {
> + dev_err(dev, "Failed to map ident buffer\n");
> + return err;
> + }
> +
> + cmd = (union pds_core_adminq_cmd) {
> + .fwctl_ident = {
> + .opcode = PDS_FWCTL_CMD_IDENT,
> + .version = 0,
> + .len = cpu_to_le32(sizeof(*ident)),
> + .ident_pa = cpu_to_le64(ident_pa),
> + }
> + };
> +
> + err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
> + if (err)
> + dev_err(dev, "Failed to send adminq cmd opcode: %u err: %d\n",
> + cmd.fwctl_ident.opcode, err);
> + else
> + pdsfc->ident = *ident;
> +
> + dma_free_coherent(dev->parent, sizeof(*ident), ident, ident_pa);
> +
> + return err;
> +}
> +
> +static void *pdsfc_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
> + void *in, size_t in_len, size_t *out_len)
> +{
> + return NULL;
> +}
> +
> +static const struct fwctl_ops pdsfc_ops = {
> + .device_type = FWCTL_DEVICE_TYPE_PDS,
> + .uctx_size = sizeof(struct pdsfc_uctx),
> + .open_uctx = pdsfc_open_uctx,
> + .close_uctx = pdsfc_close_uctx,
> + .info = pdsfc_info,
> + .fw_rpc = pdsfc_fw_rpc,
> +};
> +
> +static int pdsfc_probe(struct auxiliary_device *adev,
> + const struct auxiliary_device_id *id)
> +{
> + struct pds_auxiliary_dev *padev =
> + container_of(adev, struct pds_auxiliary_dev, aux_dev);
> + struct device *dev = &adev->dev;
> + struct pdsfc_dev *pdsfc;
> + int err;
> +
> + pdsfc = fwctl_alloc_device(&padev->vf_pdev->dev, &pdsfc_ops,
> + struct pdsfc_dev, fwctl);
> + if (!pdsfc)
> + return dev_err_probe(dev, -ENOMEM, "Failed to allocate fwctl device struct\n");
> + pdsfc->padev = padev;
> +
> + err = pdsfc_identify(pdsfc);
> + if (err) {
> + fwctl_put(&pdsfc->fwctl);
> + return dev_err_probe(dev, err, "Failed to identify device\n");
> + }
> +
> + err = fwctl_register(&pdsfc->fwctl);
> + if (err) {
> + fwctl_put(&pdsfc->fwctl);
> + return dev_err_probe(dev, err, "Failed to register device\n");
> + }
> +
> + auxiliary_set_drvdata(adev, pdsfc);
> +
> + return 0;
> +}
> +
> +static void pdsfc_remove(struct auxiliary_device *adev)
> +{
> + struct pdsfc_dev *pdsfc = auxiliary_get_drvdata(adev);
> +
> + fwctl_unregister(&pdsfc->fwctl);
> + fwctl_put(&pdsfc->fwctl);
> +}
> +
> +static const struct auxiliary_device_id pdsfc_id_table[] = {
> + {.name = PDS_CORE_DRV_NAME "." PDS_DEV_TYPE_FWCTL_STR },
> + {}
> +};
> +MODULE_DEVICE_TABLE(auxiliary, pdsfc_id_table);
> +
> +static struct auxiliary_driver pdsfc_driver = {
> + .name = "pds_fwctl",
> + .probe = pdsfc_probe,
> + .remove = pdsfc_remove,
> + .id_table = pdsfc_id_table,
> +};
> +
> +module_auxiliary_driver(pdsfc_driver);
> +
> +MODULE_IMPORT_NS("FWCTL");
> +MODULE_DESCRIPTION("pds fwctl driver");
> +MODULE_AUTHOR("Shannon Nelson <shannon.nelson@amd.com>");
> +MODULE_AUTHOR("Brett Creeley <brett.creeley@amd.com>");
> +MODULE_LICENSE("GPL");
> diff --git a/include/linux/pds/pds_adminq.h b/include/linux/pds/pds_adminq.h
> index 4b4e9a98b37b..22c6d77b3dcb 100644
> --- a/include/linux/pds/pds_adminq.h
> +++ b/include/linux/pds/pds_adminq.h
> @@ -1179,6 +1179,84 @@ struct pds_lm_host_vf_status_cmd {
> u8 status;
> };
>
> +enum pds_fwctl_cmd_opcode {
> + PDS_FWCTL_CMD_IDENT = 70,
> +};
> +
> +/**
> + * struct pds_fwctl_cmd - Firmware control command structure
> + * @opcode: Opcode
> + * @rsvd: Reserved
> + * @ep: Endpoint identifier
> + * @op: Operation identifier
> + */
> +struct pds_fwctl_cmd {
> + u8 opcode;
> + u8 rsvd[3];
> + __le32 ep;
> + __le32 op;
> +} __packed;
> +
> +/**
> + * struct pds_fwctl_comp - Firmware control completion structure
> + * @status: Status of the firmware control operation
> + * @rsvd: Reserved
> + * @comp_index: Completion index in little-endian format
> + * @rsvd2: Reserved
> + * @color: Color bit indicating the state of the completion
> + */
> +struct pds_fwctl_comp {
> + u8 status;
> + u8 rsvd;
> + __le16 comp_index;
> + u8 rsvd2[11];
> + u8 color;
> +} __packed;
> +
> +/**
> + * struct pds_fwctl_ident_cmd - Firmware control identification command structure
> + * @opcode: Operation code for the command
> + * @rsvd: Reserved
> + * @version: Interface version
> + * @rsvd2: Reserved
> + * @len: Length of the identification data
> + * @ident_pa: Physical address of the identification data
> + */
> +struct pds_fwctl_ident_cmd {
> + u8 opcode;
> + u8 rsvd;
> + u8 version;
> + u8 rsvd2;
> + __le32 len;
> + __le64 ident_pa;
> +} __packed;
> +
> +/* future feature bits here
> + * enum pds_fwctl_features {
> + * };
> + * (compilers don't like empty enums)
> + */
> +
> +/**
> + * struct pds_fwctl_ident - Firmware control identification structure
> + * @features: Supported features (enum pds_fwctl_features)
> + * @version: Interface version
> + * @rsvd: Reserved
> + * @max_req_sz: Maximum request size
> + * @max_resp_sz: Maximum response size
> + * @max_req_sg_elems: Maximum number of request SGs
> + * @max_resp_sg_elems: Maximum number of response SGs
> + */
> +struct pds_fwctl_ident {
> + __le64 features;
> + u8 version;
> + u8 rsvd[3];
> + __le32 max_req_sz;
> + __le32 max_resp_sz;
> + u8 max_req_sg_elems;
> + u8 max_resp_sg_elems;
> +} __packed;
> +
> union pds_core_adminq_cmd {
> u8 opcode;
> u8 bytes[64];
> @@ -1216,6 +1294,9 @@ union pds_core_adminq_cmd {
> struct pds_lm_dirty_enable_cmd lm_dirty_enable;
> struct pds_lm_dirty_disable_cmd lm_dirty_disable;
> struct pds_lm_dirty_seq_ack_cmd lm_dirty_seq_ack;
> +
> + struct pds_fwctl_cmd fwctl;
> + struct pds_fwctl_ident_cmd fwctl_ident;
> };
>
> union pds_core_adminq_comp {
> @@ -1243,6 +1324,8 @@ union pds_core_adminq_comp {
>
> struct pds_lm_state_size_comp lm_state_size;
> struct pds_lm_dirty_status_comp lm_dirty_status;
> +
> + struct pds_fwctl_comp fwctl;
> };
>
> #ifndef __CHECKER__
> diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
> index c2d5abc5a726..716ac0eee42d 100644
> --- a/include/uapi/fwctl/fwctl.h
> +++ b/include/uapi/fwctl/fwctl.h
> @@ -44,6 +44,7 @@ enum fwctl_device_type {
> FWCTL_DEVICE_TYPE_ERROR = 0,
> FWCTL_DEVICE_TYPE_MLX5 = 1,
> FWCTL_DEVICE_TYPE_CXL = 2,
> + FWCTL_DEVICE_TYPE_PDS = 4,
> };
>
> /**
> diff --git a/include/uapi/fwctl/pds.h b/include/uapi/fwctl/pds.h
> new file mode 100644
> index 000000000000..558e030b7583
> --- /dev/null
> +++ b/include/uapi/fwctl/pds.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/* Copyright(c) Advanced Micro Devices, Inc */
> +
> +/*
> + * fwctl interface info for pds_fwctl
> + */
> +
> +#ifndef _UAPI_FWCTL_PDS_H_
> +#define _UAPI_FWCTL_PDS_H_
> +
> +#include <linux/types.h>
> +
> +/*
> + * struct fwctl_info_pds
> + *
> + * Return basic information about the FW interface available.
> + */
Please use proper kdoc formatting for the comment block.
> +struct fwctl_info_pds {
> + __u32 uctx_caps;
> +};
> +
> +enum pds_fwctl_capabilities {
> + PDS_FWCTL_QUERY_CAP = 0,
> + PDS_FWCTL_SEND_CAP,
> +};
> +#endif /* _UAPI_FWCTL_PDS_H_ */
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 6/6] pds_fwctl: add Documentation entries
2025-03-07 18:53 ` [PATCH v3 6/6] pds_fwctl: add Documentation entries Shannon Nelson
@ 2025-03-10 20:27 ` Dave Jiang
2025-03-17 22:41 ` Nelson, Shannon
0 siblings, 1 reply; 21+ messages in thread
From: Dave Jiang @ 2025-03-10 20:27 UTC (permalink / raw)
To: Shannon Nelson, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/7/25 11:53 AM, Shannon Nelson wrote:
> Add pds_fwctl to the driver and fwctl documentation pages.
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> ---
> Documentation/userspace-api/fwctl/fwctl.rst | 1 +
> Documentation/userspace-api/fwctl/index.rst | 1 +
> .../userspace-api/fwctl/pds_fwctl.rst | 40 +++++++++++++++++++
> 3 files changed, 42 insertions(+)
> create mode 100644 Documentation/userspace-api/fwctl/pds_fwctl.rst
>
> diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
> index 04ad78a7cd48..fdcfe418a83f 100644
> --- a/Documentation/userspace-api/fwctl/fwctl.rst
> +++ b/Documentation/userspace-api/fwctl/fwctl.rst
> @@ -150,6 +150,7 @@ fwctl User API
>
> .. kernel-doc:: include/uapi/fwctl/fwctl.h
> .. kernel-doc:: include/uapi/fwctl/mlx5.h
> +.. kernel-doc:: include/uapi/fwctl/pds.h
>
> sysfs Class
> -----------
> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
> index d9d40a468a31..316ac456ad3b 100644
> --- a/Documentation/userspace-api/fwctl/index.rst
> +++ b/Documentation/userspace-api/fwctl/index.rst
> @@ -11,3 +11,4 @@ to securely construct and execute RPCs inside device firmware.
>
> fwctl
> fwctl-cxl
> + pds_fwctl
> diff --git a/Documentation/userspace-api/fwctl/pds_fwctl.rst b/Documentation/userspace-api/fwctl/pds_fwctl.rst
> new file mode 100644
> index 000000000000..e8a63d4215d0
> --- /dev/null
> +++ b/Documentation/userspace-api/fwctl/pds_fwctl.rst
> @@ -0,0 +1,40 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +================
> +fwctl pds driver
> +================
> +
> +:Author: Shannon Nelson
> +
> +Overview
> +========
> +
> +The PDS Core device makes an fwctl service available through an
s/an fwctl/a fwctl/
> +auxiliary_device named pds_core.fwctl.N. The pds_fwctl driver binds to
> +this device and registers itself with the fwctl subsystem. The resulting
> +userspace interface is used by an application that is a part of the
> +AMD/Pensando software package for the Distributed Service Card (DSC).
> +
> +The pds_fwctl driver has little knowledge of the firmware's internals,
> +only knows how to send commands through pds_core's message queue to the
s/ , only/. It only/
> +firmware for fwctl requests. The set of fwctl operations available
> +depends on the firmware in the DSC, and the userspace application
> +version must match the firmware so that they can talk to each other.
> +
> +When a connection is created the pds_fwctl driver requests from the
> +firmware a list of firmware object endpoints, and for each endpoint the
> +driver requests a list of operations for the endpoint. Each operation
> +description includes a minimum scope level that the pds_fwctl driver can
> +use for filtering privilege levels.
Maybe a bit more details on the privilege levels?
> +
> +pds_fwctl User API
> +==================
> +
> +.. kernel-doc:: include/uapi/fwctl/pds.h
> +
> +Each RPC request includes the target endpoint and the operation id, and in
> +and out buffer lengths and pointers. The driver verifies the existence
> +of the requested endpoint and operations, then checks the current scope
s/then/and then it/
> +against the required scope of the operation. The request is then put
> +together with the request data and sent through pds_core's message queue
> +to the firmware, and the results are returned to the caller.
May be a good idea to touch on each of the ioctls being exported as well as maybe provide some sample user code on how to perform the RPCs. This documentation should serve as a guide to an application programmer on how to write the user application that accesses the fwctl char dev.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
2025-03-10 17:33 ` Dave Jiang
@ 2025-03-12 18:22 ` Jonathan Cameron
1 sibling, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-03-12 18:22 UTC (permalink / raw)
To: Shannon Nelson
Cc: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
kuba, lbloch, leonro, linux-cxl, linux-rdma, netdev, saeedm,
brett.creeley
On Fri, 7 Mar 2025 10:53:26 -0800
Shannon Nelson <shannon.nelson@amd.com> 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@nvidia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
FWIW LGTM
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/6] pds_fwctl: initial driver framework
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
2025-03-10 18:28 ` Dave Jiang
@ 2025-03-12 18:25 ` Jonathan Cameron
1 sibling, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-03-12 18:25 UTC (permalink / raw)
To: Shannon Nelson
Cc: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
kuba, lbloch, leonro, linux-cxl, linux-rdma, netdev, saeedm,
brett.creeley
On Fri, 7 Mar 2025 10:53:27 -0800
Shannon Nelson <shannon.nelson@amd.com> wrote:
> Initial files for adding a new fwctl driver for the AMD/Pensando PDS
> devices. This sets up a simple auxiliary_bus driver that registers
> with fwctl subsystem. It expects that a pds_core device has set up
> the auxiliary_device pds_core.fwctl
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/6] pds_fwctl: add rpc and query support
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
2025-03-07 23:38 ` Jason Gunthorpe
@ 2025-03-12 18:29 ` Jonathan Cameron
1 sibling, 0 replies; 21+ messages in thread
From: Jonathan Cameron @ 2025-03-12 18:29 UTC (permalink / raw)
To: Shannon Nelson
Cc: jgg, andrew.gospodarek, aron.silverton, dan.j.williams,
daniel.vetter, dave.jiang, dsahern, gregkh, hch, itayavr, jiri,
kuba, lbloch, leonro, linux-cxl, linux-rdma, netdev, saeedm,
brett.creeley
On Fri, 7 Mar 2025 10:53:28 -0800
Shannon Nelson <shannon.nelson@amd.com> wrote:
> From: Brett Creeley <brett.creeley@amd.com>
>
> The pds_fwctl driver doesn't know what RPC operations are available
> in the firmware, so also doesn't know what scope they might have. The
> userland utility supplies the firmware "endpoint" and "operation" id values
> and this driver queries the firmware for endpoints and their available
> operations. The operation descriptions include the scope information
> which the driver uses for scope testing.
>
> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
(obviously with bitfield.h!)
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 5/6] pds_fwctl: add rpc and query support
2025-03-07 23:38 ` Jason Gunthorpe
@ 2025-03-17 22:24 ` Nelson, Shannon
0 siblings, 0 replies; 21+ messages in thread
From: Nelson, Shannon @ 2025-03-17 22:24 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: andrew.gospodarek, aron.silverton, dan.j.williams, daniel.vetter,
dave.jiang, dsahern, gregkh, hch, itayavr, jiri, Jonathan.Cameron,
kuba, lbloch, leonro, linux-cxl, linux-rdma, netdev, saeedm,
brett.creeley
On 3/7/2025 3:38 PM, Jason Gunthorpe wrote:
>
> On Fri, Mar 07, 2025 at 10:53:28AM -0800, Shannon Nelson wrote:
>
>> +#define PDS_FWCTL_RPC_OPCODE_GET_CMD(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_CMD_MASK, op)
>> +#define PDS_FWCTL_RPC_OPCODE_GET_VER(op) FIELD_GET(PDS_FWCTL_RPC_OPCODE_VER_MASK, op)
>
> ../drivers/fwctl/pds/main.c:302:7: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
> 302 | if (PDS_FWCTL_RPC_OPCODE_CMP(rpc->in.op, op_entry[i].id)) {
>
> Add:
>
> #include <linux/bitfield.h>
>
> Jason
Odd that I didn't run into that... perhaps a .config difference.
Sure, I can add that.
sln
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 2/6] pds_core: specify auxiliary_device to be created
2025-03-10 17:26 ` Dave Jiang
@ 2025-03-17 22:30 ` Nelson, Shannon
0 siblings, 0 replies; 21+ messages in thread
From: Nelson, Shannon @ 2025-03-17 22:30 UTC (permalink / raw)
To: Dave Jiang, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/10/2025 10:26 AM, Dave Jiang wrote:
>
> On 3/7/25 11:53 AM, Shannon Nelson wrote:
>> In preparation for adding a new auxiliary_device for the
>> PF, make the vif type an argument to pdsc_auxbus_dev_add().
>
>> We also now pass in the address to where we'll keep the new
>> padev pointer so that the caller can specify where to save it
>> but we can still change it under the mutex and keep the mutex
>> usage within the function.
>
> Please consider changing the commit log to use imperative language and avoid using pronouns such as "we".
>
> https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html#describe-your-changes
> https://kernelnewbies.org/PatchPhilosophy#:~:text=Please%20read%20that%20whole%20section,it%20into%20the%20git%20history.
>
> Maybe something like:
> Pass in the address of the padev pointer so the caller can specify where to save it. ...
Done - thanks,
sln
>
>>
>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
>> ---
>> drivers/net/ethernet/amd/pds_core/auxbus.c | 37 ++++++++++-----------
>> drivers/net/ethernet/amd/pds_core/core.h | 7 ++--
>> drivers/net/ethernet/amd/pds_core/devlink.c | 5 +--
>> drivers/net/ethernet/amd/pds_core/main.c | 11 +++---
>> 4 files changed, 33 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/amd/pds_core/auxbus.c b/drivers/net/ethernet/amd/pds_core/auxbus.c
>> index 78fba368e797..563de9e7ce0a 100644
>> --- a/drivers/net/ethernet/amd/pds_core/auxbus.c
>> +++ b/drivers/net/ethernet/amd/pds_core/auxbus.c
>> @@ -175,29 +175,32 @@ static struct pds_auxiliary_dev *pdsc_auxbus_dev_register(struct pdsc *cf,
>> return padev;
>> }
>>
>> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf)
>> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
>> + struct pds_auxiliary_dev **pd_ptr)
>> {
>> struct pds_auxiliary_dev *padev;
>>
>> + if (!*pd_ptr)
>> + return;
>> +
>> mutex_lock(&pf->config_lock);
>>
>> - padev = pf->vfs[cf->vf_id].padev;
>> - if (padev) {
>> - pds_client_unregister(pf, padev->client_id);
>> - auxiliary_device_delete(&padev->aux_dev);
>> - auxiliary_device_uninit(&padev->aux_dev);
>> - padev->client_id = 0;
>> - }
>> - pf->vfs[cf->vf_id].padev = NULL;
>> + padev = *pd_ptr;
>> + pds_client_unregister(pf, padev->client_id);
>> + auxiliary_device_delete(&padev->aux_dev);
>> + auxiliary_device_uninit(&padev->aux_dev);
>> + padev->client_id = 0;
>> + *pd_ptr = NULL;
>>
>> mutex_unlock(&pf->config_lock);
>> }
>>
>> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
>> + enum pds_core_vif_types vt,
>> + struct pds_auxiliary_dev **pd_ptr)
>> {
>> struct pds_auxiliary_dev *padev;
>> char devname[PDS_DEVNAME_LEN];
>> - enum pds_core_vif_types vt;
>> unsigned long mask;
>> u16 vt_support;
>> int client_id;
>> @@ -206,6 +209,9 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> if (!cf)
>> return -ENODEV;
>>
>> + if (vt >= PDS_DEV_TYPE_MAX)
>> + return -EINVAL;
>> +
>> mutex_lock(&pf->config_lock);
>>
>> mask = BIT_ULL(PDSC_S_FW_DEAD) |
>> @@ -217,17 +223,10 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> goto out_unlock;
>> }
>>
>> - /* We only support vDPA so far, so it is the only one to
>> - * be verified that it is available in the Core device and
>> - * enabled in the devlink param. In the future this might
>> - * become a loop for several VIF types.
>> - */
>> -
>> /* 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.
>> */
>> - vt = PDS_DEV_TYPE_VDPA;
>> vt_support = !!le16_to_cpu(pf->dev_ident.vif_types[vt]);
>> if (!(vt_support &&
>> pf->viftype_status[vt].supported &&
>> @@ -253,7 +252,7 @@ int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf)
>> err = PTR_ERR(padev);
>> goto out_unlock;
>> }
>> - pf->vfs[cf->vf_id].padev = padev;
>> + *pd_ptr = padev;
>>
>> out_unlock:
>> mutex_unlock(&pf->config_lock);
>> diff --git a/drivers/net/ethernet/amd/pds_core/core.h b/drivers/net/ethernet/amd/pds_core/core.h
>> index 631a59cfdd7e..f075e68c64db 100644
>> --- a/drivers/net/ethernet/amd/pds_core/core.h
>> +++ b/drivers/net/ethernet/amd/pds_core/core.h
>> @@ -303,8 +303,11 @@ void pdsc_health_thread(struct work_struct *work);
>> int pdsc_register_notify(struct notifier_block *nb);
>> void pdsc_unregister_notify(struct notifier_block *nb);
>> void pdsc_notify(unsigned long event, void *data);
>> -int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf);
>> -void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf);
>> +int pdsc_auxbus_dev_add(struct pdsc *cf, struct pdsc *pf,
>> + enum pds_core_vif_types vt,
>> + struct pds_auxiliary_dev **pd_ptr);
>> +void pdsc_auxbus_dev_del(struct pdsc *cf, struct pdsc *pf,
>> + struct pds_auxiliary_dev **pd_ptr);
>>
>> void pdsc_process_adminq(struct pdsc_qcq *qcq);
>> void pdsc_work_thread(struct work_struct *work);
>> diff --git a/drivers/net/ethernet/amd/pds_core/devlink.c b/drivers/net/ethernet/amd/pds_core/devlink.c
>> index 4e2b92ddef6f..c5c787df61a4 100644
>> --- a/drivers/net/ethernet/amd/pds_core/devlink.c
>> +++ b/drivers/net/ethernet/amd/pds_core/devlink.c
>> @@ -57,9 +57,10 @@ int pdsc_dl_enable_set(struct devlink *dl, u32 id,
>> struct pdsc *vf = pdsc->vfs[vf_id].vf;
>>
>> if (ctx->val.vbool)
>> - err = pdsc_auxbus_dev_add(vf, pdsc);
>> + err = pdsc_auxbus_dev_add(vf, pdsc, vt_entry->vif_id,
>> + &pdsc->vfs[vf_id].padev);
>> else
>> - pdsc_auxbus_dev_del(vf, pdsc);
>> + pdsc_auxbus_dev_del(vf, pdsc, &pdsc->vfs[vf_id].padev);
>> }
>>
>> return err;
>> diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
>> index 660268ff9562..a3a68889137b 100644
>> --- a/drivers/net/ethernet/amd/pds_core/main.c
>> +++ b/drivers/net/ethernet/amd/pds_core/main.c
>> @@ -190,7 +190,8 @@ static int pdsc_init_vf(struct pdsc *vf)
>> devl_unlock(dl);
>>
>> pf->vfs[vf->vf_id].vf = vf;
>> - err = pdsc_auxbus_dev_add(vf, pf);
>> + err = pdsc_auxbus_dev_add(vf, pf, PDS_DEV_TYPE_VDPA,
>> + &pf->vfs[vf->vf_id].padev);
>> if (err) {
>> devl_lock(dl);
>> devl_unregister(dl);
>> @@ -417,7 +418,7 @@ static void pdsc_remove(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf)) {
>> - pdsc_auxbus_dev_del(pdsc, pf);
>> + pdsc_auxbus_dev_del(pdsc, pf, &pf->vfs[pdsc->vf_id].padev);
>> pf->vfs[pdsc->vf_id].vf = NULL;
>> }
>> } else {
>> @@ -482,7 +483,8 @@ static void pdsc_reset_prepare(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf))
>> - pdsc_auxbus_dev_del(pdsc, pf);
>> + pdsc_auxbus_dev_del(pdsc, pf,
>> + &pf->vfs[pdsc->vf_id].padev);
>> }
>>
>> pdsc_unmap_bars(pdsc);
>> @@ -527,7 +529,8 @@ static void pdsc_reset_done(struct pci_dev *pdev)
>>
>> pf = pdsc_get_pf_struct(pdsc->pdev);
>> if (!IS_ERR(pf))
>> - pdsc_auxbus_dev_add(pdsc, pf);
>> + pdsc_auxbus_dev_add(pdsc, pf, PDS_DEV_TYPE_VDPA,
>> + &pf->vfs[pdsc->vf_id].padev);
>> }
>> }
>>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device
2025-03-10 17:33 ` Dave Jiang
@ 2025-03-17 22:37 ` Nelson, Shannon
0 siblings, 0 replies; 21+ messages in thread
From: Nelson, Shannon @ 2025-03-17 22:37 UTC (permalink / raw)
To: Dave Jiang, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/10/2025 10:33 AM, Dave Jiang wrote:
>
> 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@nvidia.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.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/
Got it, thanks,
sln
>
> 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
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 4/6] pds_fwctl: initial driver framework
2025-03-10 18:28 ` Dave Jiang
@ 2025-03-17 22:40 ` Nelson, Shannon
0 siblings, 0 replies; 21+ messages in thread
From: Nelson, Shannon @ 2025-03-17 22:40 UTC (permalink / raw)
To: Dave Jiang, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/10/2025 11:28 AM, Dave Jiang wrote:
>
> On 3/7/25 11:53 AM, Shannon Nelson wrote:
>> Initial files for adding a new fwctl driver for the AMD/Pensando PDS
>> devices. This sets up a simple auxiliary_bus driver that registers
>> with fwctl subsystem. It expects that a pds_core device has set up
>> the auxiliary_device pds_core.fwctl
>>
>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>
> minor comment below.
>> ---
>> MAINTAINERS | 7 ++
>> drivers/fwctl/Kconfig | 10 ++
>> drivers/fwctl/Makefile | 1 +
>> drivers/fwctl/pds/Makefile | 4 +
>> drivers/fwctl/pds/main.c | 169 +++++++++++++++++++++++++++++++++
>> include/linux/pds/pds_adminq.h | 83 ++++++++++++++++
>> include/uapi/fwctl/fwctl.h | 1 +
>> include/uapi/fwctl/pds.h | 26 +++++
>> 8 files changed, 301 insertions(+)
>> create mode 100644 drivers/fwctl/pds/Makefile
>> create mode 100644 drivers/fwctl/pds/main.c
>> create mode 100644 include/uapi/fwctl/pds.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 3381e41dcf37..c63fd76a3684 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -9576,6 +9576,13 @@ L: linux-kernel@vger.kernel.org
>> S: Maintained
>> F: drivers/fwctl/mlx5/
>>
>> +FWCTL PDS DRIVER
>> +M: Brett Creeley <brett.creeley@amd.com>
>> +R: Shannon Nelson <shannon.nelson@amd.com>
>> +L: linux-kernel@vger.kernel.org
>> +S: Maintained
>> +F: drivers/fwctl/pds/
>> +
>> GALAXYCORE GC0308 CAMERA SENSOR DRIVER
>> M: Sebastian Reichel <sre@kernel.org>
>> L: linux-media@vger.kernel.org
>> diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
>> index f802cf5d4951..b5583b12a011 100644
>> --- a/drivers/fwctl/Kconfig
>> +++ b/drivers/fwctl/Kconfig
>> @@ -19,5 +19,15 @@ config FWCTL_MLX5
>> This will allow configuration and debug tools to work out of the box on
>> mainstream kernel.
>>
>> + If you don't know what to do here, say N.
>> +
>> +config FWCTL_PDS
>> + tristate "AMD/Pensando pds fwctl driver"
>> + depends on PDS_CORE
>> + help
>> + The pds_fwctl driver provides an fwctl interface for a user process
>> + to access the debug and configuration information of the AMD/Pensando
>> + DSC hardware family.
>> +
>> If you don't know what to do here, say N.
>> endif
>> diff --git a/drivers/fwctl/Makefile b/drivers/fwctl/Makefile
>> index 1c535f694d7f..c093b5f661d6 100644
>> --- a/drivers/fwctl/Makefile
>> +++ b/drivers/fwctl/Makefile
>> @@ -1,5 +1,6 @@
>> # SPDX-License-Identifier: GPL-2.0
>> obj-$(CONFIG_FWCTL) += fwctl.o
>> obj-$(CONFIG_FWCTL_MLX5) += mlx5/
>> +obj-$(CONFIG_FWCTL_PDS) += pds/
>>
>> fwctl-y += main.o
>> diff --git a/drivers/fwctl/pds/Makefile b/drivers/fwctl/pds/Makefile
>> new file mode 100644
>> index 000000000000..cc2317c07be1
>> --- /dev/null
>> +++ b/drivers/fwctl/pds/Makefile
>> @@ -0,0 +1,4 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_FWCTL_PDS) += pds_fwctl.o
>> +
>> +pds_fwctl-y += main.o
>> diff --git a/drivers/fwctl/pds/main.c b/drivers/fwctl/pds/main.c
>> new file mode 100644
>> index 000000000000..27942315a602
>> --- /dev/null
>> +++ b/drivers/fwctl/pds/main.c
>> @@ -0,0 +1,169 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright(c) Advanced Micro Devices, Inc */
>> +
>> +#include <linux/module.h>
>> +#include <linux/auxiliary_bus.h>
>> +#include <linux/pci.h>
>> +#include <linux/vmalloc.h>
>> +
>> +#include <uapi/fwctl/fwctl.h>
>> +#include <uapi/fwctl/pds.h>
>> +#include <linux/fwctl.h>
>> +
>> +#include <linux/pds/pds_common.h>
>> +#include <linux/pds/pds_core_if.h>
>> +#include <linux/pds/pds_adminq.h>
>> +#include <linux/pds/pds_auxbus.h>
>> +
>> +struct pdsfc_uctx {
>> + struct fwctl_uctx uctx;
>> + u32 uctx_caps;
>> +};
>> +
>> +struct pdsfc_dev {
>> + struct fwctl_device fwctl;
>> + struct pds_auxiliary_dev *padev;
>> + u32 caps;
>> + struct pds_fwctl_ident ident;
>> +};
>> +
>> +static int pdsfc_open_uctx(struct fwctl_uctx *uctx)
>> +{
>> + struct pdsfc_dev *pdsfc = container_of(uctx->fwctl, struct pdsfc_dev, fwctl);
>> + struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
>> +
>> + pdsfc_uctx->uctx_caps = pdsfc->caps;
>> +
>> + return 0;
>> +}
>> +
>> +static void pdsfc_close_uctx(struct fwctl_uctx *uctx)
>> +{
>> +}
>> +
>> +static void *pdsfc_info(struct fwctl_uctx *uctx, size_t *length)
>> +{
>> + struct pdsfc_uctx *pdsfc_uctx = container_of(uctx, struct pdsfc_uctx, uctx);
>> + struct fwctl_info_pds *info;
>> +
>> + info = kzalloc(sizeof(*info), GFP_KERNEL);
>> + if (!info)
>> + return ERR_PTR(-ENOMEM);
>> +
>> + info->uctx_caps = pdsfc_uctx->uctx_caps;
>> +
>> + return info;
>> +}
>> +
>> +static int pdsfc_identify(struct pdsfc_dev *pdsfc)
>> +{
>> + struct device *dev = &pdsfc->fwctl.dev;
>> + union pds_core_adminq_comp comp = {0};
>> + union pds_core_adminq_cmd cmd;
>> + struct pds_fwctl_ident *ident;
>> + dma_addr_t ident_pa;
>> + int err;
>> +
>> + ident = dma_alloc_coherent(dev->parent, sizeof(*ident), &ident_pa, GFP_KERNEL);
>> + err = dma_mapping_error(dev->parent, ident_pa);
>> + if (err) {
>> + dev_err(dev, "Failed to map ident buffer\n");
>> + return err;
>> + }
>> +
>> + cmd = (union pds_core_adminq_cmd) {
>> + .fwctl_ident = {
>> + .opcode = PDS_FWCTL_CMD_IDENT,
>> + .version = 0,
>> + .len = cpu_to_le32(sizeof(*ident)),
>> + .ident_pa = cpu_to_le64(ident_pa),
>> + }
>> + };
>> +
>> + err = pds_client_adminq_cmd(pdsfc->padev, &cmd, sizeof(cmd), &comp, 0);
>> + if (err)
>> + dev_err(dev, "Failed to send adminq cmd opcode: %u err: %d\n",
>> + cmd.fwctl_ident.opcode, err);
>> + else
>> + pdsfc->ident = *ident;
>> +
>> + dma_free_coherent(dev->parent, sizeof(*ident), ident, ident_pa);
>> +
>> + return err;
>> +}
>> +
>> +static void *pdsfc_fw_rpc(struct fwctl_uctx *uctx, enum fwctl_rpc_scope scope,
>> + void *in, size_t in_len, size_t *out_len)
>> +{
>> + return NULL;
>> +}
>> +
>> +static const struct fwctl_ops pdsfc_ops = {
>> + .device_type = FWCTL_DEVICE_TYPE_PDS,
>> + .uctx_size = sizeof(struct pdsfc_uctx),
>> + .open_uctx = pdsfc_open_uctx,
>> + .close_uctx = pdsfc_close_uctx,
>> + .info = pdsfc_info,
>> + .fw_rpc = pdsfc_fw_rpc,
>> +};
>> +
>> +static int pdsfc_probe(struct auxiliary_device *adev,
>> + const struct auxiliary_device_id *id)
>> +{
>> + struct pds_auxiliary_dev *padev =
>> + container_of(adev, struct pds_auxiliary_dev, aux_dev);
>> + struct device *dev = &adev->dev;
>> + struct pdsfc_dev *pdsfc;
>> + int err;
>> +
>> + pdsfc = fwctl_alloc_device(&padev->vf_pdev->dev, &pdsfc_ops,
>> + struct pdsfc_dev, fwctl);
>> + if (!pdsfc)
>> + return dev_err_probe(dev, -ENOMEM, "Failed to allocate fwctl device struct\n");
>> + pdsfc->padev = padev;
>> +
>> + err = pdsfc_identify(pdsfc);
>> + if (err) {
>> + fwctl_put(&pdsfc->fwctl);
>> + return dev_err_probe(dev, err, "Failed to identify device\n");
>> + }
>> +
>> + err = fwctl_register(&pdsfc->fwctl);
>> + if (err) {
>> + fwctl_put(&pdsfc->fwctl);
>> + return dev_err_probe(dev, err, "Failed to register device\n");
>> + }
>> +
>> + auxiliary_set_drvdata(adev, pdsfc);
>> +
>> + return 0;
>> +}
>> +
>> +static void pdsfc_remove(struct auxiliary_device *adev)
>> +{
>> + struct pdsfc_dev *pdsfc = auxiliary_get_drvdata(adev);
>> +
>> + fwctl_unregister(&pdsfc->fwctl);
>> + fwctl_put(&pdsfc->fwctl);
>> +}
>> +
>> +static const struct auxiliary_device_id pdsfc_id_table[] = {
>> + {.name = PDS_CORE_DRV_NAME "." PDS_DEV_TYPE_FWCTL_STR },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(auxiliary, pdsfc_id_table);
>> +
>> +static struct auxiliary_driver pdsfc_driver = {
>> + .name = "pds_fwctl",
>> + .probe = pdsfc_probe,
>> + .remove = pdsfc_remove,
>> + .id_table = pdsfc_id_table,
>> +};
>> +
>> +module_auxiliary_driver(pdsfc_driver);
>> +
>> +MODULE_IMPORT_NS("FWCTL");
>> +MODULE_DESCRIPTION("pds fwctl driver");
>> +MODULE_AUTHOR("Shannon Nelson <shannon.nelson@amd.com>");
>> +MODULE_AUTHOR("Brett Creeley <brett.creeley@amd.com>");
>> +MODULE_LICENSE("GPL");
>> diff --git a/include/linux/pds/pds_adminq.h b/include/linux/pds/pds_adminq.h
>> index 4b4e9a98b37b..22c6d77b3dcb 100644
>> --- a/include/linux/pds/pds_adminq.h
>> +++ b/include/linux/pds/pds_adminq.h
>> @@ -1179,6 +1179,84 @@ struct pds_lm_host_vf_status_cmd {
>> u8 status;
>> };
>>
>> +enum pds_fwctl_cmd_opcode {
>> + PDS_FWCTL_CMD_IDENT = 70,
>> +};
>> +
>> +/**
>> + * struct pds_fwctl_cmd - Firmware control command structure
>> + * @opcode: Opcode
>> + * @rsvd: Reserved
>> + * @ep: Endpoint identifier
>> + * @op: Operation identifier
>> + */
>> +struct pds_fwctl_cmd {
>> + u8 opcode;
>> + u8 rsvd[3];
>> + __le32 ep;
>> + __le32 op;
>> +} __packed;
>> +
>> +/**
>> + * struct pds_fwctl_comp - Firmware control completion structure
>> + * @status: Status of the firmware control operation
>> + * @rsvd: Reserved
>> + * @comp_index: Completion index in little-endian format
>> + * @rsvd2: Reserved
>> + * @color: Color bit indicating the state of the completion
>> + */
>> +struct pds_fwctl_comp {
>> + u8 status;
>> + u8 rsvd;
>> + __le16 comp_index;
>> + u8 rsvd2[11];
>> + u8 color;
>> +} __packed;
>> +
>> +/**
>> + * struct pds_fwctl_ident_cmd - Firmware control identification command structure
>> + * @opcode: Operation code for the command
>> + * @rsvd: Reserved
>> + * @version: Interface version
>> + * @rsvd2: Reserved
>> + * @len: Length of the identification data
>> + * @ident_pa: Physical address of the identification data
>> + */
>> +struct pds_fwctl_ident_cmd {
>> + u8 opcode;
>> + u8 rsvd;
>> + u8 version;
>> + u8 rsvd2;
>> + __le32 len;
>> + __le64 ident_pa;
>> +} __packed;
>> +
>> +/* future feature bits here
>> + * enum pds_fwctl_features {
>> + * };
>> + * (compilers don't like empty enums)
>> + */
>> +
>> +/**
>> + * struct pds_fwctl_ident - Firmware control identification structure
>> + * @features: Supported features (enum pds_fwctl_features)
>> + * @version: Interface version
>> + * @rsvd: Reserved
>> + * @max_req_sz: Maximum request size
>> + * @max_resp_sz: Maximum response size
>> + * @max_req_sg_elems: Maximum number of request SGs
>> + * @max_resp_sg_elems: Maximum number of response SGs
>> + */
>> +struct pds_fwctl_ident {
>> + __le64 features;
>> + u8 version;
>> + u8 rsvd[3];
>> + __le32 max_req_sz;
>> + __le32 max_resp_sz;
>> + u8 max_req_sg_elems;
>> + u8 max_resp_sg_elems;
>> +} __packed;
>> +
>> union pds_core_adminq_cmd {
>> u8 opcode;
>> u8 bytes[64];
>> @@ -1216,6 +1294,9 @@ union pds_core_adminq_cmd {
>> struct pds_lm_dirty_enable_cmd lm_dirty_enable;
>> struct pds_lm_dirty_disable_cmd lm_dirty_disable;
>> struct pds_lm_dirty_seq_ack_cmd lm_dirty_seq_ack;
>> +
>> + struct pds_fwctl_cmd fwctl;
>> + struct pds_fwctl_ident_cmd fwctl_ident;
>> };
>>
>> union pds_core_adminq_comp {
>> @@ -1243,6 +1324,8 @@ union pds_core_adminq_comp {
>>
>> struct pds_lm_state_size_comp lm_state_size;
>> struct pds_lm_dirty_status_comp lm_dirty_status;
>> +
>> + struct pds_fwctl_comp fwctl;
>> };
>>
>> #ifndef __CHECKER__
>> diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
>> index c2d5abc5a726..716ac0eee42d 100644
>> --- a/include/uapi/fwctl/fwctl.h
>> +++ b/include/uapi/fwctl/fwctl.h
>> @@ -44,6 +44,7 @@ enum fwctl_device_type {
>> FWCTL_DEVICE_TYPE_ERROR = 0,
>> FWCTL_DEVICE_TYPE_MLX5 = 1,
>> FWCTL_DEVICE_TYPE_CXL = 2,
>> + FWCTL_DEVICE_TYPE_PDS = 4,
>> };
>>
>> /**
>> diff --git a/include/uapi/fwctl/pds.h b/include/uapi/fwctl/pds.h
>> new file mode 100644
>> index 000000000000..558e030b7583
>> --- /dev/null
>> +++ b/include/uapi/fwctl/pds.h
>> @@ -0,0 +1,26 @@
>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>> +/* Copyright(c) Advanced Micro Devices, Inc */
>> +
>> +/*
>> + * fwctl interface info for pds_fwctl
>> + */
>> +
>> +#ifndef _UAPI_FWCTL_PDS_H_
>> +#define _UAPI_FWCTL_PDS_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/*
>> + * struct fwctl_info_pds
>> + *
>> + * Return basic information about the FW interface available.
>> + */
>
> Please use proper kdoc formatting for the comment block.
Will do, thanks,
sln
>
>> +struct fwctl_info_pds {
>> + __u32 uctx_caps;
>> +};
>> +
>> +enum pds_fwctl_capabilities {
>> + PDS_FWCTL_QUERY_CAP = 0,
>> + PDS_FWCTL_SEND_CAP,
>> +};
>> +#endif /* _UAPI_FWCTL_PDS_H_ */
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v3 6/6] pds_fwctl: add Documentation entries
2025-03-10 20:27 ` Dave Jiang
@ 2025-03-17 22:41 ` Nelson, Shannon
0 siblings, 0 replies; 21+ messages in thread
From: Nelson, Shannon @ 2025-03-17 22:41 UTC (permalink / raw)
To: Dave Jiang, jgg, andrew.gospodarek, aron.silverton,
dan.j.williams, daniel.vetter, dsahern, gregkh, hch, itayavr,
jiri, Jonathan.Cameron, kuba, lbloch, leonro, linux-cxl,
linux-rdma, netdev, saeedm
Cc: brett.creeley
On 3/10/2025 1:27 PM, Dave Jiang wrote:
>
> On 3/7/25 11:53 AM, Shannon Nelson wrote:
>> Add pds_fwctl to the driver and fwctl documentation pages.
>>
>> Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> ---
>> Documentation/userspace-api/fwctl/fwctl.rst | 1 +
>> Documentation/userspace-api/fwctl/index.rst | 1 +
>> .../userspace-api/fwctl/pds_fwctl.rst | 40 +++++++++++++++++++
>> 3 files changed, 42 insertions(+)
>> create mode 100644 Documentation/userspace-api/fwctl/pds_fwctl.rst
>>
>> diff --git a/Documentation/userspace-api/fwctl/fwctl.rst b/Documentation/userspace-api/fwctl/fwctl.rst
>> index 04ad78a7cd48..fdcfe418a83f 100644
>> --- a/Documentation/userspace-api/fwctl/fwctl.rst
>> +++ b/Documentation/userspace-api/fwctl/fwctl.rst
>> @@ -150,6 +150,7 @@ fwctl User API
>>
>> .. kernel-doc:: include/uapi/fwctl/fwctl.h
>> .. kernel-doc:: include/uapi/fwctl/mlx5.h
>> +.. kernel-doc:: include/uapi/fwctl/pds.h
>>
>> sysfs Class
>> -----------
>> diff --git a/Documentation/userspace-api/fwctl/index.rst b/Documentation/userspace-api/fwctl/index.rst
>> index d9d40a468a31..316ac456ad3b 100644
>> --- a/Documentation/userspace-api/fwctl/index.rst
>> +++ b/Documentation/userspace-api/fwctl/index.rst
>> @@ -11,3 +11,4 @@ to securely construct and execute RPCs inside device firmware.
>>
>> fwctl
>> fwctl-cxl
>> + pds_fwctl
>> diff --git a/Documentation/userspace-api/fwctl/pds_fwctl.rst b/Documentation/userspace-api/fwctl/pds_fwctl.rst
>> new file mode 100644
>> index 000000000000..e8a63d4215d0
>> --- /dev/null
>> +++ b/Documentation/userspace-api/fwctl/pds_fwctl.rst
>> @@ -0,0 +1,40 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +================
>> +fwctl pds driver
>> +================
>> +
>> +:Author: Shannon Nelson
>> +
>> +Overview
>> +========
>> +
>> +The PDS Core device makes an fwctl service available through an
>
> s/an fwctl/a fwctl/
>
>> +auxiliary_device named pds_core.fwctl.N. The pds_fwctl driver binds to
>> +this device and registers itself with the fwctl subsystem. The resulting
>> +userspace interface is used by an application that is a part of the
>> +AMD/Pensando software package for the Distributed Service Card (DSC).
>> +
>> +The pds_fwctl driver has little knowledge of the firmware's internals,
>> +only knows how to send commands through pds_core's message queue to the
> s/ , only/. It only/
>
>> +firmware for fwctl requests. The set of fwctl operations available
>> +depends on the firmware in the DSC, and the userspace application
>> +version must match the firmware so that they can talk to each other.
>> +
>> +When a connection is created the pds_fwctl driver requests from the
>> +firmware a list of firmware object endpoints, and for each endpoint the
>> +driver requests a list of operations for the endpoint. Each operation
>> +description includes a minimum scope level that the pds_fwctl driver can
>> +use for filtering privilege levels.
>
> Maybe a bit more details on the privilege levels?
>
>> +
>> +pds_fwctl User API
>> +==================
>> +
>> +.. kernel-doc:: include/uapi/fwctl/pds.h
>> +
>> +Each RPC request includes the target endpoint and the operation id, and in
>> +and out buffer lengths and pointers. The driver verifies the existence
>> +of the requested endpoint and operations, then checks the current scope
> s/then/and then it/
>
>> +against the required scope of the operation. The request is then put
>> +together with the request data and sent through pds_core's message queue
>> +to the firmware, and the results are returned to the caller.
>
> May be a good idea to touch on each of the ioctls being exported as well as maybe provide some sample user code on how to perform the RPCs. This documentation should serve as a guide to an application programmer on how to write the user application that accesses the fwctl char dev.
>
Thanks, I'll work on this doc a little more.
sln
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2025-03-17 22:41 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-07 18:53 [PATCH v3 0/6] pds_fwctl: fwctl for AMD/Pensando core devices Shannon Nelson
2025-03-07 18:53 ` [PATCH v3 1/6] pds_core: make pdsc_auxbus_dev_del() void Shannon Nelson
2025-03-10 16:58 ` Dave Jiang
2025-03-07 18:53 ` [PATCH v3 2/6] pds_core: specify auxiliary_device to be created Shannon Nelson
2025-03-10 17:26 ` Dave Jiang
2025-03-17 22:30 ` Nelson, Shannon
2025-03-07 18:53 ` [PATCH v3 3/6] pds_core: add new fwctl auxiliary_device Shannon Nelson
2025-03-10 17:33 ` Dave Jiang
2025-03-17 22:37 ` Nelson, Shannon
2025-03-12 18:22 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 4/6] pds_fwctl: initial driver framework Shannon Nelson
2025-03-10 18:28 ` Dave Jiang
2025-03-17 22:40 ` Nelson, Shannon
2025-03-12 18:25 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 5/6] pds_fwctl: add rpc and query support Shannon Nelson
2025-03-07 23:38 ` Jason Gunthorpe
2025-03-17 22:24 ` Nelson, Shannon
2025-03-12 18:29 ` Jonathan Cameron
2025-03-07 18:53 ` [PATCH v3 6/6] pds_fwctl: add Documentation entries Shannon Nelson
2025-03-10 20:27 ` Dave Jiang
2025-03-17 22:41 ` Nelson, Shannon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).