From patchwork Wed Sep 26 04:42:32 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Roland Dreier X-Patchwork-Id: 1507441 Return-Path: X-Original-To: patchwork-linux-rdma@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id CC903DF238 for ; Wed, 26 Sep 2012 04:42:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751365Ab2IZEmk (ORCPT ); Wed, 26 Sep 2012 00:42:40 -0400 Received: from na3sys010aog103.obsmtp.com ([74.125.245.74]:57640 "HELO na3sys010aog103.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1750868Ab2IZEmj (ORCPT ); Wed, 26 Sep 2012 00:42:39 -0400 Received: from mail-pb0-f46.google.com ([209.85.160.46]) (using TLSv1) by na3sys010aob103.postini.com ([74.125.244.12]) with SMTP ID DSNKUGKHvpZJ333TReT+NdAqYRYyGy4CvLPa@postini.com; Tue, 25 Sep 2012 21:42:39 PDT Received: by pbbrr4 with SMTP id rr4so1254828pbb.19 for ; Tue, 25 Sep 2012 21:42:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=purestorage.com; s=google; h=sender:from:to:cc:subject:date:message-id:x-mailer; bh=2qEN55zAJkhHr2gSeE662/ansJbkzHwBZN1//Sm4VBQ=; b=mREhbOFpmcSHIctQvFOQWAn+NyRHSsKaeKZIYltC7ywARY+LwPUZtrv4rEfSqBwS2o GGSsXGsEJaCGT0fH9NcR8qt/wecqZkIGfCeSu+gIkltoX8r5Dk9mlu62kFS57BTM+Flz Dk6IeUAb87ZrR8oYHUmA3qwNWYYF63tZEJv+w= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=sender:from:to:cc:subject:date:message-id:x-mailer :x-gm-message-state; bh=2qEN55zAJkhHr2gSeE662/ansJbkzHwBZN1//Sm4VBQ=; b=cwRFAc5pv4ineJDk3JVssBd1pbFQUOhDmWidD0Tb9mQmbZxNJ9rGltaYSiQ0SVrV4x Uydo5I3ERJgU/PtdrSxJNLsF1Ho6doEmUV5njxd9BNOVmxeP1ETQtUi8IyQlwRfOj53U xj0P4ol12q8tGjhsaFfkyP89Y5+NBkTGck27Wn9h8iRUVMo8JwdObbVqDPx/pYbqJhs3 Tja7D+X4kd2726GB+nFC8P3xYR7SvHPG/bo8rM9JWxYvbF+fRw2AZs2YloIDZb+og/l9 uJBve2YT0DbIyaPe8GVfz8Am2CMP9xljaPUzR83wFOl16VyP3H6z8qcz7ALCXQK0ltHz j32Q== Received: by 10.68.217.202 with SMTP id pa10mr52267166pbc.15.1348634557836; Tue, 25 Sep 2012 21:42:37 -0700 (PDT) Received: from roland-t410s.home.digitalvampire.org (c-69-181-166-205.hsd1.ca.comcast.net. [69.181.166.205]) by mx.google.com with ESMTPS id pi1sm1424451pbb.7.2012.09.25.21.42.36 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 25 Sep 2012 21:42:37 -0700 (PDT) From: Roland Dreier To: Jack Morgenstein Cc: linux-rdma@vger.kernel.org Subject: [PATCH] mlx4_core: Fix crash on uninitialized priv->cmd.slave_sem Date: Tue, 25 Sep 2012 21:42:32 -0700 Message-Id: <1348634552-21047-1-git-send-email-roland@kernel.org> X-Mailer: git-send-email 1.7.10.4 X-Gm-Message-State: ALoCoQlhHRo7RS3/gyIdJ4AtQTBEF8UqbsQRNPuqIFO6jYc5glKFEvxgsVeGzE4vSRAkay6j19L2 Sender: linux-rdma-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-rdma@vger.kernel.org From: Roland Dreier On an SR-IOV master device, __mlx4_init_one() calls mlx4_init_hca() before mlx4_multi_func_init(). However, for unlucky configurations, mlx4_init_hca() might call mlx4_SENSE_PORT() (via mlx4_dev_cap()), and that calls mlx4_cmd_imm() with MLX4_CMD_WRAPPED set. However, on a multifunction device with MLX4_CMD_WRAPPED, __mlx4_cmd() calls into mlx4_slave_cmd(), and that immediately tries to do down(&priv->cmd.slave_sem); but priv->cmd.slave_sem isn't initialized until mlx4_multi_func_init() (which we haven't called yet). The next thing it tries to do is access priv->mfunc.vhcr, but that hasn't been allocated yet. Fix this by moving the initialization of slave_sem and vhcr up into mlx4_cmd_init(). Also, since slave_sem is really just being used as a mutex, convert it into a slave_cmd_mutex. Signed-off-by: Roland Dreier Acked-by: Jack Morgenstein --- Jack, I needed this to get my (CX3 w/ FW 2.11.500) adapter to work in SR-IOV mode. Is it possible you never tested SR-IOV on an adapter with ports in autosensing mode? drivers/net/ethernet/mellanox/mlx4/cmd.c | 51 ++++++++++++++++++----------- drivers/net/ethernet/mellanox/mlx4/main.c | 11 ++++--- drivers/net/ethernet/mellanox/mlx4/mlx4.h | 2 +- 3 files changed, 38 insertions(+), 26 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c index 90774b7..3d1899f 100644 --- a/drivers/net/ethernet/mellanox/mlx4/cmd.c +++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c @@ -395,7 +395,8 @@ static int mlx4_slave_cmd(struct mlx4_dev *dev, u64 in_param, u64 *out_param, struct mlx4_vhcr_cmd *vhcr = priv->mfunc.vhcr; int ret; - down(&priv->cmd.slave_sem); + mutex_lock(&priv->cmd.slave_cmd_mutex); + vhcr->in_param = cpu_to_be64(in_param); vhcr->out_param = out_param ? cpu_to_be64(*out_param) : 0; vhcr->in_modifier = cpu_to_be32(in_modifier); @@ -403,6 +404,7 @@ static int mlx4_slave_cmd(struct mlx4_dev *dev, u64 in_param, u64 *out_param, vhcr->token = cpu_to_be16(CMD_POLL_TOKEN); vhcr->status = 0; vhcr->flags = !!(priv->cmd.use_events) << 6; + if (mlx4_is_master(dev)) { ret = mlx4_master_process_vhcr(dev, dev->caps.function, vhcr); if (!ret) { @@ -439,7 +441,8 @@ static int mlx4_slave_cmd(struct mlx4_dev *dev, u64 in_param, u64 *out_param, mlx4_err(dev, "failed execution of VHCR_POST command" "opcode 0x%x\n", op); } - up(&priv->cmd.slave_sem); + + mutex_unlock(&priv->cmd.slave_cmd_mutex); return ret; } @@ -1559,14 +1562,15 @@ static void mlx4_master_do_cmd(struct mlx4_dev *dev, int slave, u8 cmd, if ((slave_state[slave].last_cmd != MLX4_COMM_CMD_VHCR_EN) && (slave_state[slave].last_cmd != MLX4_COMM_CMD_VHCR_POST)) goto reset_slave; - down(&priv->cmd.slave_sem); + + mutex_lock(&priv->cmd.slave_cmd_mutex); if (mlx4_master_process_vhcr(dev, slave, NULL)) { mlx4_err(dev, "Failed processing vhcr for slave:%d," " resetting slave.\n", slave); - up(&priv->cmd.slave_sem); + mutex_unlock(&priv->cmd.slave_cmd_mutex); goto reset_slave; } - up(&priv->cmd.slave_sem); + mutex_unlock(&priv->cmd.slave_cmd_mutex); break; default: mlx4_warn(dev, "Bad comm cmd:%d from slave:%d\n", cmd, slave); @@ -1707,14 +1711,6 @@ int mlx4_multi_func_init(struct mlx4_dev *dev) struct mlx4_slave_state *s_state; int i, j, err, port; - priv->mfunc.vhcr = dma_alloc_coherent(&(dev->pdev->dev), PAGE_SIZE, - &priv->mfunc.vhcr_dma, - GFP_KERNEL); - if (!priv->mfunc.vhcr) { - mlx4_err(dev, "Couldn't allocate vhcr.\n"); - return -ENOMEM; - } - if (mlx4_is_master(dev)) priv->mfunc.comm = ioremap(pci_resource_start(dev->pdev, priv->fw.comm_bar) + @@ -1777,7 +1773,6 @@ int mlx4_multi_func_init(struct mlx4_dev *dev) if (mlx4_init_resource_tracker(dev)) goto err_thread; - sema_init(&priv->cmd.slave_sem, 1); err = mlx4_ARM_COMM_CHANNEL(dev); if (err) { mlx4_err(dev, " Failed to arm comm channel eq: %x\n", @@ -1791,8 +1786,6 @@ int mlx4_multi_func_init(struct mlx4_dev *dev) mlx4_err(dev, "Couldn't sync toggles\n"); goto err_comm; } - - sema_init(&priv->cmd.slave_sem, 1); } return 0; @@ -1822,6 +1815,7 @@ int mlx4_cmd_init(struct mlx4_dev *dev) struct mlx4_priv *priv = mlx4_priv(dev); mutex_init(&priv->cmd.hcr_mutex); + mutex_init(&priv->cmd.slave_cmd_mutex); sema_init(&priv->cmd.poll_sem, 1); priv->cmd.use_events = 0; priv->cmd.toggle = 1; @@ -1838,14 +1832,30 @@ int mlx4_cmd_init(struct mlx4_dev *dev) } } + if (mlx4_is_mfunc(dev)) { + priv->mfunc.vhcr = dma_alloc_coherent(&(dev->pdev->dev), PAGE_SIZE, + &priv->mfunc.vhcr_dma, + GFP_KERNEL); + if (!priv->mfunc.vhcr) { + mlx4_err(dev, "Couldn't allocate VHCR.\n"); + goto err_hcr; + } + } + priv->cmd.pool = pci_pool_create("mlx4_cmd", dev->pdev, MLX4_MAILBOX_SIZE, MLX4_MAILBOX_SIZE, 0); if (!priv->cmd.pool) - goto err_hcr; + goto err_vhcr; return 0; +err_vhcr: + if (mlx4_is_mfunc(dev)) + dma_free_coherent(&(dev->pdev->dev), PAGE_SIZE, + priv->mfunc.vhcr, priv->mfunc.vhcr_dma); + priv->mfunc.vhcr = NULL; + err_hcr: if (!mlx4_is_slave(dev)) iounmap(priv->cmd.hcr); @@ -1868,9 +1878,6 @@ void mlx4_multi_func_cleanup(struct mlx4_dev *dev) } iounmap(priv->mfunc.comm); - dma_free_coherent(&(dev->pdev->dev), PAGE_SIZE, - priv->mfunc.vhcr, priv->mfunc.vhcr_dma); - priv->mfunc.vhcr = NULL; } void mlx4_cmd_cleanup(struct mlx4_dev *dev) @@ -1881,6 +1888,10 @@ void mlx4_cmd_cleanup(struct mlx4_dev *dev) if (!mlx4_is_slave(dev)) iounmap(priv->cmd.hcr); + if (mlx4_is_mfunc(dev)) + dma_free_coherent(&(dev->pdev->dev), PAGE_SIZE, + priv->mfunc.vhcr, priv->mfunc.vhcr_dma); + priv->mfunc.vhcr = NULL; } /* diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c index 8d3b475..9f06e04 100644 --- a/drivers/net/ethernet/mellanox/mlx4/main.c +++ b/drivers/net/ethernet/mellanox/mlx4/main.c @@ -1159,10 +1159,10 @@ static void mlx4_slave_exit(struct mlx4_dev *dev) { struct mlx4_priv *priv = mlx4_priv(dev); - down(&priv->cmd.slave_sem); + mutex_lock(&priv->cmd.slave_cmd_mutex); if (mlx4_comm_cmd(dev, MLX4_COMM_CMD_RESET, 0, MLX4_COMM_TIME)) mlx4_warn(dev, "Failed to close slave function.\n"); - up(&priv->cmd.slave_sem); + mutex_unlock(&priv->cmd.slave_cmd_mutex); } static int map_bf_area(struct mlx4_dev *dev) @@ -1214,7 +1214,7 @@ static int mlx4_init_slave(struct mlx4_dev *dev) u32 slave_read; u32 cmd_channel_ver; - down(&priv->cmd.slave_sem); + mutex_lock(&priv->cmd.slave_cmd_mutex); priv->cmd.max_cmds = 1; mlx4_warn(dev, "Sending reset\n"); ret_from_reset = mlx4_comm_cmd(dev, MLX4_COMM_CMD_RESET, 0, @@ -1263,12 +1263,13 @@ static int mlx4_init_slave(struct mlx4_dev *dev) goto err; if (mlx4_comm_cmd(dev, MLX4_COMM_CMD_VHCR_EN, dma, MLX4_COMM_TIME)) goto err; - up(&priv->cmd.slave_sem); + + mutex_unlock(&priv->cmd.slave_cmd_mutex); return 0; err: mlx4_comm_cmd(dev, MLX4_COMM_CMD_RESET, 0, 0); - up(&priv->cmd.slave_sem); + mutex_unlock(&priv->cmd.slave_cmd_mutex); return -EIO; } diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4.h b/drivers/net/ethernet/mellanox/mlx4/mlx4.h index e5eee31..1bb3e5e 100644 --- a/drivers/net/ethernet/mellanox/mlx4/mlx4.h +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4.h @@ -513,9 +513,9 @@ struct mlx4_cmd { struct pci_pool *pool; void __iomem *hcr; struct mutex hcr_mutex; + struct mutex slave_cmd_mutex; struct semaphore poll_sem; struct semaphore event_sem; - struct semaphore slave_sem; int max_cmds; spinlock_t context_lock; int free_head;