Patch Detail
get:
Show a patch.
patch:
Update a patch.
put:
Update a patch.
GET /api/patches/730824/?format=api
{ "id": 730824, "url": "http://patchwork.ozlabs.org/api/patches/730824/?format=api", "web_url": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1487721348-25617-5-git-send-email-bimmy.pujari@intel.com/", "project": { "id": 46, "url": "http://patchwork.ozlabs.org/api/projects/46/?format=api", "name": "Intel Wired Ethernet development", "link_name": "intel-wired-lan", "list_id": "intel-wired-lan.osuosl.org", "list_email": "intel-wired-lan@osuosl.org", "web_url": "", "scm_url": "", "webscm_url": "", "list_archive_url": "", "list_archive_url_format": "", "commit_url_format": "" }, "msgid": "<1487721348-25617-5-git-send-email-bimmy.pujari@intel.com>", "list_archive_url": null, "date": "2017-02-21T23:55:42", "name": "[next,S61,04/10] i40e: Fixed race conditions in VF reset", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "1248cbab604accc4589de8d7b056b3d1cacf58eb", "submitter": { "id": 68919, "url": "http://patchwork.ozlabs.org/api/people/68919/?format=api", "name": "Pujari, Bimmy", "email": "bimmy.pujari@intel.com" }, "delegate": { "id": 68, "url": "http://patchwork.ozlabs.org/api/users/68/?format=api", "username": "jtkirshe", "first_name": "Jeff", "last_name": "Kirsher", "email": "jeffrey.t.kirsher@intel.com" }, "mbox": "http://patchwork.ozlabs.org/project/intel-wired-lan/patch/1487721348-25617-5-git-send-email-bimmy.pujari@intel.com/mbox/", "series": [], "comments": "http://patchwork.ozlabs.org/api/patches/730824/comments/", "check": "pending", "checks": "http://patchwork.ozlabs.org/api/patches/730824/checks/", "tags": {}, "related": [], "headers": { "Return-Path": "<intel-wired-lan-bounces@lists.osuosl.org>", "X-Original-To": [ "incoming@patchwork.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Delivered-To": [ "patchwork-incoming@bilbo.ozlabs.org", "intel-wired-lan@lists.osuosl.org" ], "Received": [ "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3vSbWH1PX6z9s73\n\tfor <incoming@patchwork.ozlabs.org>;\n\tWed, 22 Feb 2017 09:58:02 +1100 (AEDT)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id F267B8AE0D;\n\tTue, 21 Feb 2017 22:58:00 +0000 (UTC)", "from hemlock.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id v1RSJnps5zob; Tue, 21 Feb 2017 22:58:00 +0000 (UTC)", "from ash.osuosl.org (ash.osuosl.org [140.211.166.34])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id 4750A8A686;\n\tTue, 21 Feb 2017 22:58:00 +0000 (UTC)", "from hemlock.osuosl.org (smtp2.osuosl.org [140.211.166.133])\n\tby ash.osuosl.org (Postfix) with ESMTP id C278B1BFF60\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 21 Feb 2017 22:57:56 +0000 (UTC)", "from localhost (localhost [127.0.0.1])\n\tby hemlock.osuosl.org (Postfix) with ESMTP id BEA7A8A521\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 21 Feb 2017 22:57:56 +0000 (UTC)", "from hemlock.osuosl.org ([127.0.0.1])\n\tby localhost (.osuosl.org [127.0.0.1]) (amavisd-new, port 10024)\n\twith ESMTP id vrbq+ewH3Pwk for <intel-wired-lan@lists.osuosl.org>;\n\tTue, 21 Feb 2017 22:57:56 +0000 (UTC)", "from mga01.intel.com (mga01.intel.com [192.55.52.88])\n\tby hemlock.osuosl.org (Postfix) with ESMTPS id 10BD18A60F\n\tfor <intel-wired-lan@lists.osuosl.org>;\n\tTue, 21 Feb 2017 22:57:56 +0000 (UTC)", "from fmsmga002.fm.intel.com ([10.253.24.26])\n\tby fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;\n\t21 Feb 2017 14:57:55 -0800", "from bimmy.jf.intel.com (HELO bimmy.linux1.jf.intel.com)\n\t([10.166.35.87])\n\tby fmsmga002.fm.intel.com with ESMTP; 21 Feb 2017 14:57:55 -0800" ], "X-Virus-Scanned": [ "amavisd-new at osuosl.org", "amavisd-new at osuosl.org" ], "X-Greylist": "domain auto-whitelisted by SQLgrey-1.7.6", "X-ExtLoop1": "1", "X-IronPort-AV": "E=Sophos; i=\"5.35,191,1484035200\"; d=\"scan'208\";\n\ta=\"1133083134\"", "From": "Bimmy Pujari <bimmy.pujari@intel.com>", "To": "intel-wired-lan@lists.osuosl.org", "Date": "Tue, 21 Feb 2017 15:55:42 -0800", "Message-Id": "<1487721348-25617-5-git-send-email-bimmy.pujari@intel.com>", "X-Mailer": "git-send-email 2.4.11", "In-Reply-To": "<1487721348-25617-1-git-send-email-bimmy.pujari@intel.com>", "References": "<1487721348-25617-1-git-send-email-bimmy.pujari@intel.com>", "Cc": "Robert Konklewski <robertx.konklewski@intel.com>", "Subject": "[Intel-wired-lan] [next PATCH S61 04/10] i40e: Fixed race\n\tconditions in VF reset", "X-BeenThere": "intel-wired-lan@lists.osuosl.org", "X-Mailman-Version": "2.1.18-1", "Precedence": "list", "List-Id": "Intel Wired Ethernet Linux Kernel Driver Development\n\t<intel-wired-lan.lists.osuosl.org>", "List-Unsubscribe": "<http://lists.osuosl.org/mailman/options/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=unsubscribe>", "List-Archive": "<http://lists.osuosl.org/pipermail/intel-wired-lan/>", "List-Post": "<mailto:intel-wired-lan@lists.osuosl.org>", "List-Help": "<mailto:intel-wired-lan-request@lists.osuosl.org?subject=help>", "List-Subscribe": "<http://lists.osuosl.org/mailman/listinfo/intel-wired-lan>, \n\t<mailto:intel-wired-lan-request@lists.osuosl.org?subject=subscribe>", "MIME-Version": "1.0", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "intel-wired-lan-bounces@lists.osuosl.org", "Sender": "\"Intel-wired-lan\" <intel-wired-lan-bounces@lists.osuosl.org>" }, "content": "From: Robert Konklewski <robertx.konklewski@intel.com>\n\nFirst, this patch eliminates IOMMU DMAR Faults caused by VF hardware.\nThis is done by enabling VF hardware only after VSI resources are\nfreed. Otherwise, hardware could DMA into memory that is (or just has\nbeen) being freed.\n\nThen, the VF driver is activated only after VSI resources have been\nreallocated. That's because the VF driver can request resources\nimmediately after it's activated. So they need to be ready at that\npoint.\n\nThe second race condition happens when the OS initiates a VF reset,\nand then before it's finished modifies VF's settings by changing its\nMAC, VLAN ID, bandwidth allocation, anti-spoof checking, etc. These\nfunctions needed to be blocked while VF is undergoing reset. Otherwise,\nthey could operate on data structures that had just been freed or not\nyet fully initialized.\n\nSigned-off-by: Robert Konklewski <robertx.konklewski@intel.com>\nChange-ID: I43ba5a7ae2c9a1cce3911611ffc4598ae33ae3ff\n---\nTesting Hints:\n Create 8 VMs with 8 VFs each (the more the better).\n Set MTU=9000 for all VFs on Guest OS boot (this triggers VF reset).\n Reboot all VMs at once, wait until they are up, repeat.\n\n drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c | 43 ++++++++++++++++++----\n 1 file changed, 35 insertions(+), 8 deletions(-)", "diff": "diff --git a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c\nindex a970ba3..86537f1 100644\n--- a/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c\n+++ b/drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c\n@@ -813,6 +813,11 @@ static void i40e_free_vf_res(struct i40e_vf *vf)\n \tu32 reg_idx, reg;\n \tint i, msix_vf;\n \n+\t/* Start by disabling VF's configuration API to prevent the OS from\n+\t * accessing the VF's VSI after it's freed / invalidated.\n+\t */\n+\tclear_bit(I40E_VF_STAT_INIT, &vf->vf_states);\n+\n \t/* free vsi & disconnect it from the parent uplink */\n \tif (vf->lan_vsi_idx) {\n \t\ti40e_vsi_release(pf->vsi[vf->lan_vsi_idx]);\n@@ -854,7 +859,6 @@ static void i40e_free_vf_res(struct i40e_vf *vf)\n \t */\n \tvf->num_queue_pairs = 0;\n \tvf->vf_states = 0;\n-\tclear_bit(I40E_VF_STAT_INIT, &vf->vf_states);\n }\n \n /**\n@@ -945,6 +949,14 @@ void i40e_reset_vf(struct i40e_vf *vf, bool flr)\n \t/* warn the VF */\n \tclear_bit(I40E_VF_STAT_ACTIVE, &vf->vf_states);\n \n+\t/* Disable VF's configuration API during reset. The flag is re-enabled\n+\t * in i40e_alloc_vf_res(), when it's safe again to access VF's VSI.\n+\t * It's normally disabled in i40e_free_vf_res(), but it's safer\n+\t * to do it earlier to give some time to finish to any VF config\n+\t * functions that may still be running at this point.\n+\t */\n+\tclear_bit(I40E_VF_STAT_INIT, &vf->vf_states);\n+\n \t/* In the case of a VFLR, the HW has already reset the VF and we\n \t * just need to clean up, so don't hit the VFRTRIG register.\n \t */\n@@ -988,11 +1000,6 @@ void i40e_reset_vf(struct i40e_vf *vf, bool flr)\n \tif (!rsd)\n \t\tdev_err(&pf->pdev->dev, \"VF reset check timeout on VF %d\\n\",\n \t\t\tvf->vf_id);\n-\twr32(hw, I40E_VFGEN_RSTAT1(vf->vf_id), I40E_VFR_COMPLETED);\n-\t/* clear the reset bit in the VPGEN_VFRTRIG reg */\n-\treg = rd32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id));\n-\treg &= ~I40E_VPGEN_VFRTRIG_VFSWR_MASK;\n-\twr32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id), reg);\n \n \t/* On initial reset, we won't have any queues */\n \tif (vf->lan_vsi_idx == 0)\n@@ -1000,8 +1007,24 @@ void i40e_reset_vf(struct i40e_vf *vf, bool flr)\n \n \ti40e_vsi_stop_rings(pf->vsi[vf->lan_vsi_idx]);\n complete_reset:\n-\t/* reallocate VF resources to reset the VSI state */\n+\t/* free VF resources to begin resetting the VSI state */\n \ti40e_free_vf_res(vf);\n+\n+\t/* Enable hardware by clearing the reset bit in the VPGEN_VFRTRIG reg.\n+\t * By doing this we allow HW to access VF memory at any point. If we\n+\t * did it any sooner, HW could access memory while it was being freed\n+\t * in i40e_free_vf_res(), causing an IOMMU fault.\n+\t *\n+\t * On the other hand, this needs to be done ASAP, because the VF driver\n+\t * is waiting for this to happen and may report a timeout. It's\n+\t * harmless, but it gets logged into Guest OS kernel log, so best avoid\n+\t * it.\n+\t */\n+\treg = rd32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id));\n+\treg &= ~I40E_VPGEN_VFRTRIG_VFSWR_MASK;\n+\twr32(hw, I40E_VPGEN_VFRTRIG(vf->vf_id), reg);\n+\n+\t/* reallocate VF resources to finish resetting the VSI state */\n \tif (!i40e_alloc_vf_res(vf)) {\n \t\tint abs_vf_id = vf->vf_id + hw->func_caps.vf_base_id;\n \t\ti40e_enable_vf_mappings(vf);\n@@ -1012,7 +1035,11 @@ void i40e_reset_vf(struct i40e_vf *vf, bool flr)\n \t\t\ti40e_notify_client_of_vf_reset(pf, abs_vf_id);\n \t\tvf->num_vlan = 0;\n \t}\n-\t/* tell the VF the reset is done */\n+\n+\t/* Tell the VF driver the reset is done. This needs to be done only\n+\t * after VF has been fully initialized, because the VF driver may\n+\t * request resources immediately after setting this flag.\n+\t */\n \twr32(hw, I40E_VFGEN_RSTAT1(vf->vf_id), I40E_VFR_VFACTIVE);\n \n \ti40e_flush(hw);\n", "prefixes": [ "next", "S61", "04/10" ] }