From owner-svn-src-head@freebsd.org Thu May 30 13:39:27 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 0D91315C33BA; Thu, 30 May 2019 13:39:27 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 7840E8B102; Thu, 30 May 2019 13:39:26 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4423919CB2; Thu, 30 May 2019 13:39:26 +0000 (UTC) (envelope-from mw@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x4UDdQ1d072071; Thu, 30 May 2019 13:39:26 GMT (envelope-from mw@FreeBSD.org) Received: (from mw@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x4UDdP9r072069; Thu, 30 May 2019 13:39:25 GMT (envelope-from mw@FreeBSD.org) Message-Id: <201905301339.x4UDdP9r072069@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: mw set sender to mw@FreeBSD.org using -f From: Marcin Wojtas Date: Thu, 30 May 2019 13:39:25 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r348409 - head/sys/dev/ena X-SVN-Group: head X-SVN-Commit-Author: mw X-SVN-Commit-Paths: head/sys/dev/ena X-SVN-Commit-Revision: 348409 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 7840E8B102 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.98 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.98)[-0.981,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 May 2019 13:39:27 -0000 Author: mw Date: Thu May 30 13:39:25 2019 New Revision: 348409 URL: https://svnweb.freebsd.org/changeset/base/348409 Log: Split ENA reset routine into restore and destroy stages For alignment with Linux driver and better handling ena_detach(), the reset is now calling ena_device_restore() and ena_device_destroy(). The ena_device_destroy() is also being called on ena_detach(), so the code will be more readable. The watchdog is now being activated after reset only, if it was active before. There were added additional checks to ensure, that there is no race with the link state change AENQ handler. Submitted by: Michal Krawczyk Obtained from: Semihalf Sponsored by: Amazon, Inc. Modified: head/sys/dev/ena/ena.c head/sys/dev/ena/ena.h Modified: head/sys/dev/ena/ena.c ============================================================================== --- head/sys/dev/ena/ena.c Thu May 30 13:37:15 2019 (r348408) +++ head/sys/dev/ena/ena.c Thu May 30 13:39:25 2019 (r348409) @@ -2286,11 +2286,6 @@ ena_up(struct ena_adapter *adapter) return (ENXIO); } - if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter))) { - device_printf(adapter->pdev, "device is not running!\n"); - return (ENXIO); - } - if (!ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) { device_printf(adapter->pdev, "device is going UP\n"); @@ -4066,87 +4061,172 @@ ena_timer_service(void *data) } static void -ena_reset_task(void *arg, int pending) +ena_destroy_device(struct ena_adapter *adapter, bool graceful) { - struct ena_com_dev_get_features_ctx get_feat_ctx; - struct ena_adapter *adapter = (struct ena_adapter *)arg; + if_t ifp = adapter->ifp; struct ena_com_dev *ena_dev = adapter->ena_dev; bool dev_up; - int rc; - if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_TRIGGER_RESET, adapter))) { - device_printf(adapter->pdev, - "device reset scheduled but trigger_reset is off\n"); + if (!ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter)) return; - } - sx_xlock(&adapter->ioctl_sx); + if_link_state_change(ifp, LINK_STATE_DOWN); callout_drain(&adapter->timer_service); dev_up = ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter); + if (dev_up) + ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter); + else + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter); - ena_com_set_admin_running_state(ena_dev, false); - ena_down(adapter); + if (!graceful) + ena_com_set_admin_running_state(ena_dev, false); + + if (ENA_FLAG_ISSET(ENA_FLAG_DEV_UP, adapter)) + ena_down(adapter); + + /* + * Stop the device from sending AENQ events (if the device was up, and + * the trigger reset was on, ena_down already performs device reset) + */ + if (!(ENA_FLAG_ISSET(ENA_FLAG_TRIGGER_RESET, adapter) && dev_up)) + ena_com_dev_reset(adapter->ena_dev, adapter->reset_reason); + ena_free_mgmnt_irq(adapter); + ena_disable_msix(adapter); + ena_com_abort_admin_commands(ena_dev); + ena_com_wait_for_abort_completion(ena_dev); + ena_com_admin_destroy(ena_dev); + ena_com_mmio_reg_read_request_destroy(ena_dev); adapter->reset_reason = ENA_REGS_RESET_NORMAL; + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_TRIGGER_RESET, adapter); + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEVICE_RUNNING, adapter); +} - /* Finished destroy part. Restart the device */ - rc = ena_device_init(adapter, adapter->pdev, &get_feat_ctx, - &adapter->wd_active); - if (unlikely(rc != 0)) { +static int +ena_device_validate_params(struct ena_adapter *adapter, + struct ena_com_dev_get_features_ctx *get_feat_ctx) +{ + + if (memcmp(get_feat_ctx->dev_attr.mac_addr, adapter->mac_addr, + ETHER_ADDR_LEN) != 0) { device_printf(adapter->pdev, - "ENA device init failed! (err: %d)\n", rc); - goto err_dev_free; + "Error, mac address are different\n"); + return (EINVAL); } + if (get_feat_ctx->dev_attr.max_mtu < if_getmtu(adapter->ifp)) { + device_printf(adapter->pdev, + "Error, device max mtu is smaller than ifp MTU\n"); + return (EINVAL); + } + + return 0; +} + +static int +ena_restore_device(struct ena_adapter *adapter) +{ + struct ena_com_dev_get_features_ctx get_feat_ctx; + struct ena_com_dev *ena_dev = adapter->ena_dev; + if_t ifp = adapter->ifp; + device_t dev = adapter->pdev; + int wd_active; + int rc; + + ENA_FLAG_SET_ATOMIC(ENA_FLAG_ONGOING_RESET, adapter); + + rc = ena_device_init(adapter, dev, &get_feat_ctx, &wd_active); + if (rc != 0) { + device_printf(dev, "Cannot initialize device\n"); + goto err; + } + /* + * Only enable WD if it was enabled before reset, so it won't override + * value set by the user by the sysctl. + */ + if (adapter->wd_active != 0) + adapter->wd_active = wd_active; + + rc = ena_device_validate_params(adapter, &get_feat_ctx); + if (rc != 0) { + device_printf(dev, "Validation of device parameters failed\n"); + goto err_device_destroy; + } + rc = ena_handle_updated_queues(adapter, &get_feat_ctx); - if (unlikely(rc != 0)) - goto err_dev_free; + if (rc != 0) + goto err_device_destroy; + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_ONGOING_RESET, adapter); + /* Make sure we don't have a race with AENQ Links state handler */ + if (ENA_FLAG_ISSET(ENA_FLAG_LINK_UP, adapter)) + if_link_state_change(ifp, LINK_STATE_UP); + rc = ena_enable_msix_and_set_admin_interrupts(adapter, adapter->num_queues); - if (unlikely(rc != 0)) { - device_printf(adapter->pdev, "Enable MSI-X failed\n"); - goto err_com_free; + if (rc != 0) { + device_printf(dev, "Enable MSI-X failed\n"); + goto err_device_destroy; } /* If the interface was up before the reset bring it up */ - if (dev_up) { + if (ENA_FLAG_ISSET(ENA_FLAG_DEV_UP_BEFORE_RESET, adapter)) { rc = ena_up(adapter); - if (unlikely(rc != 0)) { - device_printf(adapter->pdev, - "Failed to create I/O queues\n"); - goto err_msix_free; + if (rc != 0) { + device_printf(dev, "Failed to create I/O queues\n"); + goto err_disable_msix; } } + ENA_FLAG_SET_ATOMIC(ENA_FLAG_DEVICE_RUNNING, adapter); callout_reset_sbt(&adapter->timer_service, SBT_1S, SBT_1S, ena_timer_service, (void *)adapter, 0); - sx_unlock(&adapter->ioctl_sx); + device_printf(dev, + "Device reset completed successfully, Driver info: %s\n", ena_version); - return; + return (rc); -err_msix_free: +err_disable_msix: ena_free_mgmnt_irq(adapter); ena_disable_msix(adapter); -err_com_free: +err_device_destroy: ena_com_abort_admin_commands(ena_dev); ena_com_wait_for_abort_completion(ena_dev); ena_com_admin_destroy(ena_dev); - ena_com_mmio_reg_read_request_destroy(ena_dev); ena_com_dev_reset(ena_dev, ENA_REGS_RESET_DRIVER_INVALID_STATE); -err_dev_free: - device_printf(adapter->pdev, "ENA reset failed!\n"); + ena_com_mmio_reg_read_request_destroy(ena_dev); +err: ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_DEVICE_RUNNING, adapter); + ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_ONGOING_RESET, adapter); + device_printf(dev, "Reset attempt failed. Can not reset the device\n"); + + return (rc); +} + +static void +ena_reset_task(void *arg, int pending) +{ + struct ena_adapter *adapter = (struct ena_adapter *)arg; + + if (unlikely(!ENA_FLAG_ISSET(ENA_FLAG_TRIGGER_RESET, adapter))) { + device_printf(adapter->pdev, + "device reset scheduled but trigger_reset is off\n"); + return; + } + + sx_xlock(&adapter->ioctl_sx); + ena_destroy_device(adapter, false); + ena_restore_device(adapter); sx_unlock(&adapter->ioctl_sx); } @@ -4403,6 +4483,7 @@ ena_detach(device_t pdev) sx_xlock(&adapter->ioctl_sx); ena_down(adapter); + ena_destroy_device(adapter, true); sx_unlock(&adapter->ioctl_sx); ena_free_all_io_rings_resources(adapter); @@ -4412,9 +4493,6 @@ ena_detach(device_t pdev) ena_free_counters((counter_u64_t *)&adapter->dev_stats, sizeof(struct ena_stats_dev)); - if (likely(ENA_FLAG_ISSET(ENA_FLAG_RSS_ACTIVE, adapter))) - ena_com_rss_destroy(ena_dev); - rc = ena_free_rx_dma_tag(adapter); if (unlikely(rc != 0)) device_printf(adapter->pdev, @@ -4425,24 +4503,15 @@ ena_detach(device_t pdev) device_printf(adapter->pdev, "Unmapped TX DMA tag associations\n"); - /* Reset the device only if the device is running. */ - if (ENA_FLAG_ISSET(ENA_FLAG_DEVICE_RUNNING, adapter)) - ena_com_dev_reset(ena_dev, adapter->reset_reason); - - ena_com_delete_host_info(ena_dev); - ena_free_irqs(adapter); - ena_com_abort_admin_commands(ena_dev); + ena_free_pci_resources(adapter); - ena_com_wait_for_abort_completion(ena_dev); + if (likely(ENA_FLAG_ISSET(ENA_FLAG_RSS_ACTIVE, adapter))) + ena_com_rss_destroy(ena_dev); - ena_com_admin_destroy(ena_dev); + ena_com_delete_host_info(ena_dev); - ena_com_mmio_reg_read_request_destroy(ena_dev); - - ena_free_pci_resources(adapter); - mtx_destroy(&adapter->global_mtx); sx_destroy(&adapter->ioctl_sx); @@ -4480,15 +4549,13 @@ ena_update_on_link_change(void *adapter_data, if (status != 0) { device_printf(adapter->pdev, "link is UP\n"); - if_link_state_change(ifp, LINK_STATE_UP); ENA_FLAG_SET_ATOMIC(ENA_FLAG_LINK_UP, adapter); - } else if (status == 0) { + if (!ENA_FLAG_ISSET(ENA_FLAG_ONGOING_RESET, adapter)) + if_link_state_change(ifp, LINK_STATE_UP); + } else { device_printf(adapter->pdev, "link is DOWN\n"); if_link_state_change(ifp, LINK_STATE_DOWN); ENA_FLAG_CLEAR_ATOMIC(ENA_FLAG_LINK_UP, adapter); - } else { - device_printf(adapter->pdev, "invalid value recvd\n"); - BUG(); } } Modified: head/sys/dev/ena/ena.h ============================================================================== --- head/sys/dev/ena/ena.h Thu May 30 13:37:15 2019 (r348408) +++ head/sys/dev/ena/ena.h Thu May 30 13:39:25 2019 (r348409) @@ -164,6 +164,7 @@ enum ena_flags_t { ENA_FLAG_MSIX_ENABLED, ENA_FLAG_TRIGGER_RESET, ENA_FLAG_ONGOING_RESET, + ENA_FLAG_DEV_UP_BEFORE_RESET, ENA_FLAG_RSS_ACTIVE, ENA_FLAGS_NUMBER = ENA_FLAG_RSS_ACTIVE };