From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Assmann <sassmann@kpanic.de>
Date: Mon, 19 Oct 2015 12:50:39 +0200
Subject: [Intel-wired-lan] [PATCH] igb: fix NULL derefs due to skipped
	SR-IOV enabling
In-Reply-To: <5624E0C102000078000AC476@prv-mh.provo.novell.com>
References: <5624E0C102000078000AC476@prv-mh.provo.novell.com>
Message-ID: <5624CAFF.4060400@kpanic.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: intel-wired-lan@osuosl.org
List-ID: <intel-wired-lan.osuosl.org>

On 19.10.2015 12:23, Jan Beulich wrote:
> The combined effect of commits 6423fc3416 ("igb: do not re-init SR-IOV
> during probe") and ceee3450b3 ("igb: make sure SR-IOV init uses the
> right number of queues") causes VFs no longer getting set up, leading
> to NULL pointer dereferences due to the adapter's ->vf_data being NULL
> while ->vfs_allocated_count is non-zero. The first commit not only
> neglected the side effect of igb_sriov_reinit() that the second commit
> tried to account for, but also that of setting IGB_FLAG_HAS_MSIX,
> without which igb_enable_sriov() is effectively a no-op. Calling
> igb_{,re}set_interrupt_capability() as done here seems to address this,
> but I'm not sure whether this is better than sinply reverting the other
> two commits.

Jan,

this should already be fixed by the following net-next commit
cbfe360a1541a32e9e28f8f8ac925d2b7979d767 igb: assume MSI-X interrupts during initialization

  Stefan

> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Cc: Stefan Assmann <sassmann@kpanic.de>
> Cc: Todd Fujinaka <todd.fujinaka@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Shannon Nelson <shannon.nelson@intel.com>
> Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
> Cc: Don Skidmore <donald.c.skidmore@intel.com>
> Cc: Matthew Vick <matthew.vick@intel.com>
> Cc: John Ronciak <john.ronciak@intel.com>
> Cc: Mitch Williams <mitch.a.williams@intel.com>
> ---
> As a side note, while looking at all the interactions here it seems
> suspicious to me that igb_request_msix() failure does not lead to
> SR-IOV being disabled on the device, despite IGB_FLAG_HAS_MSIX being
> set being a prereq to it as explained above.
> ---
>  drivers/net/ethernet/intel/igb/igb_main.c |    8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> --- 4.3-rc6/drivers/net/ethernet/intel/igb/igb_main.c
> +++ 4.3-rc6-IGB-VF-setup/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -2856,6 +2856,14 @@ static void igb_probe_vfs(struct igb_ada
>  	if ((hw->mac.type == e1000_i210) || (hw->mac.type == e1000_i211))
>  		return;
>  
> +	/*
> +	 * Of the below we really only want the effect of getting
> +	 * IGB_FLAG_HAS_MSIX set (if available), without which
> +	 * igb_enable_sriov() has no effect.
> +	 */
> +	igb_set_interrupt_capability(adapter, true);
> +	igb_reset_interrupt_capability(adapter);
> +
>  	pci_sriov_set_totalvfs(pdev, 7);
>  	igb_enable_sriov(pdev, max_vfs);
>  
> 
> 
>