netdev - Re: [PATCH net] net: amd-xgbe: disable interrupts during pci removal

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] [day] [month] [year] [list]

Date:   Wed, 9 Feb 2022 08:12:13 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Raju Rangoju <Raju.Rangoju@....com>, davem@...emloft.net,
        kuba@...nel.org
Cc:     netdev@...r.kernel.org, Shyam-sundar.S-k@....com,
        Selwin Sebastian <Selwin.Sebastian@....com>
Subject: Re: [PATCH net] net: amd-xgbe: disable interrupts during pci removal

On 2/8/22 22:32, Raju Rangoju wrote:
> Hardware interrupts are enabled during the pci probe, however,
> they are not disabled during pci removal.
> 
> Disable all hardware interrupts during pci removal to avoid any
> issues.

Are there issues? I would expect the change log to say what the issues are 
and hence the reason for making this change.

> 
> Fixes: e75377404726 ("amd-xgbe: Update PCI support to use new IRQ functions")

Is this the right fixes tag? The XP_IOWRITE() that sets XP_INT_EN was 
introduced in: 47f164deab22 ("amd-xgbe: Add PCI device support"). If the 
idea is to be certain interrupts are not enabled for the device after 
removal, wouldn't that really be the tag to use?

Or was the introduction of using pci_free_irq_vectors() in combination 
with XP_IN_EN not being zeroed the true cause?

> Suggested-by: Selwin Sebastian <Selwin.Sebastian@....com>
> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com>
> ---
>   drivers/net/ethernet/amd/xgbe/xgbe-pci.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> index efdcf484a510..2af3da4b2d05 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-pci.c
> @@ -425,6 +425,9 @@ static void xgbe_pci_remove(struct pci_dev *pdev)
>   
>   	pci_free_irq_vectors(pdata->pcidev);
>   
> +	/* Disable all interrupts in the hardware */
> +	XP_IOWRITE(pdata, XP_INT_EN, 0x0);
> +

Shouldn't this be done before calling pci_free_irq_vectors().

Thanks,
Tom

>   	xgbe_free_pdata(pdata);
>   }
>