Re: [PATCH v2] accel/qaic: Add support for AI200 virtual functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] System crash on VF suspend due to unconditional NULL pointer dereference of qdev->mqts_ch.
--

commit 136267f1b302748ae2c7ab124a5490ff5bbea19a
Author: Zack McKevitt <zmckevit@qti.qualcomm.com>

accel/qaic: Add support for AI200 virtual functions

This patch registers the qaic_pci_sriov_configure() callback to allow
virtual functions to be created through the AIC200's sysfs numvfs entry.
It also defines a new MHI family for the channels supported by AIC200 VFs.

Link: https://lore.kernel.org/all/20260814165907.C76841F00A3A@smtp.kernel.org/

> diff --git a/drivers/accel/qaic/qaic_drv.c b/drivers/accel/qaic/qaic_drv.c
> index 0acf6cdd44f81..6427105ebdf75 100644
> --- a/drivers/accel/qaic/qaic_drv.c
> +++ b/drivers/accel/qaic/qaic_drv.c
[ ... ]
> @@ -768,6 +814,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),

[Severity: High]
This is a pre-existing issue, but since this patch introduces virtual
functions that can be suspended, will this cause a system crash on VF suspend?

When the system suspends a QAIC virtual function, qaic_pm_suspend() is called
via the qaic_pm_ops struct. This function unconditionally calls into the
timesync code:

drivers/accel/qaic/qaic_drv.c:qaic_pm_suspend() {
        ...
        qaic_mqts_ch_stop_timer(qdev->mqts_ch);
        ...
}

However, since VFs do not support or initialize the QAIC_TIMESYNC_PERIODIC
channel, qdev->mqts_ch is always NULL for them. This leads to an unconditional
NULL pointer dereference when stopping the timer:

drivers/accel/qaic/qaic_timesync.c:qaic_mqts_ch_stop_timer() {
        struct mqts_dev *mqtsdev = dev_get_drvdata(&mhi_dev->dev);
        ...
}

Because dev_get_drvdata accesses dev->driver_data, passing the evaluated
address of the NULL pointer's dev struct will trigger a kernel panic. Should
qaic_pm_suspend() or qaic_mqts_ch_stop_timer() check for a NULL channel
before attempting to stop the timer?

>  	},
>  };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831135008.1418190-1-youssef.abdulrahman@oss.qualcomm.com?part=1