diff mbox series

[SRU,F:linux-bluefield,v1,1/1] UBUNTU: SAUCE: mlx-bootctl: rshim logging display from linux sysfs

Message ID 8f4bb75041167893ebd991562d4bfcaa2bbaf8ff.1620234248.git.limings@nvidia.com
State New
Headers show
Series UBUNTU: SAUCE: mlx-bootctl: rshim logging display from linux sysfs | expand

Commit Message

Liming Sun May 5, 2021, 5:10 p.m. UTC
This commit adds the rshim logging display support from linux sysfs.
The display logic is ported from the user-space rshim driver. Module
parameter 'rsh_log_clear_on_read' is added to clear the the logging
buffer after read when this flag is set.

Example:

  cat /sys/bus/platform/drivers/mlx-bootctl/rsh_log
   INFO[BL2]: start
   INFO[BL2]: DDR POST passed
   INFO[BL2]: UEFI loaded
   INFO[BL31]: start
   INFO[BL31]: runtime
   ...

  echo 1 > /sys/module/mlx_bootctl/parameters/rsh_log_clear_on_read

Change-Id: I50129fc30f9e2e7b6b38713c740968f9235b532a
---
 drivers/platform/mellanox/mlx-bootctl.c | 408 +++++++++++++++++++++++++++++---
 1 file changed, 376 insertions(+), 32 deletions(-)

Comments

Krzysztof Kozlowski May 5, 2021, 6:20 p.m. UTC | #1
On 05/05/2021 13:10, Liming Sun wrote:
> This commit adds the rshim logging display support from linux sysfs.
> The display logic is ported from the user-space rshim driver. Module
> parameter 'rsh_log_clear_on_read' is added to clear the the logging
> buffer after read when this flag is set.
> 

Hi,

Thanks for your patch.

You miss the BugLink at the beginning. The example here might be helpful:
https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat

> Example:
> 
>   cat /sys/bus/platform/drivers/mlx-bootctl/rsh_log
>    INFO[BL2]: start
>    INFO[BL2]: DDR POST passed
>    INFO[BL2]: UEFI loaded
>    INFO[BL31]: start
>    INFO[BL31]: runtime
>    ...
> 
>   echo 1 > /sys/module/mlx_bootctl/parameters/rsh_log_clear_on_read
> 
> Change-Id: I50129fc30f9e2e7b6b38713c740968f9235b532a

Please strip out Gerrit stuff.

You missed here your own SoB.

> ---
>  drivers/platform/mellanox/mlx-bootctl.c | 408 +++++++++++++++++++++++++++++---
>  1 file changed, 376 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlx-bootctl.c b/drivers/platform/mellanox/mlx-bootctl.c
> index 7ec7c9a..4c85139 100644
> --- a/drivers/platform/mellanox/mlx-bootctl.c
> +++ b/drivers/platform/mellanox/mlx-bootctl.c
> @@ -1,4 +1,4 @@
> -// SPDX-License-Identifier: GPL-2.0
> +// SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause

This does not look like related and usually relicensing requires SoB
from copyright owners (e.g. involved people, their representative, the
company).

>  /*
>   *  Mellanox boot control driver
>   *  This driver provides a sysfs interface for systems management
> @@ -64,6 +64,10 @@ struct boot_name {
>  static void __iomem *rsh_scratch_buf_ctl;
>  static void __iomem *rsh_scratch_buf_data;
>  
> +static int rsh_log_clear_on_read;
> +module_param(rsh_log_clear_on_read, int, 0644);
> +MODULE_PARM_DESC(rsh_log_clear_on_read, "Clear rshim logging buffer after read.");
> +
>  /*
>   * Objects are stored within the MFG partition per type. Type 0 is not
>   * supported.
> @@ -86,7 +90,7 @@ enum {
>   * The expected format is: "XX:XX:XX:XX:XX:XX"
>   */
>  #define MLNX_MFG_OOB_MAC_FORMAT_LEN \
> -        ((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN - 1))
> +	((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN - 1))

Does not look like related change.

>  
>  /* The SMC calls in question are atomic, so we don't have to lock here. */
>  static int smc_call1(unsigned int smc_op, int smc_arg)
> @@ -290,7 +294,7 @@ static ssize_t fw_reset_store(struct device_driver *drv,
>  
>  static ssize_t oob_mac_show(struct device_driver *drv, char *buf)
>  {
> -	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN] = { 0 };
> +	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN + 1] = { 0 };
>  	struct arm_smccc_res res;
>  	u8 *mac_byte_ptr;
>  
> @@ -315,14 +319,15 @@ static ssize_t oob_mac_store(struct device_driver *drv, const char *buf,
>  	struct arm_smccc_res res;
>  	u64 mac_addr = 0;
>  	u8 *mac_byte_ptr;
> -	int byte_idx;
> +	int byte_idx, len;
>  
>  	if ((count - 1) != MLNX_MFG_OOB_MAC_FORMAT_LEN)
>  		return -EINVAL;
>  
> -	if (MLNX_MFG_OOB_MAC_LEN != sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
> -					   &byte[0], &byte[1], &byte[2],
> -					   &byte[3], &byte[4], &byte[5]))
> +	len = sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
> +		     &byte[0], &byte[1], &byte[2],
> +		     &byte[3], &byte[4], &byte[5]);
> +	if (len != MLNX_MFG_OOB_MAC_LEN)
>  		return -EINVAL;
>  
>  	mac_byte_ptr = (u8 *)&mac_addr;
> @@ -340,7 +345,7 @@ static ssize_t oob_mac_store(struct device_driver *drv, const char *buf,
>  
>  static u8 get_opn_type(u8 word)
>  {
> -	switch(word) {
> +	switch (word) {

Does not look like related change.

>  	case 0:
>  		return MLNX_MFG_TYPE_OPN_0;
>  	case 1:
> @@ -452,7 +457,32 @@ static ssize_t mfg_lock_store(struct device_driver *drv, const char *buf,
>  #define RSH_LOG_LEVEL_SHIFT	0
>  
>  /* Module ID and type used here. */
> -#define RSH_LOG_TYPE		0x04ULL	/* message */
> +#define BF_RSH_LOG_TYPE_UNKNOWN		0x00ULL
> +#define BF_RSH_LOG_TYPE_PANIC		0x01ULL
> +#define BF_RSH_LOG_TYPE_EXCEPTION	0x02ULL
> +#define BF_RSH_LOG_TYPE_UNUSED		0x03ULL
> +#define BF_RSH_LOG_TYPE_MSG		0x04ULL
> +
> +/* Utility macro. */
> +#define BF_RSH_LOG_MOD_MASK		0x0FULL
> +#define BF_RSH_LOG_MOD_SHIFT		60
> +#define BF_RSH_LOG_TYPE_MASK		0x0FULL
> +#define BF_RSH_LOG_TYPE_SHIFT		56
> +#define BF_RSH_LOG_LEN_MASK		0x7FULL
> +#define BF_RSH_LOG_LEN_SHIFT		48
> +#define BF_RSH_LOG_ARG_MASK		0xFFFFFFFFULL
> +#define BF_RSH_LOG_ARG_SHIFT		16
> +#define BF_RSH_LOG_HAS_ARG_MASK		0xFFULL
> +#define BF_RSH_LOG_HAS_ARG_SHIFT	8
> +#define BF_RSH_LOG_LEVEL_MASK		0xFFULL
> +#define BF_RSH_LOG_LEVEL_SHIFT		0
> +#define BF_RSH_LOG_PC_MASK		0xFFFFFFFFULL
> +#define BF_RSH_LOG_PC_SHIFT		0
> +#define BF_RSH_LOG_SYNDROME_MASK	0xFFFFFFFFULL
> +#define BF_RSH_LOG_SYNDROME_SHIFT	0
> +
> +#define BF_RSH_LOG_HEADER_GET(f, h) \
> +	(((h) >> BF_RSH_LOG_##f##_SHIFT) & BF_RSH_LOG_##f##_MASK)
>  
>  /* Log message level. */
>  enum {
> @@ -461,16 +491,159 @@ enum {
>  	RSH_LOG_ERR
>  };
>  
> -const char *rsh_log_level[] = {"INFO", "WARN", "ERR"};
> +/* Log module */
> +const char * const rsh_log_mod[] = {
> +	"MISC", "BL1", "BL2", "BL2R", "BL31", "UEFI"
> +};
> +
> +const char *rsh_log_level[] = {"INFO", "WARN", "ERR", "ASSERT"};
> +
> +#define AARCH64_MRS_REG_SHIFT 5
> +#define AARCH64_MRS_REG_MASK  0xffff
> +#define AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT 26
> +
> +struct rsh_log_reg {
> +	char *name;
> +	u32 opcode;
> +} rsh_log_reg;
> +
> +static struct rsh_log_reg rsh_log_regs[] = {
> +	{"actlr_el1",		0b1100000010000001},
> +	{"actlr_el2",		0b1110000010000001},
> +	{"actlr_el3",		0b1111000010000001},
> +	{"afsr0_el1",		0b1100001010001000},
> +	{"afsr0_el2",		0b1110001010001000},
> +	{"afsr0_el3",		0b1111001010001000},
> +	{"afsr1_el1",		0b1100001010001001},
> +	{"afsr1_el2",		0b1110001010001001},
> +	{"afsr1_el3",		0b1111001010001001},
> +	{"amair_el1",		0b1100010100011000},
> +	{"amair_el2",		0b1110010100011000},
> +	{"amair_el3",		0b1111010100011000},
> +	{"ccsidr_el1",		0b1100100000000000},
> +	{"clidr_el1",		0b1100100000000001},
> +	{"cntkctl_el1",		0b1100011100001000},
> +	{"cntp_ctl_el0",	0b1101111100010001},
> +	{"cntp_cval_el0",	0b1101111100010010},
> +	{"cntv_ctl_el0",	0b1101111100011001},
> +	{"cntv_cval_el0",	0b1101111100011010},
> +	{"contextidr_el1",	0b1100011010000001},
> +	{"cpacr_el1",		0b1100000010000010},
> +	{"cptr_el2",		0b1110000010001010},
> +	{"cptr_el3",		0b1111000010001010},
> +	{"vtcr_el2",		0b1110000100001010},
> +	{"ctr_el0",		0b1101100000000001},
> +	{"currentel",		0b1100001000010010},
> +	{"dacr32_el2",		0b1110000110000000},
> +	{"daif",		0b1101101000010001},
> +	{"dczid_el0",		0b1101100000000111},
> +	{"dlr_el0",		0b1101101000101001},
> +	{"dspsr_el0",		0b1101101000101000},
> +	{"elr_el1",		0b1100001000000001},
> +	{"elr_el2",		0b1110001000000001},
> +	{"elr_el3",		0b1111001000000001},
> +	{"esr_el1",		0b1100001010010000},
> +	{"esr_el2",		0b1110001010010000},
> +	{"esr_el3",		0b1111001010010000},
> +	{"esselr_el1",		0b1101000000000000},
> +	{"far_el1",		0b1100001100000000},
> +	{"far_el2",		0b1110001100000000},
> +	{"far_el3",		0b1111001100000000},
> +	{"fpcr",		0b1101101000100000},
> +	{"fpexc32_el2",		0b1110001010011000},
> +	{"fpsr",		0b1101101000100001},
> +	{"hacr_el2",		0b1110000010001111},
> +	{"har_el2",		0b1110000010001000},
> +	{"hpfar_el2",		0b1110001100000100},
> +	{"hstr_el2",		0b1110000010001011},
> +	{"far_el1",		0b1100001100000000},
> +	{"far_el2",		0b1110001100000000},
> +	{"far_el3",		0b1111001100000000},
> +	{"hcr_el2",		0b1110000010001000},
> +	{"hpfar_el2",		0b1110001100000100},
> +	{"id_aa64afr0_el1",	0b1100000000101100},
> +	{"id_aa64afr1_el1",	0b1100000000101101},
> +	{"id_aa64dfr0_el1",	0b1100000000101100},
> +	{"id_aa64isar0_el1",	0b1100000000110000},
> +	{"id_aa64isar1_el1",	0b1100000000110001},
> +	{"id_aa64mmfr0_el1",	0b1100000000111000},
> +	{"id_aa64mmfr1_el1",	0b1100000000111001},
> +	{"id_aa64pfr0_el1",	0b1100000000100000},
> +	{"id_aa64pfr1_el1",	0b1100000000100001},
> +	{"ifsr32_el2",		0b1110001010000001},
> +	{"isr_el1",		0b1100011000001000},
> +	{"mair_el1",		0b1100010100010000},
> +	{"mair_el2",		0b1110010100010000},
> +	{"mair_el3",		0b1111010100010000},
> +	{"midr_el1",		0b1100000000000000},
> +	{"mpidr_el1",		0b1100000000000101},
> +	{"nzcv",		0b1101101000010000},
> +	{"revidr_el1",		0b1100000000000110},
> +	{"rmr_el3",		0b1111011000000010},
> +	{"par_el1",		0b1100001110100000},
> +	{"rvbar_el3",		0b1111011000000001},
> +	{"scr_el3",		0b1111000010001000},
> +	{"sctlr_el1",		0b1100000010000000},
> +	{"sctlr_el2",		0b1110000010000000},
> +	{"sctlr_el3",		0b1111000010000000},
> +	{"sp_el0",		0b1100001000001000},
> +	{"sp_el1",		0b1110001000001000},
> +	{"spsel",		0b1100001000010000},
> +	{"spsr_abt",		0b1110001000011001},
> +	{"spsr_el1",		0b1100001000000000},
> +	{"spsr_el2",		0b1110001000000000},
> +	{"spsr_el3",		0b1111001000000000},
> +	{"spsr_fiq",		0b1110001000011011},
> +	{"spsr_irq",		0b1110001000011000},
> +	{"spsr_und",		0b1110001000011010},
> +	{"tcr_el1",		0b1100000100000010},
> +	{"tcr_el2",		0b1110000100000010},
> +	{"tcr_el3",		0b1111000100000010},
> +	{"tpidr_el0",		0b1101111010000010},
> +	{"tpidr_el1",		0b1100011010000100},
> +	{"tpidr_el2",		0b1110011010000010},
> +	{"tpidr_el3",		0b1111011010000010},
> +	{"tpidpro_el0",		0b1101111010000011},
> +	{"vbar_el1",		0b1100011000000000},
> +	{"vbar_el2",		0b1110011000000000},
> +	{"vbar_el3",		0b1111011000000000},
> +	{"vmpidr_el2",		0b1110000000000101},
> +	{"vpidr_el2",		0b1110000000000000},
> +	{"ttbr0_el1",		0b1100000100000000},
> +	{"ttbr0_el2",		0b1110000100000000},
> +	{"ttbr0_el3",		0b1111000100000000},
> +	{"ttbr1_el1",		0b1100000100000001},
> +	{"vtcr_el2",		0b1110000100001010},
> +	{"vttbr_el2",		0b1110000100001000},
> +	{NULL,			0b0000000000000000},
> +};
>  
>  /* Size(8-byte words) of the log buffer. */
> -#define RSH_SCRATCH_BUF_CTL_IDX_MAX	0x7f
> +#define RSH_SCRATCH_BUF_CTL_IDX_MASK	0x7f
> +
> +static int rsh_log_sem_lock(void)
> +{
> +	unsigned long timeout;
> +
> +	/* Take the semaphore. */
> +	timeout = jiffies + msecs_to_jiffies(100);
> +	while (readq(rsh_semaphore)) {
> +		if (time_after(jiffies, timeout))
> +			return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}
> +
> +static void rsh_log_sem_unlock(void)
> +{
> +	writeq(0, rsh_semaphore);
> +}
>  
>  static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
>  			     size_t count)
>  {
> -	int idx, num, len, size = (int)count, level = RSH_LOG_INFO;
> -	unsigned long timeout;
> +	int idx, num, len, size = (int)count, level = RSH_LOG_INFO, rc;
>  	u64 data;
>  
>  	if (!size)
> @@ -501,22 +674,20 @@ static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
>  	}
>  
>  	/* Take the semaphore. */
> -	timeout = jiffies + msecs_to_jiffies(100);
> -	while (readq(rsh_semaphore)) {
> -		if (time_after(jiffies, timeout))
> -			return -ETIMEDOUT;
> -	}
> +	rc = rsh_log_sem_lock();
> +	if (rc)
> +		return rc;
>  
>  	/* Calculate how many words are available. */
>  	num = (size + sizeof(u64) - 1) / sizeof(u64);
>  	idx = readq(rsh_scratch_buf_ctl);
> -	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MAX)
> -		num = RSH_SCRATCH_BUF_CTL_IDX_MAX - idx - 1;
> +	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MASK)
> +		num = RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1;
>  	if (num <= 0)
>  		goto done;
>  
>  	/* Write Header. */
> -	data = (RSH_LOG_TYPE << RSH_LOG_TYPE_SHIFT) |
> +	data = (BF_RSH_LOG_TYPE_MSG << RSH_LOG_TYPE_SHIFT) |
>  		((u64)num << RSH_LOG_LEN_SHIFT) |
>  		((u64)level << RSH_LOG_LEVEL_SHIFT);
>  	writeq(data, rsh_scratch_buf_data);
> @@ -528,7 +699,7 @@ static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
>  			memcpy(&data, buf, len);
>  			len = 0;
>  		} else {
> -			memcpy (&data, buf, sizeof(u64));
> +			memcpy(&data, buf, sizeof(u64));
>  			len -= sizeof(u64);
>  			buf += sizeof(u64);
>  		}
> @@ -537,24 +708,197 @@ static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
>  
>  done:
>  	/* Release the semaphore. */
> -	writeq(0, rsh_semaphore);
> +	rsh_log_sem_unlock();
>  
>  	/* Ignore the rest if no more space. */
>  	return count;
>  }
>  
> -#define MBC_DRV_ATTR(_name) DRIVER_ATTR_RW(_name)
> +static char *rsh_log_get_reg_name(u64 opcode)
> +{
> +	struct rsh_log_reg *reg = rsh_log_regs;
> +
> +	while (reg->name) {
> +		if (reg->opcode == opcode)
> +			return reg->name;
> +		reg++;
> +	}
> +
> +	return "unknown";
> +}
> +
> +static int rsh_log_show_crash(u64 hdr, char *buf)
> +{
> +	int i, module, type, len, n = 0;
> +	u32 pc, syndrome, ec;
> +	u64 opcode, data;
> +	char *p = buf;
> +
> +	module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
> +	if (module >= ARRAY_SIZE(rsh_log_mod))
> +		module = 0;
> +	type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
> +	len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> +
> +	if (type == BF_RSH_LOG_TYPE_EXCEPTION) {
> +		syndrome = BF_RSH_LOG_HEADER_GET(SYNDROME, hdr);
> +		ec = syndrome >> AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT;
> +		n = sprintf(p, " Exception(%s): syndrome = 0x%x%s\n",
> +			    rsh_log_mod[module], syndrome,
> +			    (ec == 0x24 || ec == 0x25) ? "(Data Abort)" :
> +			    (ec == 0x2f) ? "(SError)" : "");
> +	} else if (type == BF_RSH_LOG_TYPE_PANIC) {
> +		pc = BF_RSH_LOG_HEADER_GET(PC, hdr);
> +		n = sprintf(p, " PANIC(%s): PC = 0x%x\n", rsh_log_mod[module],
> +			    pc);
> +	}
> +	if (n > 0)
> +		p += n;
> +
> +	/*
> +	 * Read the registers in a loop. 'len' is the total number of words in
> +	 * 8-bytes. Two words are read in each loop.
> +	 */
> +	for (i = 0; i < len/2; i++) {
> +		opcode = readq(rsh_scratch_buf_data);
> +		data = readq(rsh_scratch_buf_data);
> +
> +		opcode = (opcode >> AARCH64_MRS_REG_SHIFT) &
> +			AARCH64_MRS_REG_MASK;
> +		n = sprintf(p, "   %-16s0x%llx\n", rsh_log_get_reg_name(opcode),
> +			    (unsigned long long)data);
> +		if (n > 0)
> +			p += n;
> +	}
> +
> +	return p - buf;
> +}
> +
> +static int rsh_log_format_msg(char *buf, const char *msg, ...)
> +{
> +	va_list args;
> +	int len;
> +
> +	va_start(args, msg);
> +	len = vsprintf(buf, msg, args);
> +	va_end(args);
> +
> +	return len;
> +}
> +
> +static int rsh_log_show_msg(u64 hdr, char *buf)
> +{
> +	int has_arg = BF_RSH_LOG_HEADER_GET(HAS_ARG, hdr);
> +	int level = BF_RSH_LOG_HEADER_GET(LEVEL, hdr);
> +	int module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
> +	int len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> +	u32 arg = BF_RSH_LOG_HEADER_GET(ARG, hdr);
> +	char *msg, *p;
> +	u64 data;
> +
> +	if (len <= 0)
> +		return -EINVAL;
> +
> +	if (module >= ARRAY_SIZE(rsh_log_mod))
> +		module = 0;
> +
> +	if (level >= ARRAY_SIZE(rsh_log_level))
> +		level = 0;
> +
> +	msg = kmalloc(len * sizeof(u64) + 1, GFP_KERNEL);
> +	if (!msg)
> +		return 0;
> +	p = msg;
> +
> +	while (len--) {
> +		data = readq(rsh_scratch_buf_data);
> +		memcpy(p, &data, sizeof(data));
> +		p += sizeof(data);
> +	}
> +	*p = '\0';
> +	if (!has_arg) {
> +		len = sprintf(buf, " %s[%s]: %s\n", rsh_log_level[level],
> +			rsh_log_mod[module], msg);
> +	} else {
> +		len = sprintf(buf, " %s[%s]: ", rsh_log_level[level],
> +			rsh_log_mod[module]);
> +		len += rsh_log_format_msg(buf + len, msg, arg);
> +		len += sprintf(buf + len, "\n");
> +	}
> +
> +	kfree(msg);
> +	return len;
> +}
> +
> +static ssize_t rsh_log_show(struct device_driver *drv, char *buf)
> +{
> +	u64 hdr;
> +	char *p = buf;
> +	int i, n, rc, idx, type, len;
> +
> +	if (!rsh_semaphore || !rsh_scratch_buf_ctl)
> +		return -EOPNOTSUPP;
> +
> +	/* Take the semaphore. */
> +	rc = rsh_log_sem_lock();
> +	if (rc)
> +		return rc;
> +
> +	/* Save the current index and read from 0. */
> +	idx = readq(rsh_scratch_buf_ctl) & RSH_SCRATCH_BUF_CTL_IDX_MASK;
> +	if (!idx)
> +		goto done;
> +	writeq(0, rsh_scratch_buf_ctl);
> +
> +	i = 0;
> +	while (i < idx) {
> +		hdr = readq(rsh_scratch_buf_data);
> +		type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
> +		len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> +		i += 1 + len;
> +		if (i > idx)
> +			break;
> +
> +		switch (type) {
> +		case BF_RSH_LOG_TYPE_PANIC:
> +		case BF_RSH_LOG_TYPE_EXCEPTION:
> +			n = rsh_log_show_crash(hdr, p);
> +			p += n;
> +			break;
> +		case BF_RSH_LOG_TYPE_MSG:
> +			n = rsh_log_show_msg(hdr, p);
> +			p += n;
> +			break;
> +		default:
> +			/* Drain this message. */
> +			while (len--)
> +				(void) readq(rsh_scratch_buf_data);
> +			break;
> +		}
> +	}
> +
> +	if (rsh_log_clear_on_read)
> +		writeq(0, rsh_scratch_buf_ctl);
> +	else
> +		writeq(idx, rsh_scratch_buf_ctl);
> +
> +done:
> +	/* Release the semaphore. */
> +	rsh_log_sem_unlock();
> +
> +	return p - buf;
> +}
>  
> -static MBC_DRV_ATTR(post_reset_wdog);
> -static MBC_DRV_ATTR(reset_action);
> -static MBC_DRV_ATTR(second_reset_action);
> +static DRIVER_ATTR_RW(post_reset_wdog);
> +static DRIVER_ATTR_RW(reset_action);
> +static DRIVER_ATTR_RW(second_reset_action);
>  static DRIVER_ATTR_RO(lifecycle_state);
>  static DRIVER_ATTR_RO(secure_boot_fuse_state);
>  static DRIVER_ATTR_WO(fw_reset);
> -static MBC_DRV_ATTR(oob_mac);
> -static MBC_DRV_ATTR(opn_str);
> +static DRIVER_ATTR_RW(oob_mac);
> +static DRIVER_ATTR_RW(opn_str);
>  static DRIVER_ATTR_WO(mfg_lock);
> -static DRIVER_ATTR_WO(rsh_log);
> +static DRIVER_ATTR_RW(rsh_log);
>  
>  static struct attribute *mbc_dev_attrs[] = {
>  	&driver_attr_post_reset_wdog.attr,
> @@ -626,7 +970,7 @@ static ssize_t mbc_bootfifo_read_raw(struct file *filp, struct kobject *kobj,
>  }
>  
>  static struct bin_attribute mbc_bootfifo_sysfs_attr = {
> -	.attr = { .name = "bootfifo", .mode = S_IRUSR },
> +	.attr = { .name = "bootfifo", .mode = 0400 },
>  	.read = mbc_bootfifo_read_raw,
>  };
>  
> @@ -720,4 +1064,4 @@ static int mbc_remove(struct platform_device *pdev)
>  MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
>  MODULE_VERSION(DRIVER_VERSION);
>  MODULE_AUTHOR("Mellanox Technologies");
> -MODULE_LICENSE("GPL");
> +MODULE_LICENSE("Dual BSD/GPL");

The same as beginning - relicensing is different than adding logging
display from sysfs.

Best regards,
Krzysztof
Liming Sun May 5, 2021, 6:47 p.m. UTC | #2
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> Sent: Wednesday, May 5, 2021 2:21 PM
> To: Liming Sun <limings@nvidia.com>; kernel-team@lists.ubuntu.com
> Subject: NAK: [SRU][F:linux-bluefield][PATCH v1 1/1] UBUNTU: SAUCE: mlx-
> bootctl: rshim logging display from linux sysfs
> 
> On 05/05/2021 13:10, Liming Sun wrote:
> > This commit adds the rshim logging display support from linux sysfs.
> > The display logic is ported from the user-space rshim driver. Module
> > parameter 'rsh_log_clear_on_read' is added to clear the the logging
> > buffer after read when this flag is set.
> >
> 
> Hi,
> 
> Thanks for your patch.
> 
> You miss the BugLink at the beginning. The example here might be helpful:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwiki.
> ubuntu.com%2FKernel%2FDev%2FStablePatchFormat&amp;data=04%7C01
> %7Climings%40nvidia.com%7C7d9f78b88aa6407802ed08d90ff28278%7C43083
> d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637558356538323405%7CUnkn
> own%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik
> 1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=eIbnxapdnwNM5AMJ8LRgqJ
> xODkyqAfDtgV8yeYt5v6w%3D&amp;reserved=0
> 
> > Example:
> >
> >   cat /sys/bus/platform/drivers/mlx-bootctl/rsh_log
> >    INFO[BL2]: start
> >    INFO[BL2]: DDR POST passed
> >    INFO[BL2]: UEFI loaded
> >    INFO[BL31]: start
> >    INFO[BL31]: runtime
> >    ...
> >
> >   echo 1 > /sys/module/mlx_bootctl/parameters/rsh_log_clear_on_read
> >
> > Change-Id: I50129fc30f9e2e7b6b38713c740968f9235b532a
> 
> Please strip out Gerrit stuff.
> 
> You missed here your own SoB.

Fixed in v2

> 
> > ---
> >  drivers/platform/mellanox/mlx-bootctl.c | 408
> +++++++++++++++++++++++++++++---
> >  1 file changed, 376 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/platform/mellanox/mlx-bootctl.c
> b/drivers/platform/mellanox/mlx-bootctl.c
> > index 7ec7c9a..4c85139 100644
> > --- a/drivers/platform/mellanox/mlx-bootctl.c
> > +++ b/drivers/platform/mellanox/mlx-bootctl.c
> > @@ -1,4 +1,4 @@
> > -// SPDX-License-Identifier: GPL-2.0
> > +// SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause
> 
> This does not look like related and usually relicensing requires SoB
> from copyright owners (e.g. involved people, their representative, the
> company).

Removed in v2.

> 
> >  /*
> >   *  Mellanox boot control driver
> >   *  This driver provides a sysfs interface for systems management
> > @@ -64,6 +64,10 @@ struct boot_name {
> >  static void __iomem *rsh_scratch_buf_ctl;
> >  static void __iomem *rsh_scratch_buf_data;
> >
> > +static int rsh_log_clear_on_read;
> > +module_param(rsh_log_clear_on_read, int, 0644);
> > +MODULE_PARM_DESC(rsh_log_clear_on_read, "Clear rshim logging
> buffer after read.");
> > +
> >  /*
> >   * Objects are stored within the MFG partition per type. Type 0 is not
> >   * supported.
> > @@ -86,7 +90,7 @@ enum {
> >   * The expected format is: "XX:XX:XX:XX:XX:XX"
> >   */
> >  #define MLNX_MFG_OOB_MAC_FORMAT_LEN \
> > -        ((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN -
> 1))
> > +	((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN
> - 1))
> 
> Does not look like related change.

Removed in v2

> 
> >
> >  /* The SMC calls in question are atomic, so we don't have to lock here. */
> >  static int smc_call1(unsigned int smc_op, int smc_arg)
> > @@ -290,7 +294,7 @@ static ssize_t fw_reset_store(struct device_driver
> *drv,
> >
> >  static ssize_t oob_mac_show(struct device_driver *drv, char *buf)
> >  {
> > -	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN] = { 0 };
> > +	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN + 1] = { 0 };
> >  	struct arm_smccc_res res;
> >  	u8 *mac_byte_ptr;
> >
> > @@ -315,14 +319,15 @@ static ssize_t oob_mac_store(struct device_driver
> *drv, const char *buf,
> >  	struct arm_smccc_res res;
> >  	u64 mac_addr = 0;
> >  	u8 *mac_byte_ptr;
> > -	int byte_idx;
> > +	int byte_idx, len;
> >
> >  	if ((count - 1) != MLNX_MFG_OOB_MAC_FORMAT_LEN)
> >  		return -EINVAL;
> >
> > -	if (MLNX_MFG_OOB_MAC_LEN != sscanf(buf,
> "%02x:%02x:%02x:%02x:%02x:%02x",
> > -					   &byte[0], &byte[1], &byte[2],
> > -					   &byte[3], &byte[4], &byte[5]))
> > +	len = sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
> > +		     &byte[0], &byte[1], &byte[2],
> > +		     &byte[3], &byte[4], &byte[5]);
> > +	if (len != MLNX_MFG_OOB_MAC_LEN)
> >  		return -EINVAL;
> >
> >  	mac_byte_ptr = (u8 *)&mac_addr;
> > @@ -340,7 +345,7 @@ static ssize_t oob_mac_store(struct device_driver
> *drv, const char *buf,
> >
> >  static u8 get_opn_type(u8 word)
> >  {
> > -	switch(word) {
> > +	switch (word) {
> 
> Does not look like related change.

This is just an issue reported by checkpatch. So fixed it as well.

> 
> >  	case 0:
> >  		return MLNX_MFG_TYPE_OPN_0;
> >  	case 1:
> > @@ -452,7 +457,32 @@ static ssize_t mfg_lock_store(struct device_driver
> *drv, const char *buf,
> >  #define RSH_LOG_LEVEL_SHIFT	0
> >
> >  /* Module ID and type used here. */
> > -#define RSH_LOG_TYPE		0x04ULL	/* message */
> > +#define BF_RSH_LOG_TYPE_UNKNOWN		0x00ULL
> > +#define BF_RSH_LOG_TYPE_PANIC		0x01ULL
> > +#define BF_RSH_LOG_TYPE_EXCEPTION	0x02ULL
> > +#define BF_RSH_LOG_TYPE_UNUSED		0x03ULL
> > +#define BF_RSH_LOG_TYPE_MSG		0x04ULL
> > +
> > +/* Utility macro. */
> > +#define BF_RSH_LOG_MOD_MASK		0x0FULL
> > +#define BF_RSH_LOG_MOD_SHIFT		60
> > +#define BF_RSH_LOG_TYPE_MASK		0x0FULL
> > +#define BF_RSH_LOG_TYPE_SHIFT		56
> > +#define BF_RSH_LOG_LEN_MASK		0x7FULL
> > +#define BF_RSH_LOG_LEN_SHIFT		48
> > +#define BF_RSH_LOG_ARG_MASK		0xFFFFFFFFULL
> > +#define BF_RSH_LOG_ARG_SHIFT		16
> > +#define BF_RSH_LOG_HAS_ARG_MASK		0xFFULL
> > +#define BF_RSH_LOG_HAS_ARG_SHIFT	8
> > +#define BF_RSH_LOG_LEVEL_MASK		0xFFULL
> > +#define BF_RSH_LOG_LEVEL_SHIFT		0
> > +#define BF_RSH_LOG_PC_MASK		0xFFFFFFFFULL
> > +#define BF_RSH_LOG_PC_SHIFT		0
> > +#define BF_RSH_LOG_SYNDROME_MASK	0xFFFFFFFFULL
> > +#define BF_RSH_LOG_SYNDROME_SHIFT	0
> > +
> > +#define BF_RSH_LOG_HEADER_GET(f, h) \
> > +	(((h) >> BF_RSH_LOG_##f##_SHIFT) & BF_RSH_LOG_##f##_MASK)
> >
> >  /* Log message level. */
> >  enum {
> > @@ -461,16 +491,159 @@ enum {
> >  	RSH_LOG_ERR
> >  };
> >
> > -const char *rsh_log_level[] = {"INFO", "WARN", "ERR"};
> > +/* Log module */
> > +const char * const rsh_log_mod[] = {
> > +	"MISC", "BL1", "BL2", "BL2R", "BL31", "UEFI"
> > +};
> > +
> > +const char *rsh_log_level[] = {"INFO", "WARN", "ERR", "ASSERT"};
> > +
> > +#define AARCH64_MRS_REG_SHIFT 5
> > +#define AARCH64_MRS_REG_MASK  0xffff
> > +#define AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT 26
> > +
> > +struct rsh_log_reg {
> > +	char *name;
> > +	u32 opcode;
> > +} rsh_log_reg;
> > +
> > +static struct rsh_log_reg rsh_log_regs[] = {
> > +	{"actlr_el1",		0b1100000010000001},
> > +	{"actlr_el2",		0b1110000010000001},
> > +	{"actlr_el3",		0b1111000010000001},
> > +	{"afsr0_el1",		0b1100001010001000},
> > +	{"afsr0_el2",		0b1110001010001000},
> > +	{"afsr0_el3",		0b1111001010001000},
> > +	{"afsr1_el1",		0b1100001010001001},
> > +	{"afsr1_el2",		0b1110001010001001},
> > +	{"afsr1_el3",		0b1111001010001001},
> > +	{"amair_el1",		0b1100010100011000},
> > +	{"amair_el2",		0b1110010100011000},
> > +	{"amair_el3",		0b1111010100011000},
> > +	{"ccsidr_el1",		0b1100100000000000},
> > +	{"clidr_el1",		0b1100100000000001},
> > +	{"cntkctl_el1",		0b1100011100001000},
> > +	{"cntp_ctl_el0",	0b1101111100010001},
> > +	{"cntp_cval_el0",	0b1101111100010010},
> > +	{"cntv_ctl_el0",	0b1101111100011001},
> > +	{"cntv_cval_el0",	0b1101111100011010},
> > +	{"contextidr_el1",	0b1100011010000001},
> > +	{"cpacr_el1",		0b1100000010000010},
> > +	{"cptr_el2",		0b1110000010001010},
> > +	{"cptr_el3",		0b1111000010001010},
> > +	{"vtcr_el2",		0b1110000100001010},
> > +	{"ctr_el0",		0b1101100000000001},
> > +	{"currentel",		0b1100001000010010},
> > +	{"dacr32_el2",		0b1110000110000000},
> > +	{"daif",		0b1101101000010001},
> > +	{"dczid_el0",		0b1101100000000111},
> > +	{"dlr_el0",		0b1101101000101001},
> > +	{"dspsr_el0",		0b1101101000101000},
> > +	{"elr_el1",		0b1100001000000001},
> > +	{"elr_el2",		0b1110001000000001},
> > +	{"elr_el3",		0b1111001000000001},
> > +	{"esr_el1",		0b1100001010010000},
> > +	{"esr_el2",		0b1110001010010000},
> > +	{"esr_el3",		0b1111001010010000},
> > +	{"esselr_el1",		0b1101000000000000},
> > +	{"far_el1",		0b1100001100000000},
> > +	{"far_el2",		0b1110001100000000},
> > +	{"far_el3",		0b1111001100000000},
> > +	{"fpcr",		0b1101101000100000},
> > +	{"fpexc32_el2",		0b1110001010011000},
> > +	{"fpsr",		0b1101101000100001},
> > +	{"hacr_el2",		0b1110000010001111},
> > +	{"har_el2",		0b1110000010001000},
> > +	{"hpfar_el2",		0b1110001100000100},
> > +	{"hstr_el2",		0b1110000010001011},
> > +	{"far_el1",		0b1100001100000000},
> > +	{"far_el2",		0b1110001100000000},
> > +	{"far_el3",		0b1111001100000000},
> > +	{"hcr_el2",		0b1110000010001000},
> > +	{"hpfar_el2",		0b1110001100000100},
> > +	{"id_aa64afr0_el1",	0b1100000000101100},
> > +	{"id_aa64afr1_el1",	0b1100000000101101},
> > +	{"id_aa64dfr0_el1",	0b1100000000101100},
> > +	{"id_aa64isar0_el1",	0b1100000000110000},
> > +	{"id_aa64isar1_el1",	0b1100000000110001},
> > +	{"id_aa64mmfr0_el1",	0b1100000000111000},
> > +	{"id_aa64mmfr1_el1",	0b1100000000111001},
> > +	{"id_aa64pfr0_el1",	0b1100000000100000},
> > +	{"id_aa64pfr1_el1",	0b1100000000100001},
> > +	{"ifsr32_el2",		0b1110001010000001},
> > +	{"isr_el1",		0b1100011000001000},
> > +	{"mair_el1",		0b1100010100010000},
> > +	{"mair_el2",		0b1110010100010000},
> > +	{"mair_el3",		0b1111010100010000},
> > +	{"midr_el1",		0b1100000000000000},
> > +	{"mpidr_el1",		0b1100000000000101},
> > +	{"nzcv",		0b1101101000010000},
> > +	{"revidr_el1",		0b1100000000000110},
> > +	{"rmr_el3",		0b1111011000000010},
> > +	{"par_el1",		0b1100001110100000},
> > +	{"rvbar_el3",		0b1111011000000001},
> > +	{"scr_el3",		0b1111000010001000},
> > +	{"sctlr_el1",		0b1100000010000000},
> > +	{"sctlr_el2",		0b1110000010000000},
> > +	{"sctlr_el3",		0b1111000010000000},
> > +	{"sp_el0",		0b1100001000001000},
> > +	{"sp_el1",		0b1110001000001000},
> > +	{"spsel",		0b1100001000010000},
> > +	{"spsr_abt",		0b1110001000011001},
> > +	{"spsr_el1",		0b1100001000000000},
> > +	{"spsr_el2",		0b1110001000000000},
> > +	{"spsr_el3",		0b1111001000000000},
> > +	{"spsr_fiq",		0b1110001000011011},
> > +	{"spsr_irq",		0b1110001000011000},
> > +	{"spsr_und",		0b1110001000011010},
> > +	{"tcr_el1",		0b1100000100000010},
> > +	{"tcr_el2",		0b1110000100000010},
> > +	{"tcr_el3",		0b1111000100000010},
> > +	{"tpidr_el0",		0b1101111010000010},
> > +	{"tpidr_el1",		0b1100011010000100},
> > +	{"tpidr_el2",		0b1110011010000010},
> > +	{"tpidr_el3",		0b1111011010000010},
> > +	{"tpidpro_el0",		0b1101111010000011},
> > +	{"vbar_el1",		0b1100011000000000},
> > +	{"vbar_el2",		0b1110011000000000},
> > +	{"vbar_el3",		0b1111011000000000},
> > +	{"vmpidr_el2",		0b1110000000000101},
> > +	{"vpidr_el2",		0b1110000000000000},
> > +	{"ttbr0_el1",		0b1100000100000000},
> > +	{"ttbr0_el2",		0b1110000100000000},
> > +	{"ttbr0_el3",		0b1111000100000000},
> > +	{"ttbr1_el1",		0b1100000100000001},
> > +	{"vtcr_el2",		0b1110000100001010},
> > +	{"vttbr_el2",		0b1110000100001000},
> > +	{NULL,			0b0000000000000000},
> > +};
> >
> >  /* Size(8-byte words) of the log buffer. */
> > -#define RSH_SCRATCH_BUF_CTL_IDX_MAX	0x7f
> > +#define RSH_SCRATCH_BUF_CTL_IDX_MASK	0x7f
> > +
> > +static int rsh_log_sem_lock(void)
> > +{
> > +	unsigned long timeout;
> > +
> > +	/* Take the semaphore. */
> > +	timeout = jiffies + msecs_to_jiffies(100);
> > +	while (readq(rsh_semaphore)) {
> > +		if (time_after(jiffies, timeout))
> > +			return -ETIMEDOUT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void rsh_log_sem_unlock(void)
> > +{
> > +	writeq(0, rsh_semaphore);
> > +}
> >
> >  static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
> >  			     size_t count)
> >  {
> > -	int idx, num, len, size = (int)count, level = RSH_LOG_INFO;
> > -	unsigned long timeout;
> > +	int idx, num, len, size = (int)count, level = RSH_LOG_INFO, rc;
> >  	u64 data;
> >
> >  	if (!size)
> > @@ -501,22 +674,20 @@ static ssize_t rsh_log_store(struct device_driver
> *drv, const char *buf,
> >  	}
> >
> >  	/* Take the semaphore. */
> > -	timeout = jiffies + msecs_to_jiffies(100);
> > -	while (readq(rsh_semaphore)) {
> > -		if (time_after(jiffies, timeout))
> > -			return -ETIMEDOUT;
> > -	}
> > +	rc = rsh_log_sem_lock();
> > +	if (rc)
> > +		return rc;
> >
> >  	/* Calculate how many words are available. */
> >  	num = (size + sizeof(u64) - 1) / sizeof(u64);
> >  	idx = readq(rsh_scratch_buf_ctl);
> > -	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MAX)
> > -		num = RSH_SCRATCH_BUF_CTL_IDX_MAX - idx - 1;
> > +	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MASK)
> > +		num = RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1;
> >  	if (num <= 0)
> >  		goto done;
> >
> >  	/* Write Header. */
> > -	data = (RSH_LOG_TYPE << RSH_LOG_TYPE_SHIFT) |
> > +	data = (BF_RSH_LOG_TYPE_MSG << RSH_LOG_TYPE_SHIFT) |
> >  		((u64)num << RSH_LOG_LEN_SHIFT) |
> >  		((u64)level << RSH_LOG_LEVEL_SHIFT);
> >  	writeq(data, rsh_scratch_buf_data);
> > @@ -528,7 +699,7 @@ static ssize_t rsh_log_store(struct device_driver
> *drv, const char *buf,
> >  			memcpy(&data, buf, len);
> >  			len = 0;
> >  		} else {
> > -			memcpy (&data, buf, sizeof(u64));
> > +			memcpy(&data, buf, sizeof(u64));
> >  			len -= sizeof(u64);
> >  			buf += sizeof(u64);
> >  		}
> > @@ -537,24 +708,197 @@ static ssize_t rsh_log_store(struct device_driver
> *drv, const char *buf,
> >
> >  done:
> >  	/* Release the semaphore. */
> > -	writeq(0, rsh_semaphore);
> > +	rsh_log_sem_unlock();
> >
> >  	/* Ignore the rest if no more space. */
> >  	return count;
> >  }
> >
> > -#define MBC_DRV_ATTR(_name) DRIVER_ATTR_RW(_name)
> > +static char *rsh_log_get_reg_name(u64 opcode)
> > +{
> > +	struct rsh_log_reg *reg = rsh_log_regs;
> > +
> > +	while (reg->name) {
> > +		if (reg->opcode == opcode)
> > +			return reg->name;
> > +		reg++;
> > +	}
> > +
> > +	return "unknown";
> > +}
> > +
> > +static int rsh_log_show_crash(u64 hdr, char *buf)
> > +{
> > +	int i, module, type, len, n = 0;
> > +	u32 pc, syndrome, ec;
> > +	u64 opcode, data;
> > +	char *p = buf;
> > +
> > +	module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
> > +	if (module >= ARRAY_SIZE(rsh_log_mod))
> > +		module = 0;
> > +	type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
> > +	len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> > +
> > +	if (type == BF_RSH_LOG_TYPE_EXCEPTION) {
> > +		syndrome = BF_RSH_LOG_HEADER_GET(SYNDROME, hdr);
> > +		ec = syndrome >>
> AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT;
> > +		n = sprintf(p, " Exception(%s): syndrome = 0x%x%s\n",
> > +			    rsh_log_mod[module], syndrome,
> > +			    (ec == 0x24 || ec == 0x25) ? "(Data Abort)" :
> > +			    (ec == 0x2f) ? "(SError)" : "");
> > +	} else if (type == BF_RSH_LOG_TYPE_PANIC) {
> > +		pc = BF_RSH_LOG_HEADER_GET(PC, hdr);
> > +		n = sprintf(p, " PANIC(%s): PC = 0x%x\n",
> rsh_log_mod[module],
> > +			    pc);
> > +	}
> > +	if (n > 0)
> > +		p += n;
> > +
> > +	/*
> > +	 * Read the registers in a loop. 'len' is the total number of words in
> > +	 * 8-bytes. Two words are read in each loop.
> > +	 */
> > +	for (i = 0; i < len/2; i++) {
> > +		opcode = readq(rsh_scratch_buf_data);
> > +		data = readq(rsh_scratch_buf_data);
> > +
> > +		opcode = (opcode >> AARCH64_MRS_REG_SHIFT) &
> > +			AARCH64_MRS_REG_MASK;
> > +		n = sprintf(p, "   %-16s0x%llx\n",
> rsh_log_get_reg_name(opcode),
> > +			    (unsigned long long)data);
> > +		if (n > 0)
> > +			p += n;
> > +	}
> > +
> > +	return p - buf;
> > +}
> > +
> > +static int rsh_log_format_msg(char *buf, const char *msg, ...)
> > +{
> > +	va_list args;
> > +	int len;
> > +
> > +	va_start(args, msg);
> > +	len = vsprintf(buf, msg, args);
> > +	va_end(args);
> > +
> > +	return len;
> > +}
> > +
> > +static int rsh_log_show_msg(u64 hdr, char *buf)
> > +{
> > +	int has_arg = BF_RSH_LOG_HEADER_GET(HAS_ARG, hdr);
> > +	int level = BF_RSH_LOG_HEADER_GET(LEVEL, hdr);
> > +	int module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
> > +	int len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> > +	u32 arg = BF_RSH_LOG_HEADER_GET(ARG, hdr);
> > +	char *msg, *p;
> > +	u64 data;
> > +
> > +	if (len <= 0)
> > +		return -EINVAL;
> > +
> > +	if (module >= ARRAY_SIZE(rsh_log_mod))
> > +		module = 0;
> > +
> > +	if (level >= ARRAY_SIZE(rsh_log_level))
> > +		level = 0;
> > +
> > +	msg = kmalloc(len * sizeof(u64) + 1, GFP_KERNEL);
> > +	if (!msg)
> > +		return 0;
> > +	p = msg;
> > +
> > +	while (len--) {
> > +		data = readq(rsh_scratch_buf_data);
> > +		memcpy(p, &data, sizeof(data));
> > +		p += sizeof(data);
> > +	}
> > +	*p = '\0';
> > +	if (!has_arg) {
> > +		len = sprintf(buf, " %s[%s]: %s\n", rsh_log_level[level],
> > +			rsh_log_mod[module], msg);
> > +	} else {
> > +		len = sprintf(buf, " %s[%s]: ", rsh_log_level[level],
> > +			rsh_log_mod[module]);
> > +		len += rsh_log_format_msg(buf + len, msg, arg);
> > +		len += sprintf(buf + len, "\n");
> > +	}
> > +
> > +	kfree(msg);
> > +	return len;
> > +}
> > +
> > +static ssize_t rsh_log_show(struct device_driver *drv, char *buf)
> > +{
> > +	u64 hdr;
> > +	char *p = buf;
> > +	int i, n, rc, idx, type, len;
> > +
> > +	if (!rsh_semaphore || !rsh_scratch_buf_ctl)
> > +		return -EOPNOTSUPP;
> > +
> > +	/* Take the semaphore. */
> > +	rc = rsh_log_sem_lock();
> > +	if (rc)
> > +		return rc;
> > +
> > +	/* Save the current index and read from 0. */
> > +	idx = readq(rsh_scratch_buf_ctl) &
> RSH_SCRATCH_BUF_CTL_IDX_MASK;
> > +	if (!idx)
> > +		goto done;
> > +	writeq(0, rsh_scratch_buf_ctl);
> > +
> > +	i = 0;
> > +	while (i < idx) {
> > +		hdr = readq(rsh_scratch_buf_data);
> > +		type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
> > +		len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
> > +		i += 1 + len;
> > +		if (i > idx)
> > +			break;
> > +
> > +		switch (type) {
> > +		case BF_RSH_LOG_TYPE_PANIC:
> > +		case BF_RSH_LOG_TYPE_EXCEPTION:
> > +			n = rsh_log_show_crash(hdr, p);
> > +			p += n;
> > +			break;
> > +		case BF_RSH_LOG_TYPE_MSG:
> > +			n = rsh_log_show_msg(hdr, p);
> > +			p += n;
> > +			break;
> > +		default:
> > +			/* Drain this message. */
> > +			while (len--)
> > +				(void) readq(rsh_scratch_buf_data);
> > +			break;
> > +		}
> > +	}
> > +
> > +	if (rsh_log_clear_on_read)
> > +		writeq(0, rsh_scratch_buf_ctl);
> > +	else
> > +		writeq(idx, rsh_scratch_buf_ctl);
> > +
> > +done:
> > +	/* Release the semaphore. */
> > +	rsh_log_sem_unlock();
> > +
> > +	return p - buf;
> > +}
> >
> > -static MBC_DRV_ATTR(post_reset_wdog);
> > -static MBC_DRV_ATTR(reset_action);
> > -static MBC_DRV_ATTR(second_reset_action);
> > +static DRIVER_ATTR_RW(post_reset_wdog);
> > +static DRIVER_ATTR_RW(reset_action);
> > +static DRIVER_ATTR_RW(second_reset_action);
> >  static DRIVER_ATTR_RO(lifecycle_state);
> >  static DRIVER_ATTR_RO(secure_boot_fuse_state);
> >  static DRIVER_ATTR_WO(fw_reset);
> > -static MBC_DRV_ATTR(oob_mac);
> > -static MBC_DRV_ATTR(opn_str);
> > +static DRIVER_ATTR_RW(oob_mac);
> > +static DRIVER_ATTR_RW(opn_str);
> >  static DRIVER_ATTR_WO(mfg_lock);
> > -static DRIVER_ATTR_WO(rsh_log);
> > +static DRIVER_ATTR_RW(rsh_log);
> >
> >  static struct attribute *mbc_dev_attrs[] = {
> >  	&driver_attr_post_reset_wdog.attr,
> > @@ -626,7 +970,7 @@ static ssize_t mbc_bootfifo_read_raw(struct file
> *filp, struct kobject *kobj,
> >  }
> >
> >  static struct bin_attribute mbc_bootfifo_sysfs_attr = {
> > -	.attr = { .name = "bootfifo", .mode = S_IRUSR },
> > +	.attr = { .name = "bootfifo", .mode = 0400 },
> >  	.read = mbc_bootfifo_read_raw,
> >  };
> >
> > @@ -720,4 +1064,4 @@ static int mbc_remove(struct platform_device
> *pdev)
> >  MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
> >  MODULE_VERSION(DRIVER_VERSION);
> >  MODULE_AUTHOR("Mellanox Technologies");
> > -MODULE_LICENSE("GPL");
> > +MODULE_LICENSE("Dual BSD/GPL");
> 
> The same as beginning - relicensing is different than adding logging
> display from sysfs.

Removed in v2.

> 
> Best regards,
> Krzysztof
Liming Sun May 5, 2021, 6:47 p.m. UTC | #3
BugLink: https://bugs.launchpad.net/bugs/1927263

SRU Justification:

[Impact]
* N/A.

[Fix]
* Add support to display rshim logging bufferthe check properly to make sure the buffer won't wrap-around.

[Test Case]
* cat /sys/bus/platform/drivers/mlx-bootctl/rsh_log.
  Example:
      cat /sys/bus/platform/drivers/mlx-bootctl/rsh_log
       INFO[BL2]: start
       INFO[BL2]: DDR POST passed
       INFO[BL2]: UEFI loaded
       INFO[BL31]: start
       INFO[BL31]: runtime
       ...

[Regression Potential]
* SQA has regressioned the driver with this change and didn't find any new issues.
diff mbox series

Patch

diff --git a/drivers/platform/mellanox/mlx-bootctl.c b/drivers/platform/mellanox/mlx-bootctl.c
index 7ec7c9a..4c85139 100644
--- a/drivers/platform/mellanox/mlx-bootctl.c
+++ b/drivers/platform/mellanox/mlx-bootctl.c
@@ -1,4 +1,4 @@ 
-// SPDX-License-Identifier: GPL-2.0
+// SPDX-License-Identifier: GPL-2.0 or BSD-3-Clause
 /*
  *  Mellanox boot control driver
  *  This driver provides a sysfs interface for systems management
@@ -64,6 +64,10 @@  struct boot_name {
 static void __iomem *rsh_scratch_buf_ctl;
 static void __iomem *rsh_scratch_buf_data;
 
+static int rsh_log_clear_on_read;
+module_param(rsh_log_clear_on_read, int, 0644);
+MODULE_PARM_DESC(rsh_log_clear_on_read, "Clear rshim logging buffer after read.");
+
 /*
  * Objects are stored within the MFG partition per type. Type 0 is not
  * supported.
@@ -86,7 +90,7 @@  enum {
  * The expected format is: "XX:XX:XX:XX:XX:XX"
  */
 #define MLNX_MFG_OOB_MAC_FORMAT_LEN \
-        ((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN - 1))
+	((MLNX_MFG_OOB_MAC_LEN * 2) + (MLNX_MFG_OOB_MAC_LEN - 1))
 
 /* The SMC calls in question are atomic, so we don't have to lock here. */
 static int smc_call1(unsigned int smc_op, int smc_arg)
@@ -290,7 +294,7 @@  static ssize_t fw_reset_store(struct device_driver *drv,
 
 static ssize_t oob_mac_show(struct device_driver *drv, char *buf)
 {
-	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN] = { 0 };
+	char mac_str[MLNX_MFG_OOB_MAC_FORMAT_LEN + 1] = { 0 };
 	struct arm_smccc_res res;
 	u8 *mac_byte_ptr;
 
@@ -315,14 +319,15 @@  static ssize_t oob_mac_store(struct device_driver *drv, const char *buf,
 	struct arm_smccc_res res;
 	u64 mac_addr = 0;
 	u8 *mac_byte_ptr;
-	int byte_idx;
+	int byte_idx, len;
 
 	if ((count - 1) != MLNX_MFG_OOB_MAC_FORMAT_LEN)
 		return -EINVAL;
 
-	if (MLNX_MFG_OOB_MAC_LEN != sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
-					   &byte[0], &byte[1], &byte[2],
-					   &byte[3], &byte[4], &byte[5]))
+	len = sscanf(buf, "%02x:%02x:%02x:%02x:%02x:%02x",
+		     &byte[0], &byte[1], &byte[2],
+		     &byte[3], &byte[4], &byte[5]);
+	if (len != MLNX_MFG_OOB_MAC_LEN)
 		return -EINVAL;
 
 	mac_byte_ptr = (u8 *)&mac_addr;
@@ -340,7 +345,7 @@  static ssize_t oob_mac_store(struct device_driver *drv, const char *buf,
 
 static u8 get_opn_type(u8 word)
 {
-	switch(word) {
+	switch (word) {
 	case 0:
 		return MLNX_MFG_TYPE_OPN_0;
 	case 1:
@@ -452,7 +457,32 @@  static ssize_t mfg_lock_store(struct device_driver *drv, const char *buf,
 #define RSH_LOG_LEVEL_SHIFT	0
 
 /* Module ID and type used here. */
-#define RSH_LOG_TYPE		0x04ULL	/* message */
+#define BF_RSH_LOG_TYPE_UNKNOWN		0x00ULL
+#define BF_RSH_LOG_TYPE_PANIC		0x01ULL
+#define BF_RSH_LOG_TYPE_EXCEPTION	0x02ULL
+#define BF_RSH_LOG_TYPE_UNUSED		0x03ULL
+#define BF_RSH_LOG_TYPE_MSG		0x04ULL
+
+/* Utility macro. */
+#define BF_RSH_LOG_MOD_MASK		0x0FULL
+#define BF_RSH_LOG_MOD_SHIFT		60
+#define BF_RSH_LOG_TYPE_MASK		0x0FULL
+#define BF_RSH_LOG_TYPE_SHIFT		56
+#define BF_RSH_LOG_LEN_MASK		0x7FULL
+#define BF_RSH_LOG_LEN_SHIFT		48
+#define BF_RSH_LOG_ARG_MASK		0xFFFFFFFFULL
+#define BF_RSH_LOG_ARG_SHIFT		16
+#define BF_RSH_LOG_HAS_ARG_MASK		0xFFULL
+#define BF_RSH_LOG_HAS_ARG_SHIFT	8
+#define BF_RSH_LOG_LEVEL_MASK		0xFFULL
+#define BF_RSH_LOG_LEVEL_SHIFT		0
+#define BF_RSH_LOG_PC_MASK		0xFFFFFFFFULL
+#define BF_RSH_LOG_PC_SHIFT		0
+#define BF_RSH_LOG_SYNDROME_MASK	0xFFFFFFFFULL
+#define BF_RSH_LOG_SYNDROME_SHIFT	0
+
+#define BF_RSH_LOG_HEADER_GET(f, h) \
+	(((h) >> BF_RSH_LOG_##f##_SHIFT) & BF_RSH_LOG_##f##_MASK)
 
 /* Log message level. */
 enum {
@@ -461,16 +491,159 @@  enum {
 	RSH_LOG_ERR
 };
 
-const char *rsh_log_level[] = {"INFO", "WARN", "ERR"};
+/* Log module */
+const char * const rsh_log_mod[] = {
+	"MISC", "BL1", "BL2", "BL2R", "BL31", "UEFI"
+};
+
+const char *rsh_log_level[] = {"INFO", "WARN", "ERR", "ASSERT"};
+
+#define AARCH64_MRS_REG_SHIFT 5
+#define AARCH64_MRS_REG_MASK  0xffff
+#define AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT 26
+
+struct rsh_log_reg {
+	char *name;
+	u32 opcode;
+} rsh_log_reg;
+
+static struct rsh_log_reg rsh_log_regs[] = {
+	{"actlr_el1",		0b1100000010000001},
+	{"actlr_el2",		0b1110000010000001},
+	{"actlr_el3",		0b1111000010000001},
+	{"afsr0_el1",		0b1100001010001000},
+	{"afsr0_el2",		0b1110001010001000},
+	{"afsr0_el3",		0b1111001010001000},
+	{"afsr1_el1",		0b1100001010001001},
+	{"afsr1_el2",		0b1110001010001001},
+	{"afsr1_el3",		0b1111001010001001},
+	{"amair_el1",		0b1100010100011000},
+	{"amair_el2",		0b1110010100011000},
+	{"amair_el3",		0b1111010100011000},
+	{"ccsidr_el1",		0b1100100000000000},
+	{"clidr_el1",		0b1100100000000001},
+	{"cntkctl_el1",		0b1100011100001000},
+	{"cntp_ctl_el0",	0b1101111100010001},
+	{"cntp_cval_el0",	0b1101111100010010},
+	{"cntv_ctl_el0",	0b1101111100011001},
+	{"cntv_cval_el0",	0b1101111100011010},
+	{"contextidr_el1",	0b1100011010000001},
+	{"cpacr_el1",		0b1100000010000010},
+	{"cptr_el2",		0b1110000010001010},
+	{"cptr_el3",		0b1111000010001010},
+	{"vtcr_el2",		0b1110000100001010},
+	{"ctr_el0",		0b1101100000000001},
+	{"currentel",		0b1100001000010010},
+	{"dacr32_el2",		0b1110000110000000},
+	{"daif",		0b1101101000010001},
+	{"dczid_el0",		0b1101100000000111},
+	{"dlr_el0",		0b1101101000101001},
+	{"dspsr_el0",		0b1101101000101000},
+	{"elr_el1",		0b1100001000000001},
+	{"elr_el2",		0b1110001000000001},
+	{"elr_el3",		0b1111001000000001},
+	{"esr_el1",		0b1100001010010000},
+	{"esr_el2",		0b1110001010010000},
+	{"esr_el3",		0b1111001010010000},
+	{"esselr_el1",		0b1101000000000000},
+	{"far_el1",		0b1100001100000000},
+	{"far_el2",		0b1110001100000000},
+	{"far_el3",		0b1111001100000000},
+	{"fpcr",		0b1101101000100000},
+	{"fpexc32_el2",		0b1110001010011000},
+	{"fpsr",		0b1101101000100001},
+	{"hacr_el2",		0b1110000010001111},
+	{"har_el2",		0b1110000010001000},
+	{"hpfar_el2",		0b1110001100000100},
+	{"hstr_el2",		0b1110000010001011},
+	{"far_el1",		0b1100001100000000},
+	{"far_el2",		0b1110001100000000},
+	{"far_el3",		0b1111001100000000},
+	{"hcr_el2",		0b1110000010001000},
+	{"hpfar_el2",		0b1110001100000100},
+	{"id_aa64afr0_el1",	0b1100000000101100},
+	{"id_aa64afr1_el1",	0b1100000000101101},
+	{"id_aa64dfr0_el1",	0b1100000000101100},
+	{"id_aa64isar0_el1",	0b1100000000110000},
+	{"id_aa64isar1_el1",	0b1100000000110001},
+	{"id_aa64mmfr0_el1",	0b1100000000111000},
+	{"id_aa64mmfr1_el1",	0b1100000000111001},
+	{"id_aa64pfr0_el1",	0b1100000000100000},
+	{"id_aa64pfr1_el1",	0b1100000000100001},
+	{"ifsr32_el2",		0b1110001010000001},
+	{"isr_el1",		0b1100011000001000},
+	{"mair_el1",		0b1100010100010000},
+	{"mair_el2",		0b1110010100010000},
+	{"mair_el3",		0b1111010100010000},
+	{"midr_el1",		0b1100000000000000},
+	{"mpidr_el1",		0b1100000000000101},
+	{"nzcv",		0b1101101000010000},
+	{"revidr_el1",		0b1100000000000110},
+	{"rmr_el3",		0b1111011000000010},
+	{"par_el1",		0b1100001110100000},
+	{"rvbar_el3",		0b1111011000000001},
+	{"scr_el3",		0b1111000010001000},
+	{"sctlr_el1",		0b1100000010000000},
+	{"sctlr_el2",		0b1110000010000000},
+	{"sctlr_el3",		0b1111000010000000},
+	{"sp_el0",		0b1100001000001000},
+	{"sp_el1",		0b1110001000001000},
+	{"spsel",		0b1100001000010000},
+	{"spsr_abt",		0b1110001000011001},
+	{"spsr_el1",		0b1100001000000000},
+	{"spsr_el2",		0b1110001000000000},
+	{"spsr_el3",		0b1111001000000000},
+	{"spsr_fiq",		0b1110001000011011},
+	{"spsr_irq",		0b1110001000011000},
+	{"spsr_und",		0b1110001000011010},
+	{"tcr_el1",		0b1100000100000010},
+	{"tcr_el2",		0b1110000100000010},
+	{"tcr_el3",		0b1111000100000010},
+	{"tpidr_el0",		0b1101111010000010},
+	{"tpidr_el1",		0b1100011010000100},
+	{"tpidr_el2",		0b1110011010000010},
+	{"tpidr_el3",		0b1111011010000010},
+	{"tpidpro_el0",		0b1101111010000011},
+	{"vbar_el1",		0b1100011000000000},
+	{"vbar_el2",		0b1110011000000000},
+	{"vbar_el3",		0b1111011000000000},
+	{"vmpidr_el2",		0b1110000000000101},
+	{"vpidr_el2",		0b1110000000000000},
+	{"ttbr0_el1",		0b1100000100000000},
+	{"ttbr0_el2",		0b1110000100000000},
+	{"ttbr0_el3",		0b1111000100000000},
+	{"ttbr1_el1",		0b1100000100000001},
+	{"vtcr_el2",		0b1110000100001010},
+	{"vttbr_el2",		0b1110000100001000},
+	{NULL,			0b0000000000000000},
+};
 
 /* Size(8-byte words) of the log buffer. */
-#define RSH_SCRATCH_BUF_CTL_IDX_MAX	0x7f
+#define RSH_SCRATCH_BUF_CTL_IDX_MASK	0x7f
+
+static int rsh_log_sem_lock(void)
+{
+	unsigned long timeout;
+
+	/* Take the semaphore. */
+	timeout = jiffies + msecs_to_jiffies(100);
+	while (readq(rsh_semaphore)) {
+		if (time_after(jiffies, timeout))
+			return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static void rsh_log_sem_unlock(void)
+{
+	writeq(0, rsh_semaphore);
+}
 
 static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
 			     size_t count)
 {
-	int idx, num, len, size = (int)count, level = RSH_LOG_INFO;
-	unsigned long timeout;
+	int idx, num, len, size = (int)count, level = RSH_LOG_INFO, rc;
 	u64 data;
 
 	if (!size)
@@ -501,22 +674,20 @@  static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
 	}
 
 	/* Take the semaphore. */
-	timeout = jiffies + msecs_to_jiffies(100);
-	while (readq(rsh_semaphore)) {
-		if (time_after(jiffies, timeout))
-			return -ETIMEDOUT;
-	}
+	rc = rsh_log_sem_lock();
+	if (rc)
+		return rc;
 
 	/* Calculate how many words are available. */
 	num = (size + sizeof(u64) - 1) / sizeof(u64);
 	idx = readq(rsh_scratch_buf_ctl);
-	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MAX)
-		num = RSH_SCRATCH_BUF_CTL_IDX_MAX - idx - 1;
+	if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MASK)
+		num = RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1;
 	if (num <= 0)
 		goto done;
 
 	/* Write Header. */
-	data = (RSH_LOG_TYPE << RSH_LOG_TYPE_SHIFT) |
+	data = (BF_RSH_LOG_TYPE_MSG << RSH_LOG_TYPE_SHIFT) |
 		((u64)num << RSH_LOG_LEN_SHIFT) |
 		((u64)level << RSH_LOG_LEVEL_SHIFT);
 	writeq(data, rsh_scratch_buf_data);
@@ -528,7 +699,7 @@  static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
 			memcpy(&data, buf, len);
 			len = 0;
 		} else {
-			memcpy (&data, buf, sizeof(u64));
+			memcpy(&data, buf, sizeof(u64));
 			len -= sizeof(u64);
 			buf += sizeof(u64);
 		}
@@ -537,24 +708,197 @@  static ssize_t rsh_log_store(struct device_driver *drv, const char *buf,
 
 done:
 	/* Release the semaphore. */
-	writeq(0, rsh_semaphore);
+	rsh_log_sem_unlock();
 
 	/* Ignore the rest if no more space. */
 	return count;
 }
 
-#define MBC_DRV_ATTR(_name) DRIVER_ATTR_RW(_name)
+static char *rsh_log_get_reg_name(u64 opcode)
+{
+	struct rsh_log_reg *reg = rsh_log_regs;
+
+	while (reg->name) {
+		if (reg->opcode == opcode)
+			return reg->name;
+		reg++;
+	}
+
+	return "unknown";
+}
+
+static int rsh_log_show_crash(u64 hdr, char *buf)
+{
+	int i, module, type, len, n = 0;
+	u32 pc, syndrome, ec;
+	u64 opcode, data;
+	char *p = buf;
+
+	module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
+	if (module >= ARRAY_SIZE(rsh_log_mod))
+		module = 0;
+	type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
+	len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
+
+	if (type == BF_RSH_LOG_TYPE_EXCEPTION) {
+		syndrome = BF_RSH_LOG_HEADER_GET(SYNDROME, hdr);
+		ec = syndrome >> AARCH64_ESR_ELX_EXCEPTION_CLASS_SHIFT;
+		n = sprintf(p, " Exception(%s): syndrome = 0x%x%s\n",
+			    rsh_log_mod[module], syndrome,
+			    (ec == 0x24 || ec == 0x25) ? "(Data Abort)" :
+			    (ec == 0x2f) ? "(SError)" : "");
+	} else if (type == BF_RSH_LOG_TYPE_PANIC) {
+		pc = BF_RSH_LOG_HEADER_GET(PC, hdr);
+		n = sprintf(p, " PANIC(%s): PC = 0x%x\n", rsh_log_mod[module],
+			    pc);
+	}
+	if (n > 0)
+		p += n;
+
+	/*
+	 * Read the registers in a loop. 'len' is the total number of words in
+	 * 8-bytes. Two words are read in each loop.
+	 */
+	for (i = 0; i < len/2; i++) {
+		opcode = readq(rsh_scratch_buf_data);
+		data = readq(rsh_scratch_buf_data);
+
+		opcode = (opcode >> AARCH64_MRS_REG_SHIFT) &
+			AARCH64_MRS_REG_MASK;
+		n = sprintf(p, "   %-16s0x%llx\n", rsh_log_get_reg_name(opcode),
+			    (unsigned long long)data);
+		if (n > 0)
+			p += n;
+	}
+
+	return p - buf;
+}
+
+static int rsh_log_format_msg(char *buf, const char *msg, ...)
+{
+	va_list args;
+	int len;
+
+	va_start(args, msg);
+	len = vsprintf(buf, msg, args);
+	va_end(args);
+
+	return len;
+}
+
+static int rsh_log_show_msg(u64 hdr, char *buf)
+{
+	int has_arg = BF_RSH_LOG_HEADER_GET(HAS_ARG, hdr);
+	int level = BF_RSH_LOG_HEADER_GET(LEVEL, hdr);
+	int module = BF_RSH_LOG_HEADER_GET(MOD, hdr);
+	int len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
+	u32 arg = BF_RSH_LOG_HEADER_GET(ARG, hdr);
+	char *msg, *p;
+	u64 data;
+
+	if (len <= 0)
+		return -EINVAL;
+
+	if (module >= ARRAY_SIZE(rsh_log_mod))
+		module = 0;
+
+	if (level >= ARRAY_SIZE(rsh_log_level))
+		level = 0;
+
+	msg = kmalloc(len * sizeof(u64) + 1, GFP_KERNEL);
+	if (!msg)
+		return 0;
+	p = msg;
+
+	while (len--) {
+		data = readq(rsh_scratch_buf_data);
+		memcpy(p, &data, sizeof(data));
+		p += sizeof(data);
+	}
+	*p = '\0';
+	if (!has_arg) {
+		len = sprintf(buf, " %s[%s]: %s\n", rsh_log_level[level],
+			rsh_log_mod[module], msg);
+	} else {
+		len = sprintf(buf, " %s[%s]: ", rsh_log_level[level],
+			rsh_log_mod[module]);
+		len += rsh_log_format_msg(buf + len, msg, arg);
+		len += sprintf(buf + len, "\n");
+	}
+
+	kfree(msg);
+	return len;
+}
+
+static ssize_t rsh_log_show(struct device_driver *drv, char *buf)
+{
+	u64 hdr;
+	char *p = buf;
+	int i, n, rc, idx, type, len;
+
+	if (!rsh_semaphore || !rsh_scratch_buf_ctl)
+		return -EOPNOTSUPP;
+
+	/* Take the semaphore. */
+	rc = rsh_log_sem_lock();
+	if (rc)
+		return rc;
+
+	/* Save the current index and read from 0. */
+	idx = readq(rsh_scratch_buf_ctl) & RSH_SCRATCH_BUF_CTL_IDX_MASK;
+	if (!idx)
+		goto done;
+	writeq(0, rsh_scratch_buf_ctl);
+
+	i = 0;
+	while (i < idx) {
+		hdr = readq(rsh_scratch_buf_data);
+		type = BF_RSH_LOG_HEADER_GET(TYPE, hdr);
+		len = BF_RSH_LOG_HEADER_GET(LEN, hdr);
+		i += 1 + len;
+		if (i > idx)
+			break;
+
+		switch (type) {
+		case BF_RSH_LOG_TYPE_PANIC:
+		case BF_RSH_LOG_TYPE_EXCEPTION:
+			n = rsh_log_show_crash(hdr, p);
+			p += n;
+			break;
+		case BF_RSH_LOG_TYPE_MSG:
+			n = rsh_log_show_msg(hdr, p);
+			p += n;
+			break;
+		default:
+			/* Drain this message. */
+			while (len--)
+				(void) readq(rsh_scratch_buf_data);
+			break;
+		}
+	}
+
+	if (rsh_log_clear_on_read)
+		writeq(0, rsh_scratch_buf_ctl);
+	else
+		writeq(idx, rsh_scratch_buf_ctl);
+
+done:
+	/* Release the semaphore. */
+	rsh_log_sem_unlock();
+
+	return p - buf;
+}
 
-static MBC_DRV_ATTR(post_reset_wdog);
-static MBC_DRV_ATTR(reset_action);
-static MBC_DRV_ATTR(second_reset_action);
+static DRIVER_ATTR_RW(post_reset_wdog);
+static DRIVER_ATTR_RW(reset_action);
+static DRIVER_ATTR_RW(second_reset_action);
 static DRIVER_ATTR_RO(lifecycle_state);
 static DRIVER_ATTR_RO(secure_boot_fuse_state);
 static DRIVER_ATTR_WO(fw_reset);
-static MBC_DRV_ATTR(oob_mac);
-static MBC_DRV_ATTR(opn_str);
+static DRIVER_ATTR_RW(oob_mac);
+static DRIVER_ATTR_RW(opn_str);
 static DRIVER_ATTR_WO(mfg_lock);
-static DRIVER_ATTR_WO(rsh_log);
+static DRIVER_ATTR_RW(rsh_log);
 
 static struct attribute *mbc_dev_attrs[] = {
 	&driver_attr_post_reset_wdog.attr,
@@ -626,7 +970,7 @@  static ssize_t mbc_bootfifo_read_raw(struct file *filp, struct kobject *kobj,
 }
 
 static struct bin_attribute mbc_bootfifo_sysfs_attr = {
-	.attr = { .name = "bootfifo", .mode = S_IRUSR },
+	.attr = { .name = "bootfifo", .mode = 0400 },
 	.read = mbc_bootfifo_read_raw,
 };
 
@@ -720,4 +1064,4 @@  static int mbc_remove(struct platform_device *pdev)
 MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
 MODULE_VERSION(DRIVER_VERSION);
 MODULE_AUTHOR("Mellanox Technologies");
-MODULE_LICENSE("GPL");
+MODULE_LICENSE("Dual BSD/GPL");