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] [day] [month] [year] [list]
Message-Id: <20180216134027.GB30262@korriban>
Date:   Fri, 16 Feb 2018 11:40:28 -0200
From:   Mauro Rodrigues <maurosr@...ux.vnet.ibm.com>
To:     Satish Baddipadige <satish.baddipadige@...adcom.com>
Cc:     davem@...emloft.net, netdev@...r.kernel.org,
        michael.chan@...adcom.com, prashant@...adcom.com,
        siva.kallam@...adcom.com,
        Prashant Sreedharan <prashant.sreedharan@...adcom.com>
Subject: Re: [PATCH net] tg3: APE heartbeat changes

On Fri, Feb 16, 2018 at 11:22:43AM +0530, Satish Baddipadige wrote:
> From: Prashant Sreedharan <prashant.sreedharan@...adcom.com>
> 
> In ungraceful host shutdown or driver crash case BMC connectivity is
> lost. APE firmware is missing the driver state in this
> case to keep the BMC connectivity alive.
> This patch has below change to address this issue.
> 
> Heartbeat mechanism with APE firmware. This heartbeat mechanism
> is needed to notify the APE firmware about driver state.
> 
> This patch also has the change in wait time for APE event from
> 1ms to 20ms as there can be some delay in getting response.
> 
> Signed-off-by: Prashant Sreedharan <prashant.sreedharan@...adcom.com>
> Signed-off-by: Satish Baddipadige <satish.baddipadige@...adcom.com>
> Signed-off-by: Siva Reddy Kallam <siva.kallam@...adcom.com>
> Acked-by: Michael Chan <michael.chan@...adcom.com>
Tested-by: Mauro S. M. Rodrigues <maurosr@...ux.vnet.ibm.com>

Thank you very much for the patch Satish.
> ---
>  drivers/net/ethernet/broadcom/tg3.c | 35 ++++++++++++++++++++++++-----------
>  drivers/net/ethernet/broadcom/tg3.h |  5 +++++
>  2 files changed, 29 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/ethernet/broadcom/tg3.c b/drivers/net/ethernet/broadcom/tg3.c
> index a77ee2f..36d8d1e 100644
> --- a/drivers/net/ethernet/broadcom/tg3.c
> +++ b/drivers/net/ethernet/broadcom/tg3.c
> @@ -820,7 +820,7 @@ static int tg3_ape_event_lock(struct tg3 *tp, u32 timeout_us)
> 
>  		tg3_ape_unlock(tp, TG3_APE_LOCK_MEM);
> 
> -		udelay(10);
> +		usleep_range(10, 20);
>  		timeout_us -= (timeout_us > 10) ? 10 : timeout_us;
>  	}
> 
> @@ -922,8 +922,8 @@ static int tg3_ape_send_event(struct tg3 *tp, u32 event)
>  	if (!(apedata & APE_FW_STATUS_READY))
>  		return -EAGAIN;
> 
> -	/* Wait for up to 1 millisecond for APE to service previous event. */
> -	err = tg3_ape_event_lock(tp, 1000);
> +	/* Wait for up to 20 millisecond for APE to service previous event. */
> +	err = tg3_ape_event_lock(tp, 20000);
>  	if (err)
>  		return err;
> 
> @@ -946,6 +946,7 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
> 
>  	switch (kind) {
>  	case RESET_KIND_INIT:
> +		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
>  		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG,
>  				APE_HOST_SEG_SIG_MAGIC);
>  		tg3_ape_write32(tp, TG3_APE_HOST_SEG_LEN,
> @@ -962,13 +963,6 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
>  		event = APE_EVENT_STATUS_STATE_START;
>  		break;
>  	case RESET_KIND_SHUTDOWN:
> -		/* With the interface we are currently using,
> -		 * APE does not track driver state.  Wiping
> -		 * out the HOST SEGMENT SIGNATURE forces
> -		 * the APE to assume OS absent status.
> -		 */
> -		tg3_ape_write32(tp, TG3_APE_HOST_SEG_SIG, 0x0);
> -
>  		if (device_may_wakeup(&tp->pdev->dev) &&
>  		    tg3_flag(tp, WOL_ENABLE)) {
>  			tg3_ape_write32(tp, TG3_APE_HOST_WOL_SPEED,
> @@ -990,6 +984,18 @@ static void tg3_ape_driver_state_change(struct tg3 *tp, int kind)
>  	tg3_ape_send_event(tp, event);
>  }
> 
> +static inline void tg3_send_ape_heartbeat(struct tg3 *tp,
> +					  unsigned long interval)
> +{
> +	/* Check if hb interval has exceeded */
> +	if (!tg3_flag(tp, ENABLE_APE) ||
> +	    time_before(jiffies, tp->ape_hb_jiffies + interval))
> +		return;
> +
> +	tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_COUNT, tp->ape_hb++);
> +	tp->ape_hb_jiffies = jiffies;
> +}
> +
>  static void tg3_disable_ints(struct tg3 *tp)
>  {
>  	int i;
> @@ -7262,6 +7268,7 @@ static int tg3_poll_msix(struct napi_struct *napi, int budget)
>  		}
>  	}
> 
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
>  	return work_done;
> 
>  tx_recovery:
> @@ -7344,6 +7351,7 @@ static int tg3_poll(struct napi_struct *napi, int budget)
>  		}
>  	}
> 
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL << 1);
>  	return work_done;
> 
>  tx_recovery:
> @@ -10732,7 +10740,7 @@ static int tg3_reset_hw(struct tg3 *tp, bool reset_phy)
>  	if (tg3_flag(tp, ENABLE_APE))
>  		/* Write our heartbeat update interval to APE. */
>  		tg3_ape_write32(tp, TG3_APE_HOST_HEARTBEAT_INT_MS,
> -				APE_HOST_HEARTBEAT_INT_DISABLE);
> +				APE_HOST_HEARTBEAT_INT_5SEC);
> 
>  	tg3_write_sig_post_reset(tp, RESET_KIND_INIT);
> 
> @@ -11077,6 +11085,9 @@ static void tg3_timer(struct timer_list *t)
>  		tp->asf_counter = tp->asf_multiplier;
>  	}
> 
> +	/* Update the APE heartbeat every 5 seconds.*/
> +	tg3_send_ape_heartbeat(tp, TG3_APE_HB_INTERVAL);
> +
>  	spin_unlock(&tp->lock);
> 
>  restart_timer:
> @@ -16653,6 +16664,8 @@ static int tg3_get_invariants(struct tg3 *tp, const struct pci_device_id *ent)
>  				       pci_state_reg);
> 
>  		tg3_ape_lock_init(tp);
> +		tp->ape_hb_interval =
> +			msecs_to_jiffies(APE_HOST_HEARTBEAT_INT_5SEC);
>  	}
> 
>  	/* Set up tp->grc_local_ctrl before calling
> diff --git a/drivers/net/ethernet/broadcom/tg3.h b/drivers/net/ethernet/broadcom/tg3.h
> index 47f51cc..1d61aa3 100644
> --- a/drivers/net/ethernet/broadcom/tg3.h
> +++ b/drivers/net/ethernet/broadcom/tg3.h
> @@ -2508,6 +2508,7 @@
>  #define TG3_APE_LOCK_PHY3		5
>  #define TG3_APE_LOCK_GPIO		7
> 
> +#define TG3_APE_HB_INTERVAL             (tp->ape_hb_interval)
>  #define TG3_EEPROM_SB_F1R2_MBA_OFF	0x10
> 
> 
> @@ -3423,6 +3424,10 @@ struct tg3 {
>  	struct device			*hwmon_dev;
>  	bool				link_up;
>  	bool				pcierr_recovery;
> +
> +	u32                             ape_hb;
> +	unsigned long                   ape_hb_interval;
> +	unsigned long                   ape_hb_jiffies;
>  };
> 
>  /* Accessor macros for chip and asic attributes
> -- 
> 2.1.0
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ