netdev - RE: [PATCH] ixgbe: check Master Disable bit after setting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <F6FB0E698C9B3143BDF729DF22286646913C0D10@ORSMSX110.amr.corp.intel.com>
Date:	Tue, 27 Oct 2015 17:14:50 +0000
From:	"Skidmore, Donald C" <donald.c.skidmore@...el.com>
To:	"dan.streetman@...onical.com" <dan.streetman@...onical.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>
CC:	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	"Nelson, Shannon" <shannon.nelson@...el.com>,
	"Wyborny, Carolyn" <carolyn.wyborny@...el.com>,
	"Vick, Matthew" <matthew.vick@...el.com>,
	"Ronciak, John" <john.ronciak@...el.com>,
	"Williams, Mitch A" <mitch.a.williams@...el.com>,
	"intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Dan Streetman <ddstreet@...e.org>
Subject: RE: [PATCH] ixgbe: check Master Disable bit after setting



> -----Original Message-----
> From: dan.streetman@...onical.com
> [mailto:dan.streetman@...onical.com]
> Sent: Monday, October 26, 2015 5:20 PM
> To: Kirsher, Jeffrey T
> Cc: Brandeburg, Jesse; Nelson, Shannon; Wyborny, Carolyn; Skidmore,
> Donald C; Vick, Matthew; Ronciak, John; Williams, Mitch A; intel-wired-
> lan@...ts.osuosl.org; netdev@...r.kernel.org; linux-kernel@...r.kernel.org;
> Dan Streetman; Dan Streetman
> Subject: [PATCH] ixgbe: check Master Disable bit after setting
> 
> From: Dan Streetman <dan.streetman@...onical.com>
> 
> Spec section 8.2.4.1.1 notes that after setting the PCIe Master Disable bit, it
> must be read to verify it was set before polling the Master Enable status bit.
> 
> This adds the check to verify the Master Disable bit was set.
> 
> This also corrects the spec section number reference - the Master Disable
> section is 5.2.4.3.2, not 5.2.5.3.2.
> 
> Signed-off-by: Dan Streetman <dan.streetman@...onical.com>
> Signed-off-by: Dan Streetman <ddstreet@...e.org>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> index 3f56a80..abfada7 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_common.c
> @@ -2453,6 +2453,16 @@ static s32 ixgbe_disable_pcie_master(struct
> ixgbe_hw *hw)
>  	/* Always set this bit to ensure any future transactions are blocked */
>  	IXGBE_WRITE_REG(hw, IXGBE_CTRL, IXGBE_CTRL_GIO_DIS);
> 
> +	/* Spec sec 8.2.4.1.1, Master Disable bit :
> +	 * "After doing any change to this bit the host must read that
> +	 *  the bit has been modified as expected before reading
> +	 *  STATUS.PCIe Master Enable Status bit."
> +	 */
> +	if (!(IXGBE_READ_REG(hw, IXGBE_CTRL) & IXGBE_CTRL_GIO_DIS)) {
> +		hw_err(hw, "GIO Master Disable bit didn't set\n");
> +		goto gio_dis_fail;
> +	}
> +
>  	/* Exit if master requests are blocked */
>  	if (!(IXGBE_READ_REG(hw, IXGBE_STATUS) & IXGBE_STATUS_GIO) ||
>  	    ixgbe_removed(hw->hw_addr))
> @@ -2467,13 +2477,14 @@ static s32 ixgbe_disable_pcie_master(struct
> ixgbe_hw *hw)
> 
>  	/*
>  	 * Two consecutive resets are required via CTRL.RST per datasheet
> -	 * 5.2.5.3.2 Master Disable.  We set a flag to inform the reset routine
> +	 * 5.2.4.3.2 Master Disable.  We set a flag to inform the reset
> +routine
>  	 * of this need.  The first reset prevents new master requests from
>  	 * being issued by our device.  We then must wait 1usec or more for
> any
>  	 * remaining completions from the PCIe bus to trickle in, and then
> reset
>  	 * again to clear out any effects they may have had on our device.
>  	 */
>  	hw_dbg(hw, "GIO Master Disable bit didn't clear - requesting
> resets\n");
> +gio_dis_fail:
>  	hw->mac.flags |= IXGBE_FLAGS_DOUBLE_RESET_REQUIRED;
> 
>  	/*
> --
> 2.5.0

Is this patch correcting some issue you're running in to?
I ask as I don't believe this check is necessary, since right below setting CTLR. PCIE_MASTER_DISABLE we loop waiting for  STATUS. PCIE_MASTER_ENABLE_STATUS to clear.   This should have the same effect as verify that the write to CTLR. PCIE_MASTER_DISABLE has cleared.

Thanks,
Don Skidmore <donald.c.skidmore@...el.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html