netdev - Re: [PATCH net-next 3/5] net: wwan: t7xx: PCIe reset rescan

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHNKnsT7zC4fTmc_+17Vy05aP-=vfZhwjOhbYJLOd=OZNMVD0w@mail.gmail.com>
Date:   Wed, 7 Sep 2022 01:19:14 +0300
From:   Sergey Ryazanov <ryazanov.s.a@...il.com>
To:     "Kumar, M Chetan" <m.chetan.kumar@...ux.intel.com>
Cc:     netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Johannes Berg <johannes@...solutions.net>,
        Loic Poulain <loic.poulain@...aro.org>,
        "Sudi, Krishna C" <krishna.c.sudi@...el.com>,
        Intel Corporation <linuxwwan@...el.com>,
        Haijun Liu <haijun.liu@...iatek.com>,
        Madhusmita Sahu <madhusmita.sahu@...el.com>,
        Ricardo Martinez <ricardo.martinez@...ux.intel.com>,
        Devegowda Chandrashekar <chandrashekar.devegowda@...el.com>
Subject: Re: [PATCH net-next 3/5] net: wwan: t7xx: PCIe reset rescan

On Fri, Sep 2, 2022 at 7:50 AM Kumar, M Chetan
<m.chetan.kumar@...ux.intel.com> wrote:
> On 8/30/2022 7:32 AM, Sergey Ryazanov wrote:
>> On Tue, Aug 16, 2022 at 7:12 AM <m.chetan.kumar@...el.com> wrote:
>>> From: Haijun Liu <haijun.liu@...iatek.com>
>>>
>>> PCI rescan module implements "rescan work queue". In firmware flashing
>>> or coredump collection procedure WWAN device is programmed to boot in
>>> fastboot mode and a work item is scheduled for removal & detection.
>>> The WWAN device is reset using APCI call as part driver removal flow.
>>> Work queue rescans pci bus at fixed interval for device detection,
>>> later when device is detect work queue exits.
>>
>> [skipped]
>>
>>> diff --git a/drivers/net/wwan/t7xx/t7xx_pci_rescan.c b/drivers/net/wwan/t7xx/t7xx_pci_rescan.c
>>> new file mode 100644
>>> index 000000000000..045777d8a843
>>> --- /dev/null
>>> +++ b/drivers/net/wwan/t7xx/t7xx_pci_rescan.c
>>
>> [skipped]
>>
>>> +static void t7xx_remove_rescan(struct work_struct *work)
>>> +{
>>> +       struct pci_dev *pdev;
>>> +       int num_retries = RESCAN_RETRIES;
>>> +       unsigned long flags;
>>> +
>>> +       spin_lock_irqsave(&g_mtk_rescan_context.dev_lock, flags);
>>> +       g_mtk_rescan_context.rescan_done = 0;
>>> +       pdev = g_mtk_rescan_context.dev;
>>> +       spin_unlock_irqrestore(&g_mtk_rescan_context.dev_lock, flags);
>>> +
>>> +       if (pdev) {
>>> +               pci_stop_and_remove_bus_device_locked(pdev);
>>
>> What is the purpose of removing the device then trying to find it by
>> rescanning the bus? Would not it be easier to save a PCI device
>> configuration, reset the device, and then restore the configuration?
>
> If hotplug is disabled, the device is not removed on reset. So in this
> case driver need to handle the device removal and rescan.

I still can not understand this part and need a clue. Why should the
driver disable the hotplug?

And is there a more gentle way to reset the firmware without the
device object removing?

>>> +               pr_debug("start remove and rescan flow\n");
>>> +       }
>>> +
>>> +       do {
>>> +               t7xx_pci_dev_rescan();
>>> +               spin_lock_irqsave(&g_mtk_rescan_context.dev_lock, flags);
>>> +               if (g_mtk_rescan_context.rescan_done) {
>>> +                       spin_unlock_irqrestore(&g_mtk_rescan_context.dev_lock, flags);
>>> +                       break;
>>> +               }
>>> +
>>> +               spin_unlock_irqrestore(&g_mtk_rescan_context.dev_lock, flags);
>>> +               msleep(DELAY_RESCAN_MTIME);
>>> +       } while (num_retries--);
>>> +}

-- 
Sergey