From owner-svn-src-all@freebsd.org Wed May 8 11:08:50 2019 Return-Path: Delivered-To: svn-src-all@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 BE13A15A6F5F; Wed, 8 May 2019 11:08:49 +0000 (UTC) (envelope-from hselasky@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 65D216DE97; Wed, 8 May 2019 11:08:49 +0000 (UTC) (envelope-from hselasky@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 393904F69; Wed, 8 May 2019 11:08:49 +0000 (UTC) (envelope-from hselasky@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x48B8nGZ064143; Wed, 8 May 2019 11:08:49 GMT (envelope-from hselasky@FreeBSD.org) Received: (from hselasky@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x48B8m1R064141; Wed, 8 May 2019 11:08:48 GMT (envelope-from hselasky@FreeBSD.org) Message-Id: <201905081108.x48B8m1R064141@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: hselasky set sender to hselasky@FreeBSD.org using -f From: Hans Petter Selasky Date: Wed, 8 May 2019 11:08:48 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r347323 - in head/sys/dev/mlx5: . mlx5_core X-SVN-Group: head X-SVN-Commit-Author: hselasky X-SVN-Commit-Paths: in head/sys/dev/mlx5: . mlx5_core X-SVN-Commit-Revision: 347323 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: 65D216DE97 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)[-0.999,0]; NEURAL_HAM_SHORT(-0.98)[-0.976,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 May 2019 11:08:50 -0000 Author: hselasky Date: Wed May 8 11:08:48 2019 New Revision: 347323 URL: https://svnweb.freebsd.org/changeset/base/347323 Log: Fix race between driver unload and dumping firmware in mlx5core. Present code uses lock-less accesses to the dump data to prevent top level ioctls from blocking bottom-level call to dump. Unfortunately, this depends on the type stability of the dump data structure, which makes it non-functional during driver teardown. Switch to the mutex locking scheme where top levels use the mutex in the bound regions, while copyouts and drain for completion utilize condvars. The mutex lifetime is guaranteed to be strictly larger than the time interval where driver can initiate dump, and most of the control fields of the old struct mlx5_dump_data are directly embedded into struct mlx5_core_dev. Submitted by: kib@ MFC after: 3 days Sponsored by: Mellanox Technologies Modified: head/sys/dev/mlx5/driver.h head/sys/dev/mlx5/mlx5_core/mlx5_fwdump.c head/sys/dev/mlx5/mlx5_core/mlx5_main.c Modified: head/sys/dev/mlx5/driver.h ============================================================================== --- head/sys/dev/mlx5/driver.h Wed May 8 11:08:21 2019 (r347322) +++ head/sys/dev/mlx5/driver.h Wed May 8 11:08:48 2019 (r347323) @@ -664,7 +664,6 @@ struct mlx5_special_contexts { }; struct mlx5_flow_root_namespace; -struct mlx5_dump_data; struct mlx5_core_dev { struct pci_dev *pdev; /* sync pci state */ @@ -704,7 +703,12 @@ struct mlx5_core_dev { struct mlx5_flow_root_namespace *sniffer_rx_root_ns; struct mlx5_flow_root_namespace *sniffer_tx_root_ns; u32 num_q_counter_allocated[MLX5_INTERFACE_NUMBER]; - struct mlx5_dump_data *dump_data; + const struct mlx5_crspace_regmap *dump_rege; + uint32_t *dump_data; + unsigned dump_size; + bool dump_valid; + bool dump_copyout; + struct mtx dump_lock; struct sysctl_ctx_list sysctl_ctx; int msix_eqvec; Modified: head/sys/dev/mlx5/mlx5_core/mlx5_fwdump.c ============================================================================== --- head/sys/dev/mlx5/mlx5_core/mlx5_fwdump.c Wed May 8 11:08:21 2019 (r347322) +++ head/sys/dev/mlx5/mlx5_core/mlx5_fwdump.c Wed May 8 11:08:48 2019 (r347323) @@ -39,14 +39,6 @@ extern const struct mlx5_crspace_regmap mlx5_crspace_r extern const struct mlx5_crspace_regmap mlx5_crspace_regmap_mt4115[]; extern const struct mlx5_crspace_regmap mlx5_crspace_regmap_connectx5[]; -struct mlx5_dump_data { - const struct mlx5_crspace_regmap *rege; - uint32_t *dump; - unsigned dump_size; - int dump_valid; - struct mtx dump_lock; -}; - static MALLOC_DEFINE(M_MLX5_DUMP, "MLX5DUMP", "MLX5 Firmware dump"); static unsigned @@ -61,20 +53,20 @@ mlx5_fwdump_getsize(const struct mlx5_crspace_regmap * } static void -mlx5_fwdump_destroy_dd(struct mlx5_dump_data *dd) +mlx5_fwdump_destroy_dd(struct mlx5_core_dev *mdev) { - mtx_destroy(&dd->dump_lock); - free(dd->dump, M_MLX5_DUMP); - free(dd, M_MLX5_DUMP); + mtx_assert(&mdev->dump_lock, MA_OWNED); + free(mdev->dump_data, M_MLX5_DUMP); + mdev->dump_data = NULL; } void mlx5_fwdump_prep(struct mlx5_core_dev *mdev) { - struct mlx5_dump_data *dd; int error; + mdev->dump_data = NULL; error = mlx5_vsc_find_cap(mdev); if (error != 0) { /* Inability to create a firmware dump is not fatal. */ @@ -82,47 +74,39 @@ mlx5_fwdump_prep(struct mlx5_core_dev *mdev) "mlx5_fwdump_prep failed %d\n", error); return; } - dd = malloc(sizeof(struct mlx5_dump_data), M_MLX5_DUMP, M_WAITOK); switch (pci_get_device(mdev->pdev->dev.bsddev)) { case 0x1013: - dd->rege = mlx5_crspace_regmap_mt4115; + mdev->dump_rege = mlx5_crspace_regmap_mt4115; break; case 0x1015: - dd->rege = mlx5_crspace_regmap_mt4117; + mdev->dump_rege = mlx5_crspace_regmap_mt4117; break; case 0x1017: case 0x1019: - dd->rege = mlx5_crspace_regmap_connectx5; + mdev->dump_rege = mlx5_crspace_regmap_connectx5; break; default: - free(dd, M_MLX5_DUMP); return; /* silently fail, do not prevent driver attach */ } - dd->dump_size = mlx5_fwdump_getsize(dd->rege); - dd->dump = malloc(dd->dump_size * sizeof(uint32_t), M_MLX5_DUMP, - M_WAITOK | M_ZERO); - dd->dump_valid = 0; - mtx_init(&dd->dump_lock, "mlx5dmp", NULL, MTX_DEF | MTX_NEW); - if (atomic_cmpset_rel_ptr((uintptr_t *)&mdev->dump_data, 0, - (uintptr_t)dd) == 0) - mlx5_fwdump_destroy_dd(dd); + mdev->dump_size = mlx5_fwdump_getsize(mdev->dump_rege); + mdev->dump_data = malloc(mdev->dump_size * sizeof(uint32_t), + M_MLX5_DUMP, M_WAITOK | M_ZERO); + mdev->dump_valid = false; + mdev->dump_copyout = false; } void mlx5_fwdump(struct mlx5_core_dev *mdev) { - struct mlx5_dump_data *dd; const struct mlx5_crspace_regmap *r; uint32_t i, ri; int error; dev_info(&mdev->pdev->dev, "Issuing FW dump\n"); - dd = (struct mlx5_dump_data *)atomic_load_acq_ptr((uintptr_t *) - &mdev->dump_data); - if (dd == NULL) - return; - mtx_lock(&dd->dump_lock); - if (dd->dump_valid) { + mtx_lock(&mdev->dump_lock); + if (mdev->dump_data == NULL) + goto failed; + if (mdev->dump_valid) { /* only one dump */ dev_warn(&mdev->pdev->dev, "Only one FW dump can be captured aborting FW dump\n"); @@ -136,37 +120,51 @@ mlx5_fwdump(struct mlx5_core_dev *mdev) error = mlx5_vsc_set_space(mdev, MLX5_VSC_DOMAIN_PROTECTED_CRSPACE); if (error != 0) goto unlock_vsc; - for (i = 0, r = dd->rege; r->cnt != 0; r++) { + for (i = 0, r = mdev->dump_rege; r->cnt != 0; r++) { for (ri = 0; ri < r->cnt; ri++) { error = mlx5_vsc_read(mdev, r->addr + ri * 4, - &dd->dump[i]); + &mdev->dump_data[i]); if (error != 0) goto unlock_vsc; i++; } } - atomic_store_rel_int(&dd->dump_valid, 1); + mdev->dump_valid = true; unlock_vsc: mlx5_vsc_unlock(mdev); failed: - mtx_unlock(&dd->dump_lock); + mtx_unlock(&mdev->dump_lock); } void mlx5_fwdump_clean(struct mlx5_core_dev *mdev) { - struct mlx5_dump_data *dd; - for (;;) { - dd = mdev->dump_data; - if (dd == NULL) - return; - if (atomic_cmpset_ptr((uintptr_t *)&mdev->dump_data, - (uintptr_t)dd, 0) == 1) { - mlx5_fwdump_destroy_dd(dd); - return; + mtx_lock(&mdev->dump_lock); + while (mdev->dump_copyout) + msleep(&mdev->dump_copyout, &mdev->dump_lock, 0, "mlx5fwc", 0); + mlx5_fwdump_destroy_dd(mdev); + mtx_unlock(&mdev->dump_lock); +} + +static int +mlx5_fwdump_reset(struct mlx5_core_dev *mdev) +{ + int error; + + error = 0; + mtx_lock(&mdev->dump_lock); + if (mdev->dump_data != NULL) { + while (mdev->dump_copyout) { + msleep(&mdev->dump_copyout, &mdev->dump_lock, + 0, "mlx5fwr", 0); } + mdev->dump_valid = false; + } else { + error = ENOENT; } + mtx_unlock(&mdev->dump_lock); + return (error); } static int @@ -190,29 +188,37 @@ mlx5_dbsf_to_core(const struct mlx5_tool_addr *devaddr } static int -mlx5_fwdump_copyout(struct mlx5_dump_data *dd, struct mlx5_fwdump_get *fwg) +mlx5_fwdump_copyout(struct mlx5_core_dev *mdev, struct mlx5_fwdump_get *fwg) { const struct mlx5_crspace_regmap *r; struct mlx5_fwdump_reg rv, *urv; uint32_t i, ri; int error; - if (dd == NULL) + mtx_lock(&mdev->dump_lock); + if (mdev->dump_data == NULL) { + mtx_unlock(&mdev->dump_lock); return (ENOENT); + } if (fwg->buf == NULL) { - fwg->reg_filled = dd->dump_size; + fwg->reg_filled = mdev->dump_size; + mtx_unlock(&mdev->dump_lock); return (0); } - if (atomic_load_acq_int(&dd->dump_valid) == 0) + if (!mdev->dump_valid) { + mtx_unlock(&mdev->dump_lock); return (ENOENT); + } + mdev->dump_copyout = true; + mtx_unlock(&mdev->dump_lock); urv = fwg->buf; - for (i = 0, r = dd->rege; r->cnt != 0; r++) { + for (i = 0, r = mdev->dump_rege; r->cnt != 0; r++) { for (ri = 0; ri < r->cnt; ri++) { if (i >= fwg->reg_cnt) goto out; rv.addr = r->addr + ri * 4; - rv.val = dd->dump[i]; + rv.val = mdev->dump_data[i]; error = copyout(&rv, urv, sizeof(rv)); if (error != 0) return (error); @@ -222,6 +228,10 @@ mlx5_fwdump_copyout(struct mlx5_dump_data *dd, struct } out: fwg->reg_filled = i; + mtx_lock(&mdev->dump_lock); + mdev->dump_copyout = false; + wakeup(&mdev->dump_copyout); + mtx_unlock(&mdev->dump_lock); return (0); } @@ -250,7 +260,6 @@ mlx5_ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t d struct mlx5_core_dev *mdev; struct mlx5_fwdump_get *fwg; struct mlx5_tool_addr *devaddr; - struct mlx5_dump_data *dd; struct mlx5_fw_update *fu; struct firmware fake_fw; int error; @@ -267,7 +276,7 @@ mlx5_ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t d error = mlx5_dbsf_to_core(devaddr, &mdev); if (error != 0) break; - error = mlx5_fwdump_copyout(mdev->dump_data, fwg); + error = mlx5_fwdump_copyout(mdev, fwg); break; case MLX5_FWDUMP_RESET: if ((fflag & FWRITE) == 0) { @@ -276,13 +285,8 @@ mlx5_ctl_ioctl(struct cdev *dev, u_long cmd, caddr_t d } devaddr = (struct mlx5_tool_addr *)data; error = mlx5_dbsf_to_core(devaddr, &mdev); - if (error != 0) - break; - dd = mdev->dump_data; - if (dd != NULL) - atomic_store_rel_int(&dd->dump_valid, 0); - else - error = ENOENT; + if (error == 0) + error = mlx5_fwdump_reset(mdev); break; case MLX5_FWDUMP_FORCE: if ((fflag & FWRITE) == 0) { Modified: head/sys/dev/mlx5/mlx5_core/mlx5_main.c ============================================================================== --- head/sys/dev/mlx5/mlx5_core/mlx5_main.c Wed May 8 11:08:21 2019 (r347322) +++ head/sys/dev/mlx5/mlx5_core/mlx5_main.c Wed May 8 11:08:48 2019 (r347323) @@ -1301,6 +1301,7 @@ static int init_one(struct pci_dev *pdev, spin_lock_init(&priv->ctx_lock); mutex_init(&dev->pci_status_mutex); mutex_init(&dev->intf_state_mutex); + mtx_init(&dev->dump_lock, "mlx5dmp", NULL, MTX_DEF | MTX_NEW); err = mlx5_pci_init(dev, priv); if (err) { device_printf(bsddev, "ERR: mlx5_pci_init failed %d\n", err); @@ -1335,6 +1336,7 @@ close_pci: mlx5_pci_close(dev, priv); clean_dev: sysctl_ctx_free(&dev->sysctl_ctx); + mtx_destroy(&dev->dump_lock); kfree(dev); return err; } @@ -1350,10 +1352,11 @@ static void remove_one(struct pci_dev *pdev) return; } - mlx5_fwdump_clean(dev); mlx5_pagealloc_cleanup(dev); mlx5_health_cleanup(dev); + mlx5_fwdump_clean(dev); mlx5_pci_close(dev, priv); + mtx_destroy(&dev->dump_lock); pci_set_drvdata(pdev, NULL); sysctl_ctx_free(&dev->sysctl_ctx); kfree(dev);