| Message ID | 20240626060703.31652-3-jiawenwu@trustnetic.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: txgbe: fix MSI and INTx interrupts | expand |
On Wed, 26 Jun 2024 14:07:03 +0800 Jiawen Wu wrote: > Moreover, do not free isb resources in .ndo_stop, to avoid reading > memory by a null pointer. Please provide more detail on the sequence of events leading to the null-defer. > pdev->irq = pci_irq_vector(pdev, 0); > + wx->num_q_vectors = 1; this doesn't seem obviously related > > return 0; > } > @@ -2027,6 +2028,9 @@ int wx_setup_isb_resources(struct wx *wx) > { > struct pci_dev *pdev = wx->pdev; > > + if (wx->isb_mem) > + return 0; > + > wx->isb_mem = dma_alloc_coherent(&pdev->dev, > sizeof(u32) * 4, > &wx->isb_dma, > @@ -2050,6 +2054,9 @@ void wx_free_isb_resources(struct wx *wx) > { > struct pci_dev *pdev = wx->pdev; > > + if (!wx->isb_mem) > + return; > + And neither does this. Why do you need to make these function idempotent?
On Fri, Jun 28, 2024 7:44 AM, Jakub Kicinski wrote: > On Wed, 26 Jun 2024 14:07:03 +0800 Jiawen Wu wrote: > > Moreover, do not free isb resources in .ndo_stop, to avoid reading > > memory by a null pointer. > > Please provide more detail on the sequence of events leading to the > null-defer. > > > pdev->irq = pci_irq_vector(pdev, 0); > > + wx->num_q_vectors = 1; > > this doesn't seem obviously related Umm, this is related another fix for MSI/INTx. I'll split it into a separate commit. > > > > > return 0; > > } > > @@ -2027,6 +2028,9 @@ int wx_setup_isb_resources(struct wx *wx) > > { > > struct pci_dev *pdev = wx->pdev; > > > > + if (wx->isb_mem) > > + return 0; > > + > > wx->isb_mem = dma_alloc_coherent(&pdev->dev, > > sizeof(u32) * 4, > > &wx->isb_dma, > > @@ -2050,6 +2054,9 @@ void wx_free_isb_resources(struct wx *wx) > > { > > struct pci_dev *pdev = wx->pdev; > > > > + if (!wx->isb_mem) > > + return; > > + > > And neither does this. Why do you need to make these function > idempotent? This is also to implement txgbe changes without changing the flow of ngbe.
diff --git a/drivers/net/ethernet/wangxun/libwx/wx_lib.c b/drivers/net/ethernet/wangxun/libwx/wx_lib.c index 99f55a3573c8..f098758893b3 100644 --- a/drivers/net/ethernet/wangxun/libwx/wx_lib.c +++ b/drivers/net/ethernet/wangxun/libwx/wx_lib.c @@ -1686,6 +1686,7 @@ static int wx_set_interrupt_capability(struct wx *wx) } pdev->irq = pci_irq_vector(pdev, 0); + wx->num_q_vectors = 1; return 0; } @@ -2027,6 +2028,9 @@ int wx_setup_isb_resources(struct wx *wx) { struct pci_dev *pdev = wx->pdev; + if (wx->isb_mem) + return 0; + wx->isb_mem = dma_alloc_coherent(&pdev->dev, sizeof(u32) * 4, &wx->isb_dma, @@ -2050,6 +2054,9 @@ void wx_free_isb_resources(struct wx *wx) { struct pci_dev *pdev = wx->pdev; + if (!wx->isb_mem) + return; + dma_free_coherent(&pdev->dev, sizeof(u32) * 4, wx->isb_mem, wx->isb_dma); wx->isb_mem = NULL; @@ -2386,7 +2393,8 @@ static void wx_free_all_tx_resources(struct wx *wx) void wx_free_resources(struct wx *wx) { - wx_free_isb_resources(wx); + if (wx->mac.type == wx_mac_em) + wx_free_isb_resources(wx); wx_free_all_rx_resources(wx); wx_free_all_tx_resources(wx); } diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c index ac789ec0091a..15e0fef02aac 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_irq.c @@ -111,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; +} + +static irqreturn_t txgbe_misc_irq_thread_fn(int irq, void *data) { struct txgbe *txgbe = data; struct wx *wx = txgbe->wx; @@ -157,6 +187,7 @@ void txgbe_free_misc_irq(struct txgbe *txgbe) int txgbe_setup_misc_irq(struct txgbe *txgbe) { + unsigned long flags = IRQF_ONESHOT; struct wx *wx = txgbe->wx; int hwirq, err; @@ -170,14 +201,17 @@ int txgbe_setup_misc_irq(struct txgbe *txgbe) irq_create_mapping(txgbe->misc.domain, hwirq); txgbe->misc.chip = txgbe_irq_chip; - if (wx->pdev->msix_enabled) + if (wx->pdev->msix_enabled) { txgbe->misc.irq = wx->msix_entry->vector; - else + } else { txgbe->misc.irq = wx->pdev->irq; + if (!wx->pdev->msi_enabled) + flags |= IRQF_SHARED; + } - err = request_threaded_irq(txgbe->misc.irq, NULL, - txgbe_misc_irq_handle, - IRQF_ONESHOT, + err = request_threaded_irq(txgbe->misc.irq, txgbe_misc_irq_handle, + txgbe_misc_irq_thread_fn, + flags, wx->netdev->name, txgbe); if (err) goto del_misc_irq; diff --git a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c index 76b5672c0a17..92c1fae826d0 100644 --- a/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c +++ b/drivers/net/ethernet/wangxun/txgbe/txgbe_main.c @@ -729,6 +729,7 @@ static void txgbe_remove(struct pci_dev *pdev) txgbe_remove_phy(txgbe); txgbe_free_misc_irq(txgbe); + wx_free_isb_resources(wx); pci_release_selected_regions(pdev, pci_select_bars(pdev, IORESOURCE_MEM));
Rename original txgbe_misc_irq_handle() to txgbe_misc_irq_thread_fn() since it is the handle thread to wake up. And add the primary handler to deal the case of MSI/INTx, because there is a schedule NAPI poll. Moreover, do not free isb resources in .ndo_stop, to avoid reading memory by a null pointer. Fixes: aefd013624a1 ("net: txgbe: use irq_domain for interrupt controller") Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com> --- drivers/net/ethernet/wangxun/libwx/wx_lib.c | 10 ++++- .../net/ethernet/wangxun/txgbe/txgbe_irq.c | 44 ++++++++++++++++--- .../net/ethernet/wangxun/txgbe/txgbe_main.c | 1 + 3 files changed, 49 insertions(+), 6 deletions(-)