| 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
| ||
|
Message-ID: <d6a3e94c-2b05-f9d4-2522-37153e45c1d8@gmail.com>
Date: Sun, 5 Aug 2018 13:18:55 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: "Winkler, Tomas" <tomas.winkler@...el.com>
Cc: Arnd Bergmann <arnd@...db.de>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] mei: Migrate to pci_alloc_irq_vectors API
On 05.08.2018 13:02, Winkler, Tomas wrote:
>>
>> Update the driver to the pci_alloc_irq_vectors API, this allows to get rid of
>> legacy calls like pci_enable_msi(). Another benefit is that no driver change
>> would be needed in case the hardware starts supporting MSI-X.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@...il.com>
>
> Sorry, we are losing the ONE_SHOT flag here, In spite that pci_enable_msi is marked as deprecated,
ONE_SHOT flag isn't needed here. See commit f7368a550275 ("PCI: Use IRQF_ONESHOT if
pci_request_irq() called with no handler") and also following comment in __setup_irq():
* Drivers are often written to work w/o knowledge about the
* underlying irq chip implementation, so a request for a
* threaded irq without a primary hard irq context handler
* requires the ONESHOT flag to be set. Some irq chips like
* MSI based interrupts are per se one shot safe. Check the
* chip flags, so we can avoid the unmask dance at the end of
* the threaded handler for those.
> I don't see overall action on LKML to actually deprecate it.
> For this change I would need to extensice testing on future and legacy devices, frankly I prefer not to dig into it right now.
Sure, up to you.
> Thanks
> Tomas
>
Rgds, Heiner
>> ---
>> drivers/misc/mei/pci-me.c | 25 +++++++++++--------------
>> drivers/misc/mei/pci-txe.c | 30 ++++++++++++++----------------
>> 2 files changed, 25 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/misc/mei/pci-me.c b/drivers/misc/mei/pci-me.c index
>> 55ba1c41..a60376f0 100644
>> --- a/drivers/misc/mei/pci-me.c
>> +++ b/drivers/misc/mei/pci-me.c
>> @@ -149,7 +149,6 @@ static int mei_me_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> const struct mei_cfg *cfg;
>> struct mei_device *dev;
>> struct mei_me_hw *hw;
>> - unsigned int irqflags;
>> int err;
>>
>> cfg = mei_me_get_cfg(ent->driver_data); @@ -196,18 +195,16 @@
>> static int mei_me_probe(struct pci_dev *pdev, const struct pci_device_id
>> *ent)
>> hw = to_me_hw(dev);
>> hw->mem_addr = pcim_iomap_table(pdev)[0];
>>
>> - pci_enable_msi(pdev);
>> -
>> - /* request and enable interrupt */
>> - irqflags = pci_dev_msi_enabled(pdev) ? IRQF_ONESHOT :
>> IRQF_SHARED;
>> + err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>> + if (err < 0)
>> + goto end;
>>
>> - err = request_threaded_irq(pdev->irq,
>> - mei_me_irq_quick_handler,
>> - mei_me_irq_thread_handler,
>> - irqflags, KBUILD_MODNAME, dev);
>> + /* request and enable interrupt */
>> + err = pci_request_irq(pdev, 0, mei_me_irq_quick_handler,
>> + mei_me_irq_thread_handler, dev,
>> KBUILD_MODNAME);
>> if (err) {
>> - dev_err(&pdev->dev, "request_threaded_irq failure. irq =
>> %d\n",
>> - pdev->irq);
>> + pci_err(pdev, "pci_request_irq failure. irq = %d\n",
>> + pci_irq_vector(pdev, 0));
>> goto end;
>> }
>>
>> @@ -258,7 +255,7 @@ static int mei_me_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> release_irq:
>> mei_cancel_work(dev);
>> mei_disable_interrupts(dev);
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>> end:
>> dev_err(&pdev->dev, "initialization failed.\n");
>> return err;
>> @@ -287,7 +284,7 @@ static void mei_me_shutdown(struct pci_dev *pdev)
>> mei_me_unset_pm_domain(dev);
>>
>> mei_disable_interrupts(dev);
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>> }
>>
>> /**
>> @@ -316,7 +313,7 @@ static void mei_me_remove(struct pci_dev *pdev)
>>
>> mei_disable_interrupts(dev);
>>
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>>
>> mei_deregister(dev);
>> }
>> diff --git a/drivers/misc/mei/pci-txe.c b/drivers/misc/mei/pci-txe.c index
>> b8a49581..3ea2b8f6 100644
>> --- a/drivers/misc/mei/pci-txe.c
>> +++ b/drivers/misc/mei/pci-txe.c
>> @@ -64,6 +64,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent) {
>> struct mei_device *dev;
>> struct mei_txe_hw *hw;
>> + irq_handler_t qh = NULL;
>> const int mask = BIT(SEC_BAR) | BIT(BRIDGE_BAR);
>> int err;
>>
>> @@ -100,25 +101,22 @@ static int mei_txe_probe(struct pci_dev *pdev,
>> const struct pci_device_id *ent)
>> hw = to_txe_hw(dev);
>> hw->mem_addr = pcim_iomap_table(pdev);
>>
>> - pci_enable_msi(pdev);
>> + err = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
>> + if (err < 0)
>> + goto end;
>>
>> /* clear spurious interrupts */
>> mei_clear_interrupts(dev);
>>
>> + if (!pci_dev_msi_enabled(pdev))
>> + qh = mei_txe_irq_quick_handler;
>> +
>> /* request and enable interrupt */
>> - if (pci_dev_msi_enabled(pdev))
>> - err = request_threaded_irq(pdev->irq,
>> - NULL,
>> - mei_txe_irq_thread_handler,
>> - IRQF_ONESHOT, KBUILD_MODNAME, dev);
>> - else
>> - err = request_threaded_irq(pdev->irq,
>> - mei_txe_irq_quick_handler,
>> - mei_txe_irq_thread_handler,
>> - IRQF_SHARED, KBUILD_MODNAME, dev);
>> + err = pci_request_irq(pdev, 0, qh, mei_txe_irq_thread_handler,
>> + dev, KBUILD_MODNAME);
>> if (err) {
>> - dev_err(&pdev->dev, "mei: request_threaded_irq failure. irq
>> = %d\n",
>> - pdev->irq);
>> + pci_err(pdev, "mei: pci_request_irq failure. irq = %d\n",
>> + pci_irq_vector(pdev, 0));
>> goto end;
>> }
>>
>> @@ -162,7 +160,7 @@ static int mei_txe_probe(struct pci_dev *pdev, const
>> struct pci_device_id *ent)
>> release_irq:
>> mei_cancel_work(dev);
>> mei_disable_interrupts(dev);
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>> end:
>> dev_err(&pdev->dev, "initialization failed.\n");
>> return err;
>> @@ -191,7 +189,7 @@ static void mei_txe_shutdown(struct pci_dev *pdev)
>> mei_txe_unset_pm_domain(dev);
>>
>> mei_disable_interrupts(dev);
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>> }
>>
>> /**
>> @@ -219,7 +217,7 @@ static void mei_txe_remove(struct pci_dev *pdev)
>> mei_txe_unset_pm_domain(dev);
>>
>> mei_disable_interrupts(dev);
>> - free_irq(pdev->irq, dev);
>> + pci_free_irq(pdev, 0, dev);
>>
>> mei_deregister(dev);
>> }
>> --
>> 2.18.0
>>
>
Powered by blists - more mailing lists