netdev - RE: [PATCH V4 1/3] can: flexcan: initialize all flexcan memory for ECC function

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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 29 Sep 2020 12:38:12 +0000
From:   Joakim Zhang <qiangqing.zhang@....com>
To:     Marc Kleine-Budde <mkl@...gutronix.de>,
        "linux-can@...r.kernel.org" <linux-can@...r.kernel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        dl-linux-imx <linux-imx@....com>
Subject: RE: [PATCH V4 1/3] can: flexcan: initialize all flexcan memory for
 ECC function


> -----Original Message-----
> From: Marc Kleine-Budde <mkl@...gutronix.de>
> Sent: 2020年9月29日 20:34
> To: Joakim Zhang <qiangqing.zhang@....com>; linux-can@...r.kernel.org
> Cc: netdev@...r.kernel.org; dl-linux-imx <linux-imx@....com>
> Subject: Re: [PATCH V4 1/3] can: flexcan: initialize all flexcan memory for ECC
> function
> 
> On 9/29/20 10:30 PM, Joakim Zhang wrote:
> > One issue was reported at a baremetal environment, which is used for
> > FPGA verification. "The first transfer will fail for extended ID
> > format(for both 2.0B and FD format), following frames can be
> > transmitted and received successfully for extended format, and
> > standard format don't have this issue. This issue occurred randomly
> > with high possiblity, when it occurs, the transmitter will detect a
> > BIT1 error, the receiver a CRC error. According to the spec, a
> > non-correctable error may cause this transfer failure."
> >
> > With FLEXCAN_QUIRK_DISABLE_MECR quirk, it supports correctable errors,
> > disable non-correctable errors interrupt and freeze mode. Platform has
> > ECC hardware support, but select this quirk, this issue may not come
> > to light. Initialize all FlexCAN memory before accessing them, at
> > least it can avoid non-correctable errors detected due to memory
> uninitialized.
> > The internal region can't be initialized when the hardware doesn't
> > support ECC.
> >
> > According to IMX8MPRM, Rev.C, 04/2020. There is a NOTE at the section
> > 11.8.3.13 Detection and correction of memory errors:
> > "All FlexCAN memory must be initialized before starting its operation
> > in order to have the parity bits in memory properly updated.
> > CTRL2[WRMFRZ] grants write access to all memory positions that require
> > initialization, ranging from 0x080 to 0xADF and from 0xF28 to 0xFFF
> > when the CAN FD feature is enabled. The RXMGMASK, RX14MASK,
> RX15MASK,
> > and RXFGMASK registers need to be initialized as well. MCR[RFEN] must not
> be set during memory initialization."
> >
> > Memory range from 0x080 to 0xADF, there are reserved memory
> > (unimplemented by hardware, e.g. only configure 64 MBs), these memory
> can be initialized or not.
> > In this patch, initialize all flexcan memory which includes reserved memory.
> >
> > In this patch, create FLEXCAN_QUIRK_SUPPORT_ECC for platforms which
> > has ECC feature. If you have a ECC platform in your hand, please
> > select this qurik to initialize all flexcan memory firstly, then you
> > can select FLEXCAN_QUIRK_DISABLE_MECR to only enable correctable
> errors.
> >
> > Signed-off-by: Joakim Zhang <qiangqing.zhang@....com>
> > ---
> > ChangeLogs:
> > V1->V2:
> > 	* update commit messages, add a datasheet reference.
> > 	* initialize block memory instead of trivial memory.
> > 	* inilialize reserved memory.
> > V2->V3:
> > 	* add FLEXCAN_QUIRK_SUPPORT_ECC quirk.
> > 	* remove init_ram struct.
> > V3->V4:
> > 	* move register definition into flexcan_reg.
> > ---
> >  drivers/net/can/flexcan.c | 51
> > +++++++++++++++++++++++++++++++++++++--
> >  1 file changed, 49 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> > index e86925134009..ede25db42e87 100644
> > --- a/drivers/net/can/flexcan.c
> > +++ b/drivers/net/can/flexcan.c
> > @@ -239,6 +239,8 @@
> >  #define FLEXCAN_QUIRK_SETUP_STOP_MODE BIT(8)
> >  /* Support CAN-FD mode */
> >  #define FLEXCAN_QUIRK_SUPPORT_FD BIT(9)
> > +/* support memory detection and correction */ #define
> > +FLEXCAN_QUIRK_SUPPORT_ECC BIT(10)
> >
> >  /* Structure of the message buffer */  struct flexcan_mb { @@ -292,7
> > +294,16 @@ struct flexcan_regs {
> >  	u32 rximr[64];		/* 0x880 - Not affected by Soft Reset */
> >  	u32 _reserved5[24];	/* 0x980 */
> >  	u32 gfwr_mx6;		/* 0x9e0 - MX6 */
> > -	u32 _reserved6[63];	/* 0x9e4 */
> > +	u32 _reserved6[39];	/* 0x9e4 */
> > +	u32 _rxfir[6];		/* 0xa80 */
> > +	u32 _reserved8[2];	/* 0xa98 */
> > +	u32 _rxmgmask;		/* 0xaa0 */
> > +	u32 _rxfgmask;		/* 0xaa4 */
> > +	u32 _rx14mask;		/* 0xaa8 */
> > +	u32 _rx15mask;		/* 0xaac */
> > +	u32 tx_smb[4];		/* 0xab0 */
> > +	u32 rx_smb0[4];		/* 0xac0 */
> > +	u32 rx_smb1[4];		/* 0xad0 */
> >  	u32 mecr;		/* 0xae0 */
> >  	u32 erriar;		/* 0xae4 */
> >  	u32 erridpr;		/* 0xae8 */
> > @@ -305,9 +316,13 @@ struct flexcan_regs {
> >  	u32 fdctrl;		/* 0xc00 - Not affected by Soft Reset */
> >  	u32 fdcbt;		/* 0xc04 - Not affected by Soft Reset */
> >  	u32 fdcrc;		/* 0xc08 */
> > +	u32 _reserved9[199];	/* 0xc0c */
> > +	u32 tx_smb_fd[18];	/* 0xf28 */
> > +	u32 rx_smb0_fd[18];	/* 0xf70 */
> > +	u32 rx_smb1_fd[18];	/* 0xfb8 */
> >  };
> >
> > -static_assert(sizeof(struct flexcan_regs) == 0x4 + 0xc08);
> > +static_assert(sizeof(struct flexcan_regs) ==  0x4 * 18 + 0xfb8);
> >
> >  struct flexcan_devtype_data {
> >  	u32 quirks;		/* quirks needed for different IP cores */
> > @@ -1292,6 +1307,35 @@ static void flexcan_set_bittiming(struct
> net_device *dev)
> >  		return flexcan_set_bittiming_ctrl(dev);  }
> >
> > +static void flexcan_init_ram(struct net_device *dev) {
> > +	struct flexcan_priv *priv = netdev_priv(dev);
> > +	struct flexcan_regs __iomem *regs = priv->regs;
> > +	u32 reg_ctrl2;
> > +
> > +	/* 11.8.3.13 Detection and correction of memory errors:
> > +	 * CTRL2[WRMFRZ] grants write access to all memory positions that
> > +	 * require initialization, ranging from 0x080 to 0xADF and
> > +	 * from 0xF28 to 0xFFF when the CAN FD feature is enabled.
> > +	 * The RXMGMASK, RX14MASK, RX15MASK, and RXFGMASK registers
> need to
> > +	 * be initialized as well. MCR[RFEN] must not be set during memory
> > +	 * initialization.
> > +	 */
> > +	reg_ctrl2 = priv->read(&regs->ctrl2);
> > +	reg_ctrl2 |= FLEXCAN_CTRL2_WRMFRZ;
> > +	priv->write(reg_ctrl2, &regs->ctrl2);
> > +
> > +	memset_io(&regs->mb[0][0], 0,
> > +		  (u8 *)&regs->rx_smb1[3] - &regs->mb[0][0] + 0x4);
> 
> why the cast?

Due to mb is defined as a u8. And the count of memset_io is bytes.

> > +
> > +	if (priv->can.ctrlmode & CAN_CTRLMODE_FD)
> > +		memset_io(&regs->tx_smb_fd[0], 0,
> > +			  (u8 *)&regs->rx_smb1_fd[17] - (u8 *)&regs->tx_smb_fd[0] +
> 0x4);
> 
> why the cast?
> 
> > +
> > +	reg_ctrl2 &= ~FLEXCAN_CTRL2_WRMFRZ;
> > +	priv->write(reg_ctrl2, &regs->ctrl2); }
> > +
> >  /* flexcan_chip_start
> >   *
> >   * this functions is entered with clocks enabled @@ -1316,6 +1360,9
> > @@ static int flexcan_chip_start(struct net_device *dev)
> >  	if (err)
> >  		goto out_chip_disable;
> >
> > +	if (priv->devtype_data->quirks & FLEXCAN_QUIRK_SUPPORT_ECC)
> > +		flexcan_init_ram(dev);
> > +
> >  	flexcan_set_bittiming(dev);
> >
> >  	/* MCR
> >
> 
> Marc
> 
> --
> Pengutronix e.K.                 | Marc Kleine-Budde           |
> Embedded Linux                   | https://www.pengutronix.de  |
> Vertretung West/Dortmund         | Phone: +49-231-2826-924     |
> Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-5555 |