* [PATCH] accel/qaic: Add support for AI200 virtual functions
@ 2026-08-14 16:41 Youssef Samir
2026-08-14 16:59 ` sashiko-bot
2026-08-14 20:01 ` Jeff Hugo
0 siblings, 2 replies; 3+ messages in thread
From: Youssef Samir @ 2026-08-14 16:41 UTC (permalink / raw)
To: jeff.hugo, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Troy Hanson
Register the qaic_pci_sriov_configure callback to allow VFs to be
created through AIC200's sysfs numvfs entry. Additionally, define
a new MHI family for the 5 channels supported by AIC200 VFs: NNC,
Telemetry, IPCR, Loopback, and SSR.
Co-developed-by: Troy Hanson <thanson@qti.qualcomm.com>
Signed-off-by: Troy Hanson <thanson@qti.qualcomm.com>
Signed-off-by: Zack McKevitt <zmckevit@qti.qualcomm.com>
Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
---
drivers/accel/qaic/mhi_controller.c | 192 +++++++++++++++++++++++++++-
drivers/accel/qaic/qaic.h | 1 +
drivers/accel/qaic/qaic_drv.c | 36 ++++++
3 files changed, 227 insertions(+), 2 deletions(-)
diff --git a/drivers/accel/qaic/mhi_controller.c b/drivers/accel/qaic/mhi_controller.c
index 40e6d262ef21..c720b70b1e0f 100644
--- a/drivers/accel/qaic/mhi_controller.c
+++ b/drivers/accel/qaic/mhi_controller.c
@@ -23,6 +23,7 @@ MODULE_PARM_DESC(mhi_timeout_ms, "MHI controller timeout value");
static const char *fw_image_paths[FAMILY_MAX] = {
[FAMILY_AIC100] = "qcom/aic100/sbl.bin",
[FAMILY_AIC200] = "qcom/aic200/sbl.bin",
+ [FAMILY_AIC200_VF] = NULL,
};
static const struct mhi_channel_config aic100_channels[] = {
@@ -691,6 +692,159 @@ static const struct mhi_channel_config aic200_channels[] = {
},
};
+static const struct mhi_channel_config aic200vf_channels[] = {
+ {
+ .name = "QAIC_LOOPBACK",
+ .num = 0,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_LOOPBACK",
+ .num = 1,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_SSR",
+ .num = 6,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_SSR",
+ .num = 7,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_CONTROL",
+ .num = 10,
+ .num_elements = 128,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_CONTROL",
+ .num = 11,
+ .num_elements = 128,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_TELEMETRY",
+ .num = 16,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "QAIC_TELEMETRY",
+ .num = 17,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "IPCR",
+ .num = 24,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_TO_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+ {
+ .name = "IPCR",
+ .num = 25,
+ .num_elements = 32,
+ .local_elements = 0,
+ .event_ring = 0,
+ .dir = DMA_FROM_DEVICE,
+ .ee_mask = MHI_CH_EE_AMSS,
+ .pollcfg = 0,
+ .doorbell = MHI_DB_BRST_DISABLE,
+ .lpm_notify = false,
+ .offload_channel = false,
+ .doorbell_mode_switch = false,
+ .wake_capable = false,
+ },
+};
+
static struct mhi_event_config aic100_events[] = {
{
.num_elements = 32,
@@ -721,6 +875,21 @@ static struct mhi_event_config aic200_events[] = {
},
};
+static struct mhi_event_config aic200vf_events[] = {
+ {
+ .num_elements = 32,
+ .irq_moderation_ms = 0,
+ .irq = 0,
+ .channel = U32_MAX,
+ .priority = 1,
+ .mode = MHI_DB_BRST_DISABLE,
+ .data_type = MHI_ER_CTRL,
+ .hardware_event = false,
+ .client_managed = false,
+ .offload_channel = false,
+ },
+};
+
static struct mhi_controller_config mhi_cntrl_configs[] = {
[FAMILY_AIC100] = {
.max_channels = 128,
@@ -744,6 +913,17 @@ static struct mhi_controller_config mhi_cntrl_configs[] = {
.use_bounce_buf = false,
.m2_no_db = false,
},
+ [FAMILY_AIC200_VF] = {
+ .max_channels = 128,
+ .timeout_ms = 0, /* controlled by mhi_timeout */
+ .buf_len = 0,
+ .num_channels = ARRAY_SIZE(aic200vf_channels),
+ .ch_cfg = aic200vf_channels,
+ .num_events = ARRAY_SIZE(aic200vf_events),
+ .event_cfg = aic200vf_events,
+ .use_bounce_buf = false,
+ .m2_no_db = false,
+ },
};
static int mhi_read_reg(struct mhi_controller *mhi_cntrl, void __iomem *addr, u32 *out)
@@ -864,11 +1044,19 @@ struct mhi_controller *qaic_mhi_register_controller(struct pci_dev *pci_dev, voi
mhi_cntrl->fw_image = fw_image_paths[family];
- if (family == FAMILY_AIC200) {
+ switch (family) {
+ case FAMILY_AIC200:
mhi_cntrl->name = "AIC200";
mhi_cntrl->seg_len = SZ_512K;
- } else {
+ break;
+ case FAMILY_AIC200_VF:
+ mhi_cntrl->name = "AIC200_VF";
+ break;
+ case FAMILY_AIC100:
mhi_cntrl->name = "AIC100";
+ break;
+ default:
+ mhi_cntrl->name = NULL;
}
/* use latest configured timeout */
diff --git a/drivers/accel/qaic/qaic.h b/drivers/accel/qaic/qaic.h
index 02ca99000e2a..889fa51da7da 100644
--- a/drivers/accel/qaic/qaic.h
+++ b/drivers/accel/qaic/qaic.h
@@ -35,6 +35,7 @@
enum aic_families {
FAMILY_AIC100,
FAMILY_AIC200,
+ FAMILY_AIC200_VF,
FAMILY_MAX,
};
diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
index 0acf6cdd44f8..cde8eb8c7a71 100644
--- a/drivers/accel/qaic/qaic_drv.c
+++ b/drivers/accel/qaic/qaic_drv.c
@@ -77,6 +77,13 @@ static const struct qaic_device_config aic200_config = {
.dbc_bar_idx = 2,
};
+static const struct qaic_device_config aic200vf_config = {
+ .family = FAMILY_AIC200_VF,
+ .bar_mask = BIT(0) | BIT(1) | BIT(2) | BIT(4),
+ .mhi_bar_idx = 1,
+ .dbc_bar_idx = 2,
+};
+
bool datapath_polling;
module_param(datapath_polling, bool, 0400);
MODULE_PARM_DESC(datapath_polling, "Operate the datapath in polling mode");
@@ -761,6 +768,34 @@ static const struct dev_pm_ops qaic_pm_ops = {
SYSTEM_SLEEP_PM_OPS(qaic_pm_suspend, qaic_pm_resume)
};
+static int qaic_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
+{
+ struct qaic_device *qdev = pci_get_drvdata(pdev);
+ int ret;
+
+ /* Qaic device must be online to process VF bringup */
+ if (qdev->dev_state == QAIC_OFFLINE)
+ return -ENODEV;
+
+ if (qdev->dev_state == QAIC_BOOT)
+ return -EBUSY;
+
+ if (num_vfs == 0) {
+ pci_disable_sriov(pdev);
+ return 0;
+ }
+
+ ret = pci_enable_sriov(pdev, num_vfs);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to enable SR-IOV: %d (requested %d VFs, max %d)\n",
+ ret, num_vfs, pci_sriov_get_totalvfs(pdev));
+ return ret;
+ }
+
+ dev_dbg(&pdev->dev, "Successfully enabled %d VFs\n", num_vfs);
+ return num_vfs;
+}
+
static struct pci_driver qaic_pci_driver = {
.name = QAIC_NAME,
.id_table = qaic_ids,
@@ -768,6 +803,7 @@ static struct pci_driver qaic_pci_driver = {
.remove = qaic_pci_remove,
.shutdown = qaic_pci_shutdown,
.err_handler = &qaic_pci_err_handler,
+ .sriov_configure = qaic_pci_sriov_configure,
.driver = {
.pm = pm_sleep_ptr(&qaic_pm_ops),
},
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] accel/qaic: Add support for AI200 virtual functions
2026-08-14 16:41 [PATCH] accel/qaic: Add support for AI200 virtual functions Youssef Samir
@ 2026-08-14 16:59 ` sashiko-bot
2026-08-14 20:01 ` Jeff Hugo
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-14 16:59 UTC (permalink / raw)
To: Youssef Samir; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] Missing pci_disable_sriov() in the physical function driver removal path causes SR-IOV VFs to leak.
- [High] Device state check in qaic_pci_sriov_configure incorrectly blocks SR-IOV VF teardown when the device is offline or booting.
- [High] The aic200vf_config structure is never added to the qaic_ids PCI device ID table, preventing the driver from matching and binding to VF devices.
Pre-existing issues:
- [High] Missing pci_free_irq_vectors() in MSI initialization error paths after a successful pci_alloc_irq_vectors() call.
--
--- Patch [1]: [PATCH] accel/qaic: Add support for AI200 virtual functions ---
commit c554793b8689d7226dadc15db51370aa624d506a
Author: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
accel/qaic: Add support for AI200 virtual functions
Register the qaic_pci_sriov_configure callback to allow VFs to be
created through AIC200's sysfs numvfs entry. Additionally, define
a new MHI family for the 5 channels supported by AIC200 VFs: NNC,
Telemetry, IPCR, Loopback, and SSR.
> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index 0acf6cdd44f81..cde8eb8c7a71b 100644
> --- a/drivers/accel/qaic/qaic_drv.c
> +++ b/drivers/accel/qaic/qaic_drv.c
> @@ -77,6 +77,13 @@ static const struct qaic_device_config aic200_config = {
> .dbc_bar_idx = 2,
> };
>
> +static const struct qaic_device_config aic200vf_config = {
> + .family = FAMILY_AIC200_VF,
> + .bar_mask = BIT(0) | BIT(1) | BIT(2) | BIT(4),
> + .mhi_bar_idx = 1,
> + .dbc_bar_idx = 2,
> +};
> +
[Severity: High]
This structure is defined, but is it missing an entry in the qaic_ids
PCI device ID table?
If the VF's PCI ID is not registered in the table in
drivers/accel/qaic/qaic_drv.c, won't the driver fail to match and bind
to the newly created VFs, making this configuration unreachable?
[ ... ]
> @@ -761,6 +768,34 @@ static const struct dev_pm_ops qaic_pm_ops = {
> SYSTEM_SLEEP_PM_OPS(qaic_pm_suspend, qaic_pm_resume)
> };
>
> +static int qaic_pci_sriov_configure(struct pci_dev *pdev, int num_vfs)
> +{
> + struct qaic_device *qdev = pci_get_drvdata(pdev);
> + int ret;
> +
> + /* Qaic device must be online to process VF bringup */
> + if (qdev->dev_state == QAIC_OFFLINE)
> + return -ENODEV;
> +
> + if (qdev->dev_state == QAIC_BOOT)
> + return -EBUSY;
> +
> + if (num_vfs == 0) {
> + pci_disable_sriov(pdev);
> + return 0;
> + }
[Severity: High]
Does the dev_state check in qaic_pci_sriov_configure() correctly handle the
num_vfs == 0 teardown path?
If the device encounters a hardware error and enters QAIC_OFFLINE, an admin
might write 0 to sriov_numvfs to tear down the VFs. Since the state checks
happen before checking for num_vfs == 0, won't this incorrectly return
-ENODEV and permanently trap the VFs in an enabled state?
Could the num_vfs == 0 check be moved before the dev_state validations?
> +
> + ret = pci_enable_sriov(pdev, num_vfs);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to enable SR-IOV: %d (requested %d VFs, max %d)\n",
> + ret, num_vfs, pci_sriov_get_totalvfs(pdev));
> + return ret;
> + }
> +
> + dev_dbg(&pdev->dev, "Successfully enabled %d VFs\n", num_vfs);
> + return num_vfs;
> +}
> +
> static struct pci_driver qaic_pci_driver = {
> .name = QAIC_NAME,
> .id_table = qaic_ids,
> @@ -768,6 +803,7 @@ static struct pci_driver qaic_pci_driver = {
> .remove = qaic_pci_remove,
> .shutdown = qaic_pci_shutdown,
> .err_handler = &qaic_pci_err_handler,
> + .sriov_configure = qaic_pci_sriov_configure,
[Severity: High]
Since this patch enables SR-IOV, does qaic_pci_remove() in
drivers/accel/qaic/qaic_drv.c need to call pci_disable_sriov() before
tearing down the device resources?
If an administrator unbinds the PF driver while VFs are still active,
won't leaving SR-IOV enabled while destroying the PF's MHI and DRM backing
state lead to a resource leak and potential use-after-free when the active
VFs attempt to access the hardware?
[Severity: High]
This is a pre-existing issue, but looking at init_msi() in
drivers/accel/qaic/qaic_drv.c:
drivers/accel/qaic/qaic_drv.c:init_msi() {
...
mhi_irq = pci_irq_vector(pdev, 0);
if (mhi_irq < 0)
return mhi_irq;
for (i = 0; i < qdev->num_dbc; ++i) {
ret = devm_request_threaded_irq(&pdev->dev,
pci_irq_vector(pdev, qdev->single_msi ? 0 : i + 1),
dbc_irq_handler, dbc_irq_threaded_fn, IRQF_SHARED,
"qaic_dbc", &qdev->dbc[i]);
if (ret)
return ret;
...
}
Are we leaking IRQ vectors here?
According to the PCI subsystem guidelines, failing to call
pci_free_irq_vectors() in error paths after a successful
pci_alloc_irq_vectors() leaks IRQ resources. If devm_request_threaded_irq()
fails, this path returns the error code directly without freeing the vectors.
> .driver = {
> .pm = pm_sleep_ptr(&qaic_pm_ops),
> },
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260814164125.3652876-1-youssef.abdulrahman@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] accel/qaic: Add support for AI200 virtual functions
2026-08-14 16:41 [PATCH] accel/qaic: Add support for AI200 virtual functions Youssef Samir
2026-08-14 16:59 ` sashiko-bot
@ 2026-08-14 20:01 ` Jeff Hugo
1 sibling, 0 replies; 3+ messages in thread
From: Jeff Hugo @ 2026-08-14 20:01 UTC (permalink / raw)
To: Youssef Samir, carl.vanderlip, troy.hanson, zachary.mckevitt
Cc: ogabbay, lizhi.hou, karol.wachowski, linux-arm-msm, dri-devel,
Troy Hanson
On 8/14/2026 10:41 AM, Youssef Samir wrote:
> Register the qaic_pci_sriov_configure callback to allow VFs to be
> created through AIC200's sysfs numvfs entry. Additionally, define
> a new MHI family for the 5 channels supported by AIC200 VFs: NNC,
> Telemetry, IPCR, Loopback, and SSR.
>
> Co-developed-by: Troy Hanson <thanson@qti.qualcomm.com>
> Signed-off-by: Troy Hanson <thanson@qti.qualcomm.com>
> Signed-off-by: Zack McKevitt <zmckevit@qti.qualcomm.com>
> Signed-off-by: Youssef Samir <youssef.abdulrahman@oss.qualcomm.com>
I think you accidentally reset the author. Looks like it should be
Zack, but this is formatted as if Youssef is the listed author.
Looks like Sashiko has some valid feedback. Please have a look.
-Jeff
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-14 20:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 16:41 [PATCH] accel/qaic: Add support for AI200 virtual functions Youssef Samir
2026-08-14 16:59 ` sashiko-bot
2026-08-14 20:01 ` Jeff Hugo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.