diff mbox series

[v3,1/1] mlx-bootctl: rshim logging display from linux sysfs

Message ID 1276a0dd5bf5b9d8dd0d33206b1436cc4ff64fd1.1620304357.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 6, 2021, 12:35 p.m. UTC 
BugLink: https://bugs.launchpad.net/bugs/1927263

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

Signed-off-by: Liming Sun <limings@nvidia.com>
---
 drivers/platform/mellanox/mlx-bootctl.c | 402 +++++++++++++++++++++++++++++---
 1 file changed, 373 insertions(+), 29 deletions(-)

Comments

Tim Gardner May 7, 2021, 2:19 p.m. UTC | #1 
Acked-by: Tim Gardner <tim.gardner@canonical.com>

There are a couple of places you should be using snprintf() instead of 
sprintf() in order to avoid buffer overflows. On the other hand, I 
assume this is primarily a debug interface and will only cause problems 
when used, i.e. normal run time is unaffected.

On 5/6/21 6:35 AM, Liming Sun wrote:
> BugLink: https://bugs.launchpad.net/bugs/1927263
> 
> 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
> 
> Signed-off-by: Liming Sun <limings@nvidia.com>
> ---
>   drivers/platform/mellanox/mlx-bootctl.c | 402 +++++++++++++++++++++++++++++---
>   1 file changed, 373 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlx-bootctl.c b/drivers/platform/mellanox/mlx-bootctl.c
> index 7ec7c9a..6eff360 100644
> --- a/drivers/platform/mellanox/mlx-bootctl.c
> +++ b/drivers/platform/mellanox/mlx-bootctl.c
> @@ -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.
> @@ -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,
>   };
>   
>
Liming Sun May 7, 2021, 2:37 p.m. UTC | #2 
Thanks. Yes, it's a debug interface.

This HW logging buffer is only 1KB, much less than the size of the sysfs attribute file (4KB at least?)
So the size should be ok. But you're absolutely correct that using snprintf() will make it more robust. 
We'll improve it later and send out the patch in separate commits.

- Liming

> -----Original Message-----
> From: Tim Gardner <tim.gardner@canonical.com>
> Sent: Friday, May 7, 2021 10:19 AM
> To: Liming Sun <limings@nvidia.com>; kernel-team@lists.ubuntu.com
> Subject: ACK/Cmnt: [PATCH v3 1/1] mlx-bootctl: rshim logging display from
> linux sysfs
> 
> Acked-by: Tim Gardner <tim.gardner@canonical.com>
> 
> There are a couple of places you should be using snprintf() instead of
> sprintf() in order to avoid buffer overflows. On the other hand, I
> assume this is primarily a debug interface and will only cause problems
> when used, i.e. normal run time is unaffected.
> 
> On 5/6/21 6:35 AM, Liming Sun wrote:
> > BugLink:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs
> .launchpad.net%2Fbugs%2F1927263&amp;data=04%7C01%7Climings%40nvid
> ia.com%7C669460f552614fb6022808d911632356%7C43083d15727340c1b7db3
> 9efd9ccc17a%7C0%7C0%7C637559939761797400%7CUnknown%7CTWFpbGZs
> b3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn
> 0%3D%7C1000&amp;sdata=Yq8TyefaGJICBbLH9kQ37%2F0bmASiDn80heP8R
> Aajc9c%3D&amp;reserved=0
> >
> > 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
> >
> > Signed-off-by: Liming Sun <limings@nvidia.com>
> > ---
> >   drivers/platform/mellanox/mlx-bootctl.c | 402
> +++++++++++++++++++++++++++++---
> >   1 file changed, 373 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/platform/mellanox/mlx-bootctl.c
> b/drivers/platform/mellanox/mlx-bootctl.c
> > index 7ec7c9a..6eff360 100644
> > --- a/drivers/platform/mellanox/mlx-bootctl.c
> > +++ b/drivers/platform/mellanox/mlx-bootctl.c
> > @@ -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.
> > @@ -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,
> >   };
> >
> >
> 
> --
> -----------
> Tim Gardner
> Canonical, Inc
Stefan Bader May 11, 2021, 8:02 a.m. UTC | #3 
On 06.05.21 14:35, Liming Sun wrote:
> BugLink: https://bugs.launchpad.net/bugs/1927263
> 
> 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
> 
> Signed-off-by: Liming Sun <limings@nvidia.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---

Just as a word of warning, I am not sure what happened here but in my review 
inbox on Thunderbird this thread looks quite confusing. There is 2 cover emails 
for v3 and one "v1 3/1" patch. I am replying to the one v3 patch which I believe 
is the one we should be applying. If that is wrong, please let us know.

-Stefan

>   drivers/platform/mellanox/mlx-bootctl.c | 402 +++++++++++++++++++++++++++++---
>   1 file changed, 373 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/platform/mellanox/mlx-bootctl.c b/drivers/platform/mellanox/mlx-bootctl.c
> index 7ec7c9a..6eff360 100644
> --- a/drivers/platform/mellanox/mlx-bootctl.c
> +++ b/drivers/platform/mellanox/mlx-bootctl.c
> @@ -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.
> @@ -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,
>   };
>   
>
Liming Sun May 11, 2021, 11:09 a.m. UTC | #4 
> -----Original Message-----
> From: Stefan Bader <stefan.bader@canonical.com>
> Sent: Tuesday, May 11, 2021 4:03 AM
> To: Liming Sun <limings@nvidia.com>; kernel-team@lists.ubuntu.com
> Subject: ACK/Cmnt: [PATCH v3 1/1] mlx-bootctl: rshim logging display from
> linux sysfs
> 
> On 06.05.21 14:35, Liming Sun wrote:
> > BugLink: https://bugs.launchpad.net/bugs/1927263
> >
> > 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
> >
> > Signed-off-by: Liming Sun <limings@nvidia.com>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
> > ---
> 
> Just as a word of warning, I am not sure what happened here but in my
> review
> inbox on Thunderbird this thread looks quite confusing. There is 2 cover
> emails
> for v3 and one "v1 3/1" patch. I am replying to the one v3 patch which I
> believe
> is the one we should be applying. If that is wrong, please let us know.
> 
> -Stefan

Thanks! I am not sure what happened either. Probably the cover letter
missed some information and I submit it again by accident? Not sure
exactly now. If the cover letter has all the information needed it
should be good. Next time I'll pay more attention.

> 
> >   drivers/platform/mellanox/mlx-bootctl.c | 402
> +++++++++++++++++++++++++++++---
> >   1 file changed, 373 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/platform/mellanox/mlx-bootctl.c
> b/drivers/platform/mellanox/mlx-bootctl.c
> > index 7ec7c9a..6eff360 100644
> > --- a/drivers/platform/mellanox/mlx-bootctl.c
> > +++ b/drivers/platform/mellanox/mlx-bootctl.c
> > @@ -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.
> > @@ -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,
> >   };
> >
> >
>
diff mbox series

Patch

diff --git a/drivers/platform/mellanox/mlx-bootctl.c b/drivers/platform/mellanox/mlx-bootctl.c
index 7ec7c9a..6eff360 100644
--- a/drivers/platform/mellanox/mlx-bootctl.c
+++ b/drivers/platform/mellanox/mlx-bootctl.c
@@ -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.
@@ -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,
 };