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 <[email protected]> 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/[email protected]/ > 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/[email protected]?part=1
