Re: [PATCH v2 04/13] PCI/IOV: Allow VF drivers to handle resets

[Weili Qian <qianweili@huawei.com>]

Thanks for adding us.

We've reviewed the series and confirmed that it conflicts with QM's 
existing PF↔VF reset coordination. QM currently handles PF FLR through 
driver-specific mechanisms: the PF notifies all VFs before reset an each 
VF stops itself and acknowledges back. The new 
reset_prepare()/reset_done() calls on each VF before the PF's introduce 
a second path that contends with QM's QM_RESETTING bit lock across 
reset_prepare/reset_done, causing timeouts on both host-side VFs (QM 
driver) and passthrough VFs (hisi-acc-vfio-pci driver).

We support the direction of this series and will adapt QM and 
hisi-acc-vfio-pci to work with the PCI core's unified VF handling.

Additionally, we notice that passthrough VFs bound to generic vfio-pci 
do not receive reset_prepare/reset_done notification. We're wondering 
whether there's a way to uniformly cover both host-side and passthrough 
VFs in the PCI core.

Thanks,
Weili

On 2026/10/28 0:51, Laguna, Lukasz wrote:
>
>
> Adding HiSilicon QM and hisi-acc-vfio-pci maintainers.
>
> Hi Zhou, Weili, Longfang,
>
> The series [1] adds generic handling of VFs across a PF reset in the 
> PCI core. On PF reset, the core now saves / restores all subordinate 
> VFs config state, and notify their drivers via .reset_prepare() / 
> .reset_done(), as VF state is lost on PF reset anyway.
>
> Sashiko points out above [2], that this conflicts with the 
> driver-specific VF handling in HiSilicon QM and hisi-acc-vfio-pci. It 
> seems to be a valid point.
>
> Could you please take a look and share your thoughts?
>
> [1] 
> https://lore.kernel.org/linux-pci/20261027160446.858300-1-lukasz.laguna@intel.com/
> [2] https://lore.kernel.org/linux-pci/sashiko-outbox-186926@kernel.org/
>
> Thanks,
> Lukasz
>
> On 10/27/2026 17:34, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential 
>> issue(s) to consider:
>> - [High] PCI/IOV: calling VF reset_prepare() before PF breaks hisi_qm 
>> PF FLR
>> -- 
>>
>> commit b82e20ef0c7a057bbb242268e11b3dd9218fd7d8
>> Author: Michał Winiarski <michal.winiarski@intel.com>
>>
>> PCI/IOV: Allow VF drivers to handle resets
>>
>> This commit updates PCI function, bus, and slot reset handling to lock
>> enabled VFs, notify VF drivers via .reset_prepare() and 
>> .reset_done(), and
>> save and restore VF configuration state across PF resets.
>>
>>> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
>>> index d8e7c7d8e4021..d088302547c34 100644
>>> --- a/drivers/pci/pci.c
>>> +++ b/drivers/pci/pci.c
>> [ ... ]
>>
>>> @@ -5171,6 +5202,17 @@ static void pci_dev_save_and_disable(struct 
>>> pci_dev *dev)
>>>           return;
>>>         pci_save_state(dev);
>>> +}
>>> +
>>> +static void pci_dev_save_and_disable(struct pci_dev *dev)
>>> +{
>>> +    struct pci_dev *vf;
>>> +
>>> +    for_each_pci_vf(dev, vf)
>>> +        pci_dev_reset_prepare_and_save(vf);
>>> +
>>> +    pci_dev_reset_prepare_and_save(dev);
>> [Severity: High]
>> Does calling .reset_prepare() on each VF before the PF here cause a
>> self-blocking timeout and reset failure on HiSilicon QM devices
>> (hisi_hpre, hisi_sec2, hisi_zip, and hisi_acc_vfio_pci) when 
>> resetting a PF
>> with enabled VFs?
>>
>> In pci_dev_save_and_disable(), pci_dev_reset_prepare_and_save(vf) 
>> runs for
>> every VF and then pci_dev_reset_prepare_and_save(dev) runs for the PF 
>> in the
>> same thread before any .reset_done() callback is invoked in
>> pci_dev_restore().
>>
>> When VFs are bound to hisi_acc_vfio_pci (where hisi_acc_vdev->pf_qm 
>> points
>> to the PF's struct hisi_qm), the first VF's .reset_prepare() sets the 
>> reset
>> bit on the shared PF qm->misc_ctl and holds it until .reset_done():
>>
>> drivers/vfio/pci/hisilicon/hisi_acc_vfio_pci.c:
>> hisi_acc_vf_pci_reset_prepare() {
>>     struct hisi_acc_vf_core_device *hisi_acc_vdev = 
>> hisi_acc_drvdata(pdev);
>>     struct hisi_qm *qm = hisi_acc_vdev->pf_qm;
>>     ...
>>     while (test_and_set_bit(QM_RESETTING, &qm->misc_ctl)) {
>>         msleep(1);
>>         if (++delay > QM_RESET_WAIT_TIMEOUT) {
>>             dev_err(dev, "reset prepare failed\n");
>>             return;
>>         }
>>     }
>> }
>>
>> Similarly, when VFs are bound to hisi_hpre, hisi_sec2, or hisi_zip on
>> Kunpeng920, hisi_qm_reset_prepare() -> qm_reset_prepare_ready() 
>> acquires the
>> same bit on pf_qm->misc_ctl via qm_wait_reset_finish(pf_qm) and only 
>> clears
>> it in hisi_qm_reset_done() -> qm_reset_bit_clear():
>>
>> drivers/crypto/hisilicon/qm.c:
>> qm_reset_prepare_ready() {
>>     struct pci_dev *pdev = qm->pdev;
>>     struct hisi_qm *pf_qm = pci_get_drvdata(pci_physfn(pdev));
>>
>>     if (qm->ver < QM_HW_V3)
>>         return qm_wait_reset_finish(pf_qm);
>>
>>     return qm_wait_reset_finish(qm);
>> }
>>
>> Because the first VF's .reset_done() has not run yet, every 
>> subsequent VF's
>> .reset_prepare() and the PF's own hisi_qm_reset_prepare() will spin 
>> waiting
>> for that bit on pf_qm->misc_ctl until the timeout expires, log an 
>> error, and
>> return early without stopping the remaining VFs or the PF prior to the
>> hardware reset.
>>
>> On Kunpeng930 (where qm->ver >= QM_HW_V3) with host-bound VFs, each VF's
>> hisi_qm_reset_prepare() sets the bit on its own vf_qm->misc_ctl. When 
>> the PF
>> subsequently runs hisi_qm_reset_prepare(), it notifies VFs via 
>> mailbox and
>> waits for them:
>>
>> drivers/crypto/hisilicon/qm.c:
>> hisi_qm_reset_prepare()
>>    -> qm_try_stop_vfs()
>>         -> qm_ping_all_vfs()
>>         -> qm_wait_vf_prepare_finish()
>>
>> Each VF's mailbox work handler (qm_pf_reset_vf_process() ->
>> qm_pf_reset_vf_prepare() -> qm_reset_prepare_ready(vf_qm)) then tries to
>> acquire the bit on vf_qm->misc_ctl that is already held by that VF's own
>> .reset_prepare(), timing out and causing the PF reset preparation to 
>> fail as
>> well.
>>
>
> .
>