netdev - Re: [PATCH net] net: txgbe: fix MSI and INTx interrupts

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: <08f3db6b-6781-435c-bddb-04a594a2e617@intel.com>
Date: Tue, 25 Jun 2024 13:40:57 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Jiawen Wu <jiawenwu@...stnetic.com>
CC: <mengyuanlou@...-swift.com>, <duanqiangwen@...-swift.com>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <horms@...nel.org>, <andrew@...n.ch>,
	<netdev@...r.kernel.org>
Subject: Re: [PATCH net] net: txgbe: fix MSI and INTx interrupts

On 6/21/24 10:09, Jiawen Wu wrote:
> When using MSI or INTx interrupts, request_irq() for pdev->irq will
> conflict with request_threaded_irq() for txgbe->misc.irq, to cause
> system crash.
> 
> Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller")
> Signed-off-by: Jiawen Wu <jiawenwu@...stnetic.com>
> ---
>   drivers/net/ethernet/wangxun/libwx/wx_lib.c   |  13 +-
>   .../net/ethernet/wangxun/txgbe/txgbe_irq.c    | 122 +++++++-----------
>   .../net/ethernet/wangxun/txgbe/txgbe_irq.h    |   2 +-
>   .../net/ethernet/wangxun/txgbe/txgbe_main.c   |   3 +-
>   4 files changed, 59 insertions(+), 81 deletions(-)
> 

Please split into two commits (by prepending one commit that will just
move/rename function/s) to avoid inflating the diff of the actual fix.
This will ease the review process.

[...]

> diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> index b3e3605d1edb..15e0fef02aac 100644
> --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c
> @@ -27,57 +27,19 @@ void txgbe_irq_enable(struct wx *wx, bool queues)
>   }
>   
>   /**
> - * txgbe_intr - msi/legacy mode Interrupt Handler
> - * @irq: interrupt number
> - * @data: pointer to a network interface device structure
> - **/
> -static irqreturn_t txgbe_intr(int __always_unused irq, void *data)
> -{
> -	struct wx_q_vector *q_vector;
> -	struct wx *wx  = data;
> -	struct pci_dev *pdev;
> -	u32 eicr;
> -
> -	q_vector = wx->q_vector[0];
> -	pdev = wx->pdev;
> -
> -	eicr = wx_misc_isb(wx, WX_ISB_VEC0);
> -	if (!eicr) {
> -		/* shared interrupt alert!
> -		 * the interrupt that we masked before the ICR read.
> -		 */
> -		if (netif_running(wx->netdev))
> -			txgbe_irq_enable(wx, true);
> -		return IRQ_NONE;        /* Not our interrupt */
> -	}
> -	wx->isb_mem[WX_ISB_VEC0] = 0;
> -	if (!(pdev->msi_enabled))
> -		wr32(wx, WX_PX_INTA, 1);
> -
> -	wx->isb_mem[WX_ISB_MISC] = 0;
> -	/* would disable interrupts here but it is auto disabled */
> -	napi_schedule_irqoff(&q_vector->napi);
> -
> -	/* re-enable link(maybe) and non-queue interrupts, no flush.
> -	 * txgbe_poll will re-enable the queue interrupts
> -	 */
> -	if (netif_running(wx->netdev))
> -		txgbe_irq_enable(wx, false);
> -
> -	return IRQ_HANDLED;
> -}
> -

[...]

> -
>   static int txgbe_request_gpio_irq(struct txgbe *txgbe)
>   {
>   	txgbe->gpio_irq = irq_find_mapping(txgbe->misc.domain, TXGBE_IRQ_GPIO);
> @@ -177,6 +111,36 @@ static const struct irq_domain_ops txgbe_misc_irq_domain_ops = {
>   };
>   
>   static irqreturn_t txgbe_misc_irq_handle(int irq, void *data)
> +{
> +	struct wx_q_vector *q_vector;
> +	struct txgbe *txgbe = data;
> +	struct wx *wx = txgbe->wx;
> +	u32 eicr;
> +
> +	if (wx->pdev->msix_enabled)
> +		return IRQ_WAKE_THREAD;
> +
> +	eicr = wx_misc_isb(wx, WX_ISB_VEC0);
> +	if (!eicr) {
> +		/* shared interrupt alert!
> +		 * the interrupt that we masked before the ICR read.
> +		 */
> +		if (netif_running(wx->netdev))
> +			txgbe_irq_enable(wx, true);
> +		return IRQ_NONE;        /* Not our interrupt */
> +	}
> +	wx->isb_mem[WX_ISB_VEC0] = 0;
> +	if (!(wx->pdev->msi_enabled))
> +		wr32(wx, WX_PX_INTA, 1);
> +
> +	/* would disable interrupts here but it is auto disabled */
> +	q_vector = wx->q_vector[0];
> +	napi_schedule_irqoff(&q_vector->napi);
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +

[...]