linux-kernel - Re: [PATCH] char: xillybus: Replace deprecated MSI API

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: <aed95641-4c95-5fbb-f359-885a3e2144f7@outbound.gmail.com>
Date: Thu, 17 Jul 2025 15:51:28 +0200
From: Eli Billauer <eli.billauer@...il.com>
To: Salah Triki <salah.triki@...il.com>, Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] char: xillybus: Replace deprecated MSI API

Hello,

Thanks for your patch.

I can see that you've chosen PCI_IRQ_ALL_TYPES as the selector of IRQ 
types. As far as I understand, this changes the behavior of the driver, 
allowing interrupt types that it shouldn't...?

Another thing: I have to admit that I missed the need for a call to 
pci_disable_msi() when the device shuts down (which is what 
pci_free_irq_vectors() effectively does). Thanks for bringing that to my 
attention.

However, as this is needed, it must be assured that this call is made in 
any case that the device's initialization fails. This driver is using 
the managed device API, so I suppose the correct way is to add an devm 
action for pci_free_irq_vectors(). It appears to me that just adding it 
to the remove method won't be sufficient.

Regards,
    Eli

On 16/07/2025 21:39, Salah Triki wrote:
> Replace deprecated pci_enable_msi() with pci_alloc_irq_vectors(). And
> add pci_free_irq_vectors() to free the allocated vectors before removing
> the device.
> 
> Signed-off-by: Salah Triki <salah.triki@...il.com>
> ---
>   drivers/char/xillybus/xillybus_pcie.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/char/xillybus/xillybus_pcie.c b/drivers/char/xillybus/xillybus_pcie.c
> index 9858711e3e79..fc7bffbd36ed 100644
> --- a/drivers/char/xillybus/xillybus_pcie.c
> +++ b/drivers/char/xillybus/xillybus_pcie.c
> @@ -76,7 +76,8 @@ static int xilly_probe(struct pci_dev *pdev,
>   	pci_set_master(pdev);
>   
>   	/* Set up a single MSI interrupt */
> -	if (pci_enable_msi(pdev)) {
> +	rc = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
> +	if (rc < 0) {
>   		dev_err(endpoint->dev,
>   			"Failed to enable MSI interrupts. Aborting.\n");
>   		return -ENODEV;
> @@ -112,6 +113,8 @@ static void xilly_remove(struct pci_dev *pdev)
>   {
>   	struct xilly_endpoint *endpoint = pci_get_drvdata(pdev);
>   
> +	pci_free_irq_vectors(pdev);
> +
>   	xillybus_endpoint_remove(endpoint);
>   }
>