Message-ID: <20210826221916.127243-1-pwaskiewicz@jumptrading.com>
Date:   Thu, 26 Aug 2021 17:19:16 -0500
From:   PJ Waskiewicz <pwaskiewicz@...ptrading.com>
To:     <intel-wired-lan@...ts.osuosl.org>,
        Tony Nguyen <anthony.l.nguyen@...el.com>
CC:     "David S . Miller" <davem@...emloft.net>,
        Jesse Brandeburg <jesse.brandeburg@...el.com>,
        <netdev@...r.kernel.org>, PJ Waskiewicz <pjwaskiewicz@...il.com>,
        PJ Waskiewicz <pwaskiewicz@...ptrading.com>,
        Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Subject: [PATCH 1/1] i40e: Avoid double IRQ free on error path in probe()

This fixes an error path condition when probe() fails due to the
default VSI not being available or online yet in the firmware. If
that happens, the previous teardown path would clear the interrupt
scheme, which also freed the IRQs with the OS. Then the error path
for the switch setup (pre-VSI) would attempt to free the OS IRQs
as well.

[   14.597121] i40e 0000:31:00.0: setup of MAIN VSI failed
[   14.712167] i40e 0000:31:00.0: setup_pf_switch failed: -11
[   14.755318] ------------[ cut here ]------------
[   14.766261] Trying to free already-free IRQ 266
[   14.777224] WARNING: CPU: 0 PID: 5 at kernel/irq/manage.c:1731 __free_irq+0x9a/0x300
[   14.791341] Modules linked in: XXXXXX
[   14.825361] CPU: 0 PID: 5 Comm: kworker/0:0 Not tainted <kernel omitted>
[   14.840630] Hardware name: XXXXXX
[   14.854924] Workqueue: events work_for_cpu_fn
[   14.866482] RIP: 0010:__free_irq+0x9a/0x300
[   14.877638] Code: 08 75 0e e9 3c 02 00 00 4c 39 6b 08 74 59 48 89 da 48 8b 5a 18 48 85 db 75 ee 8b 74 24 04 48 c7 c7 58 a0 aa b4 e8 3f 2e f9 ff <0f> 0b 4c 89 f6 48 89 ef e8 f9 69 7b 00 49 8b 47 40 48 8b 80 80 00
[   14.910571] RSP: 0000:ff6a6ad7401dfb60 EFLAGS: 00010086
[   14.923265] RAX: 0000000000000000 RBX: ff3c97328eb56000 RCX: 0000000000000006
[   14.937853] RDX: 0000000000000007 RSI: 0000000000000092 RDI: ff3c97333ee16a00
[   14.952290] RBP: ff3c9731cff4caa4 R08: 00000000000006b8 R09: 0000000000aaaaaa
[   14.966781] R10: 0000000000000000 R11: ff6a6ad768aff020 R12: ff3c9731cff4cb80
[   14.981436] R13: ff3c97328eb56000 R14: 0000000000000246 R15: ff3c9731cff4ca00
[   14.996041] FS:  0000000000000000(0000) GS:ff3c97333ee00000(0000) knlGS:0000000000000000
[   15.011511] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   15.024493] CR2: 00007fb6ac002000 CR3: 00000004f8c0a001 CR4: 0000000000761ef0
[   15.039373] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   15.054426] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   15.068781] PKRU: 55555554
[   15.078902] Call Trace:
[   15.088421]  ? synchronize_irq+0x3a/0xa0
[   15.099556]  free_irq+0x2e/0x60
[   15.109863]  i40e_clear_interrupt_scheme+0x53/0x190 [i40e]
[   15.122718]  i40e_probe.part.108+0x134b/0x1a40 [i40e]
[   15.135343]  ? kmem_cache_alloc+0x158/0x1c0
[   15.146688]  ? acpi_ut_update_ref_count.part.1+0x8e/0x345
[   15.159217]  ? acpi_ut_update_object_reference+0x15e/0x1e2
[   15.171879]  ? strstr+0x21/0x70
[   15.181802]  ? irq_get_irq_data+0xa/0x20
[   15.193198]  ? mp_check_pin_attr+0x13/0xc0
[   15.203909]  ? irq_get_irq_data+0xa/0x20
[   15.214310]  ? mp_map_pin_to_irq+0xd3/0x2f0
[   15.225206]  ? acpi_register_gsi_ioapic+0x93/0x170
[   15.236351]  ? pci_conf1_read+0xa4/0x100
[   15.246586]  ? pci_bus_read_config_word+0x49/0x70
[   15.257608]  ? do_pci_enable_device+0xcc/0x100
[   15.268337]  local_pci_probe+0x41/0x90
[   15.279016]  work_for_cpu_fn+0x16/0x20
[   15.289545]  process_one_work+0x1a7/0x360
[   15.300214]  worker_thread+0x1cf/0x390
[   15.309980]  ? create_worker+0x1a0/0x1a0
[   15.319854]  kthread+0x112/0x130
[   15.328806]  ? kthread_flush_work_fn+0x10/0x10
[   15.338739]  ret_from_fork+0x1f/0x40
[   15.347622] ---[ end trace 5220551832349274 ]---

Break apart the clear and reset schemes so that clear no longer
calls i40_reset_interrupt_capability(), allowing that to be called
across error paths in probe().

Fixes: 41c445ff0f482 ("i40e: main driver core")
Signed-off-by: PJ Waskiewicz <pwaskiewicz@...ptrading.com>
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 1d1f52756a93..b1cbd0eae83c 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -4862,7 +4862,8 @@ static void i40e_reset_interrupt_capability(struct i40e_pf *pf)
  * @pf: board private structure
  *
  * We go through and clear interrupt specific resources and reset the structure
- * to pre-load conditions
+ * to pre-load conditions.  OS interrupt teardown must be done separately due
+ * to VSI vs PF instantiation, and different teardown path requirements.
  **/
 static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
 {
@@ -4877,7 +4878,6 @@ static void i40e_clear_interrupt_scheme(struct i40e_pf *pf)
        for (i = 0; i < pf->num_alloc_vsi; i++)
                if (pf->vsi[i])
                        i40e_vsi_free_q_vectors(pf->vsi[i]);
-       i40e_reset_interrupt_capability(pf);
 }

 /**
@@ -10523,6 +10523,7 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
                         */
                        free_irq(pf->pdev->irq, pf);
                        i40e_clear_interrupt_scheme(pf);
+                       i40e_reset_interrupt_capability(pf);
                        if (i40e_restore_interrupt_scheme(pf))
                                goto end_unlock;
                }
@@ -15908,6 +15909,7 @@ static void i40e_remove(struct pci_dev *pdev)
        /* Clear all dynamic memory lists of rings, q_vectors, and VSIs */
        rtnl_lock();
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);
        for (i = 0; i < pf->num_alloc_vsi; i++) {
                if (pf->vsi[i]) {
                        if (!test_bit(__I40E_RECOVERY_MODE, pf->state))
@@ -16130,6 +16132,7 @@ static void i40e_shutdown(struct pci_dev *pdev)
         */
        rtnl_lock();
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);
        rtnl_unlock();

        if (system_state == SYSTEM_POWER_OFF) {
@@ -16182,6 +16185,7 @@ static int __maybe_unused i40e_suspend(struct device *dev)
         * to CPU0.
         */
        i40e_clear_interrupt_scheme(pf);
+       i40e_reset_interrupt_capability(pf);

        rtnl_unlock();

--
2.27.0


________________________________

Note: This email is for the confidential use of the named addressee(s) only and may contain proprietary, confidential, or privileged information and/or personal data. If you are not the intended recipient, you are hereby notified that any review, dissemination, or copying of this email is strictly prohibited, and requested to notify the sender immediately and destroy this email and any attachments. Email transmission cannot be guaranteed to be secure or error-free. The Company, therefore, does not make any guarantees as to the completeness or accuracy of this email or any attachments. This email is for informational purposes only and does not constitute a recommendation, offer, request, or solicitation of any kind to buy, sell, subscribe, redeem, or perform any type of transaction of a financial product. Personal data, as defined by applicable data protection and privacy laws, contained in this email may be processed by the Company, and any of its affiliated or related companies, for legal, compliance, and/or business-related purposes. You may have rights regarding your personal data; for information on exercising these rights or the Company’s treatment of personal data, please email datarequests@...ptrading.com.

Powered by blists - more mailing lists