This commit adds sysfs interface to be used to write into the
boot log which is 1KB HW buffer on BlueField SoC. The same log
buffer is also used by firmware code like ATF/UEFI, and can be
displayed by userspace tools or from external host via USB/PCIe.
Signed-off-by: Liming Sun <[email protected]>
Reviewed-by: Vadim Pasternak <[email protected]>
---
.../testing/sysfs-platform-mellanox-bootctl | 7 +
drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
2 files changed, 148 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
index 9b99a81babb1..239184db57dd 100644
--- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
+++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
@@ -75,3 +75,10 @@ KernelVersion: 6.4
Contact: "Liming Sun <[email protected]>"
Description:
The file used to access the BlueField boot fifo.
+
+What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
+Date: May 2023
+KernelVersion: 6.4
+Contact: "Liming Sun <[email protected]>"
+Description:
+ The file used to access the BlueField boot log.
diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
index 1bad1d278672..c02062cb3128 100644
--- a/drivers/platform/mellanox/mlxbf-bootctl.c
+++ b/drivers/platform/mellanox/mlxbf-bootctl.c
@@ -45,10 +45,38 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
[3] = "RMA",
};
+/* Log header format. */
+#define MLXBF_RSH_LOG_TYPE_SHIFT 56
+#define MLXBF_RSH_LOG_LEN_SHIFT 48
+#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
+
+/* Log module ID and type (only MSG type in Linux driver for now). */
+#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
+
+/* Log ctl/data register offset. */
+#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
+#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
+
+/* Log message levels. */
+enum {
+ MLXBF_RSH_LOG_INFO,
+ MLXBF_RSH_LOG_WARN,
+ MLXBF_RSH_LOG_ERR
+};
+
/* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
static void __iomem *mlxbf_rsh_boot_data;
static void __iomem *mlxbf_rsh_boot_cnt;
+/* Mapped pointer for rsh log semaphore/ctrl/data register. */
+static void __iomem *mlxbf_rsh_semaphore;
+static void __iomem *mlxbf_rsh_scratch_buf_ctl;
+static void __iomem *mlxbf_rsh_scratch_buf_data;
+
+/* Rsh log levels. */
+static const char * const mlxbf_rsh_log_level[] = {
+ "INFO", "WARN", "ERR", "ASSERT"};
+
/* ARM SMC call which is atomic and no need for lock. */
static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
{
@@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
return count;
}
+/* Size(8-byte words) of the log buffer. */
+#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
+
+static int mlxbf_rsh_log_sem_lock(void)
+{
+ unsigned long timeout;
+
+ /* Take the semaphore. */
+ timeout = jiffies + msecs_to_jiffies(100);
+ while (readq(mlxbf_rsh_semaphore)) {
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static void mlxbf_rsh_log_sem_unlock(void)
+{
+ writeq(0, mlxbf_rsh_semaphore);
+}
+
+static ssize_t rsh_log_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
+ u64 data;
+
+ if (!size)
+ return -EINVAL;
+
+ if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
+ return -EOPNOTSUPP;
+
+ /* Ignore line break at the end. */
+ if (buf[size - 1] == 0xa)
+ size--;
+
+ /* Check the message prefix. */
+ for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
+ len = strlen(mlxbf_rsh_log_level[idx]);
+ if (len + 1 < size &&
+ !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
+ buf += len + 1;
+ size -= len + 1;
+ level = idx;
+ break;
+ }
+ }
+
+ /* Ignore leading spaces. */
+ while (size > 0 && buf[0] == ' ') {
+ size--;
+ buf++;
+ }
+
+ /* Take the semaphore. */
+ rc = mlxbf_rsh_log_sem_lock();
+ if (rc)
+ return rc;
+
+ /* Calculate how many words are available. */
+ num = (size + sizeof(u64) - 1) / sizeof(u64);
+ idx = readq(mlxbf_rsh_scratch_buf_ctl);
+ 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 = (MLXBF_RSH_LOG_TYPE_MSG << MLXBF_RSH_LOG_TYPE_SHIFT) |
+ ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
+ ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+
+ /* Write message. */
+ for (idx = 0, len = size; idx < num && len > 0; idx++) {
+ if (len <= sizeof(u64)) {
+ data = 0;
+ memcpy(&data, buf, len);
+ len = 0;
+ } else {
+ memcpy(&data, buf, sizeof(u64));
+ len -= sizeof(u64);
+ buf += sizeof(u64);
+ }
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+ }
+
+done:
+ /* Release the semaphore. */
+ mlxbf_rsh_log_sem_unlock();
+
+ /* Ignore the rest if no more space. */
+ return count;
+}
+
static DEVICE_ATTR_RW(post_reset_wdog);
static DEVICE_ATTR_RW(reset_action);
static DEVICE_ATTR_RW(second_reset_action);
static DEVICE_ATTR_RO(lifecycle_state);
static DEVICE_ATTR_RO(secure_boot_fuse_state);
static DEVICE_ATTR_WO(fw_reset);
+static DEVICE_ATTR_WO(rsh_log);
static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_post_reset_wdog.attr,
@@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_lifecycle_state.attr,
&dev_attr_secure_boot_fuse_state.attr,
&dev_attr_fw_reset.attr,
+ &dev_attr_rsh_log.attr,
NULL
};
@@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
static int mlxbf_bootctl_probe(struct platform_device *pdev)
{
struct arm_smccc_res res = { 0 };
+ void __iomem *reg;
guid_t guid;
int ret;
@@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct platform_device *pdev)
if (IS_ERR(mlxbf_rsh_boot_cnt))
return PTR_ERR(mlxbf_rsh_boot_cnt);
+ /* Get the resource of the bootfifo counter register. */
+ mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(mlxbf_rsh_semaphore))
+ return PTR_ERR(mlxbf_rsh_semaphore);
+
+ /* Get the resource of the bootfifo counter register. */
+ reg = devm_platform_ioremap_resource(pdev, 3);
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+ mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
+ mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
+
/* Ensure we have the UUID we expect for this service. */
arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
--
2.30.1
Hi,
On 5/8/23 20:01, Liming Sun wrote:
> This commit adds sysfs interface to be used to write into the
> boot log which is 1KB HW buffer on BlueField SoC. The same log
> buffer is also used by firmware code like ATF/UEFI, and can be
> displayed by userspace tools or from external host via USB/PCIe.
>
> Signed-off-by: Liming Sun <[email protected]>
> Reviewed-by: Vadim Pasternak <[email protected]>
Vadim, David, I would appreciate if one of you could also
take a look at this patch.
> ---
> .../testing/sysfs-platform-mellanox-bootctl | 7 +
> drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> 2 files changed, 148 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> index 9b99a81babb1..239184db57dd 100644
> --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> @@ -75,3 +75,10 @@ KernelVersion: 6.4
> Contact: "Liming Sun <[email protected]>"
> Description:
> The file used to access the BlueField boot fifo.
> +
> +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> +Date: May 2023
> +KernelVersion: 6.4
> +Contact: "Liming Sun <[email protected]>"
> +Description:
> + The file used to access the BlueField boot log.
Access suggests reading, but this is a write-only file please make this more clear.
Also please document the accepted prefixes for log levels here / document
the log format.
> diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
> index 1bad1d278672..c02062cb3128 100644
> --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> @@ -45,10 +45,38 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
> [3] = "RMA",
> };
>
> +/* Log header format. */
> +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
> +
> +/* Log module ID and type (only MSG type in Linux driver for now). */
> +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> +
> +/* Log ctl/data register offset. */
> +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> +
> +/* Log message levels. */
> +enum {
> + MLXBF_RSH_LOG_INFO,
> + MLXBF_RSH_LOG_WARN,
> + MLXBF_RSH_LOG_ERR
> +};
> +
> /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
> static void __iomem *mlxbf_rsh_boot_data;
> static void __iomem *mlxbf_rsh_boot_cnt;
>
> +/* Mapped pointer for rsh log semaphore/ctrl/data register. */
> +static void __iomem *mlxbf_rsh_semaphore;
> +static void __iomem *mlxbf_rsh_scratch_buf_ctl;
> +static void __iomem *mlxbf_rsh_scratch_buf_data;
> +
> +/* Rsh log levels. */
> +static const char * const mlxbf_rsh_log_level[] = {
> + "INFO", "WARN", "ERR", "ASSERT"};
> +
> /* ARM SMC call which is atomic and no need for lock. */
> static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
> {
> @@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
> return count;
> }
>
> +/* Size(8-byte words) of the log buffer. */
> +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> +
> +static int mlxbf_rsh_log_sem_lock(void)
> +{
> + unsigned long timeout;
> +
> + /* Take the semaphore. */
> + timeout = jiffies + msecs_to_jiffies(100);
> + while (readq(mlxbf_rsh_semaphore)) {
> + if (time_after(jiffies, timeout))
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static void mlxbf_rsh_log_sem_unlock(void)
> +{
> + writeq(0, mlxbf_rsh_semaphore);
> +}
> +
> +static ssize_t rsh_log_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
> + u64 data;
> +
> + if (!size)
> + return -EINVAL;
> +
> + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> + return -EOPNOTSUPP;
> +
> + /* Ignore line break at the end. */
> + if (buf[size - 1] == 0xa)
Please replace 0x0a with '\n' .
> + size--;
> +
> + /* Check the message prefix. */
> + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> + len = strlen(mlxbf_rsh_log_level[idx]);
> + if (len + 1 < size &&
> + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> + buf += len + 1;
Why len + 1, this seems intended to assume some separator
between prefix and the actual message? But if that is
the case please also check that the expected separator
character is there.
Otherwise this looks good to me.
Regards,
Hans
> + size -= len + 1;
> + level = idx;
> + break;
> + }
> + }
> +
> + /* Ignore leading spaces. */
> + while (size > 0 && buf[0] == ' ') {
> + size--;
> + buf++;
> + }
> +
> + /* Take the semaphore. */
> + rc = mlxbf_rsh_log_sem_lock();
> + if (rc)
> + return rc;
> +
> + /* Calculate how many words are available. */
> + num = (size + sizeof(u64) - 1) / sizeof(u64);
> + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> + 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 = (MLXBF_RSH_LOG_TYPE_MSG << MLXBF_RSH_LOG_TYPE_SHIFT) |
> + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> +
> + /* Write message. */
> + for (idx = 0, len = size; idx < num && len > 0; idx++) {
> + if (len <= sizeof(u64)) {
> + data = 0;
> + memcpy(&data, buf, len);
> + len = 0;
> + } else {
> + memcpy(&data, buf, sizeof(u64));
> + len -= sizeof(u64);
> + buf += sizeof(u64);
> + }
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> + }
> +
> +done:
> + /* Release the semaphore. */
> + mlxbf_rsh_log_sem_unlock();
> +
> + /* Ignore the rest if no more space. */
> + return count;
> +}
> +
> static DEVICE_ATTR_RW(post_reset_wdog);
> static DEVICE_ATTR_RW(reset_action);
> static DEVICE_ATTR_RW(second_reset_action);
> static DEVICE_ATTR_RO(lifecycle_state);
> static DEVICE_ATTR_RO(secure_boot_fuse_state);
> static DEVICE_ATTR_WO(fw_reset);
> +static DEVICE_ATTR_WO(rsh_log);
>
> static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_post_reset_wdog.attr,
> @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_lifecycle_state.attr,
> &dev_attr_secure_boot_fuse_state.attr,
> &dev_attr_fw_reset.attr,
> + &dev_attr_rsh_log.attr,
> NULL
> };
>
> @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
> static int mlxbf_bootctl_probe(struct platform_device *pdev)
> {
> struct arm_smccc_res res = { 0 };
> + void __iomem *reg;
> guid_t guid;
> int ret;
>
> @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct platform_device *pdev)
> if (IS_ERR(mlxbf_rsh_boot_cnt))
> return PTR_ERR(mlxbf_rsh_boot_cnt);
>
> + /* Get the resource of the bootfifo counter register. */
> + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> + if (IS_ERR(mlxbf_rsh_semaphore))
> + return PTR_ERR(mlxbf_rsh_semaphore);
> +
> + /* Get the resource of the bootfifo counter register. */
> + reg = devm_platform_ioremap_resource(pdev, 3);
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> + mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> + mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> +
> /* Ensure we have the UUID we expect for this service. */
> arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
> guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
This commit adds sysfs interface to be used to write into the
boot log which is 1KB HW buffer on BlueField SoC. The same log
buffer is also used by firmware code like ATF/UEFI, and can be
displayed by userspace tools or from external host via USB/PCIe.
Signed-off-by: Liming Sun <[email protected]>
Reviewed-by: Vadim Pasternak <[email protected]>
---
v1->v2:
Fixes for comments from Hans:
- Add more details in Documentation about the log levels;
- Replace 0x0a with '\n';
- Solve comment 'Why len + 1, this seems intended to assume some
separator between prefix'. The change is to remove the '+ 1'
here to avoid confusion. Yes, it's trying to remove the space
separator. Since the next block 'Ignore leading spaces' already
has similar logic, no need for the '+ 1" here.
v1: Initial version.
---
.../testing/sysfs-platform-mellanox-bootctl | 9 ++
drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
2 files changed, 150 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
index 9b99a81babb1..4c5c02d8f870 100644
--- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
+++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
@@ -75,3 +75,12 @@ KernelVersion: 6.4
Contact: "Liming Sun <[email protected]>"
Description:
The file used to access the BlueField boot fifo.
+
+What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
+Date: May 2023
+KernelVersion: 6.4
+Contact: "Liming Sun <[email protected]>"
+Description:
+ The file used to write BlueField boot log with the format
+ "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
+ default if not specified.
diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
index 1bad1d278672..e88ce68acb89 100644
--- a/drivers/platform/mellanox/mlxbf-bootctl.c
+++ b/drivers/platform/mellanox/mlxbf-bootctl.c
@@ -45,10 +45,38 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
[3] = "RMA",
};
+/* Log header format. */
+#define MLXBF_RSH_LOG_TYPE_SHIFT 56
+#define MLXBF_RSH_LOG_LEN_SHIFT 48
+#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
+
+/* Log module ID and type (only MSG type in Linux driver for now). */
+#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
+
+/* Log ctl/data register offset. */
+#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
+#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
+
+/* Log message levels. */
+enum {
+ MLXBF_RSH_LOG_INFO,
+ MLXBF_RSH_LOG_WARN,
+ MLXBF_RSH_LOG_ERR
+};
+
/* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
static void __iomem *mlxbf_rsh_boot_data;
static void __iomem *mlxbf_rsh_boot_cnt;
+/* Mapped pointer for rsh log semaphore/ctrl/data register. */
+static void __iomem *mlxbf_rsh_semaphore;
+static void __iomem *mlxbf_rsh_scratch_buf_ctl;
+static void __iomem *mlxbf_rsh_scratch_buf_data;
+
+/* Rsh log levels. */
+static const char * const mlxbf_rsh_log_level[] = {
+ "INFO", "WARN", "ERR", "ASSERT"};
+
/* ARM SMC call which is atomic and no need for lock. */
static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
{
@@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
return count;
}
+/* Size(8-byte words) of the log buffer. */
+#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
+
+static int mlxbf_rsh_log_sem_lock(void)
+{
+ unsigned long timeout;
+
+ /* Take the semaphore. */
+ timeout = jiffies + msecs_to_jiffies(100);
+ while (readq(mlxbf_rsh_semaphore)) {
+ if (time_after(jiffies, timeout))
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
+static void mlxbf_rsh_log_sem_unlock(void)
+{
+ writeq(0, mlxbf_rsh_semaphore);
+}
+
+static ssize_t rsh_log_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
+ u64 data;
+
+ if (!size)
+ return -EINVAL;
+
+ if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
+ return -EOPNOTSUPP;
+
+ /* Ignore line break at the end. */
+ if (buf[size - 1] == '\n')
+ size--;
+
+ /* Check the message prefix. */
+ for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
+ len = strlen(mlxbf_rsh_log_level[idx]);
+ if (len + 1 < size &&
+ !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
+ buf += len;
+ size -= len;
+ level = idx;
+ break;
+ }
+ }
+
+ /* Ignore leading spaces. */
+ while (size > 0 && buf[0] == ' ') {
+ size--;
+ buf++;
+ }
+
+ /* Take the semaphore. */
+ rc = mlxbf_rsh_log_sem_lock();
+ if (rc)
+ return rc;
+
+ /* Calculate how many words are available. */
+ num = (size + sizeof(u64) - 1) / sizeof(u64);
+ idx = readq(mlxbf_rsh_scratch_buf_ctl);
+ 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 = (MLXBF_RSH_LOG_TYPE_MSG << MLXBF_RSH_LOG_TYPE_SHIFT) |
+ ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
+ ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+
+ /* Write message. */
+ for (idx = 0, len = size; idx < num && len > 0; idx++) {
+ if (len <= sizeof(u64)) {
+ data = 0;
+ memcpy(&data, buf, len);
+ len = 0;
+ } else {
+ memcpy(&data, buf, sizeof(u64));
+ len -= sizeof(u64);
+ buf += sizeof(u64);
+ }
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+ }
+
+done:
+ /* Release the semaphore. */
+ mlxbf_rsh_log_sem_unlock();
+
+ /* Ignore the rest if no more space. */
+ return count;
+}
+
static DEVICE_ATTR_RW(post_reset_wdog);
static DEVICE_ATTR_RW(reset_action);
static DEVICE_ATTR_RW(second_reset_action);
static DEVICE_ATTR_RO(lifecycle_state);
static DEVICE_ATTR_RO(secure_boot_fuse_state);
static DEVICE_ATTR_WO(fw_reset);
+static DEVICE_ATTR_WO(rsh_log);
static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_post_reset_wdog.attr,
@@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_lifecycle_state.attr,
&dev_attr_secure_boot_fuse_state.attr,
&dev_attr_fw_reset.attr,
+ &dev_attr_rsh_log.attr,
NULL
};
@@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
static int mlxbf_bootctl_probe(struct platform_device *pdev)
{
struct arm_smccc_res res = { 0 };
+ void __iomem *reg;
guid_t guid;
int ret;
@@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct platform_device *pdev)
if (IS_ERR(mlxbf_rsh_boot_cnt))
return PTR_ERR(mlxbf_rsh_boot_cnt);
+ /* Get the resource of the bootfifo counter register. */
+ mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(mlxbf_rsh_semaphore))
+ return PTR_ERR(mlxbf_rsh_semaphore);
+
+ /* Get the resource of the bootfifo counter register. */
+ reg = devm_platform_ioremap_resource(pdev, 3);
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+ mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
+ mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
+
/* Ensure we have the UUID we expect for this service. */
arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
--
2.30.1
> -----Original Message-----
> From: Hans de Goede <[email protected]>
> Sent: Tuesday, May 9, 2023 6:07 AM
> To: Liming Sun <[email protected]>; Vadim Pasternak
> <[email protected]>; David Thompson <[email protected]>; Mark
> Gross <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH v1 1/1] mlxbf-bootctl: Add sysfs file for BlueField boot log
>
> Hi,
>
> On 5/8/23 20:01, Liming Sun wrote:
> > This commit adds sysfs interface to be used to write into the
> > boot log which is 1KB HW buffer on BlueField SoC. The same log
> > buffer is also used by firmware code like ATF/UEFI, and can be
> > displayed by userspace tools or from external host via USB/PCIe.
> >
> > Signed-off-by: Liming Sun <[email protected]>
> > Reviewed-by: Vadim Pasternak <[email protected]>
>
> Vadim, David, I would appreciate if one of you could also
> take a look at this patch.
v2 has been posted with internal review with Vadim.
>
> > ---
> > .../testing/sysfs-platform-mellanox-bootctl | 7 +
> > drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> > 2 files changed, 148 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > index 9b99a81babb1..239184db57dd 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > @@ -75,3 +75,10 @@ KernelVersion: 6.4
> > Contact: "Liming Sun <[email protected]>"
> > Description:
> > The file used to access the BlueField boot fifo.
> > +
> > +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> > +Date: May 2023
> > +KernelVersion: 6.4
> > +Contact: "Liming Sun <[email protected]>"
> > +Description:
> > + The file used to access the BlueField boot log.
>
> Access suggests reading, but this is a write-only file please make this more
> clear.
Updated in v2 to clarify it's for write.
>
> Also please document the accepted prefixes for log levels here / document
> the log format.
Updated in v2 with more details about the log format.
>
> > diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c
> b/drivers/platform/mellanox/mlxbf-bootctl.c
> > index 1bad1d278672..c02062cb3128 100644
> > --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> > +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> > @@ -45,10 +45,38 @@ static const char * const
> mlxbf_bootctl_lifecycle_states[] = {
> > [3] = "RMA",
> > };
> >
> > +/* Log header format. */
> > +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> > +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> > +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
> > +
> > +/* Log module ID and type (only MSG type in Linux driver for now). */
> > +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> > +
> > +/* Log ctl/data register offset. */
> > +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> > +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> > +
> > +/* Log message levels. */
> > +enum {
> > + MLXBF_RSH_LOG_INFO,
> > + MLXBF_RSH_LOG_WARN,
> > + MLXBF_RSH_LOG_ERR
> > +};
> > +
> > /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT
> register. */
> > static void __iomem *mlxbf_rsh_boot_data;
> > static void __iomem *mlxbf_rsh_boot_cnt;
> >
> > +/* Mapped pointer for rsh log semaphore/ctrl/data register. */
> > +static void __iomem *mlxbf_rsh_semaphore;
> > +static void __iomem *mlxbf_rsh_scratch_buf_ctl;
> > +static void __iomem *mlxbf_rsh_scratch_buf_data;
> > +
> > +/* Rsh log levels. */
> > +static const char * const mlxbf_rsh_log_level[] = {
> > + "INFO", "WARN", "ERR", "ASSERT"};
> > +
> > /* ARM SMC call which is atomic and no need for lock. */
> > static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
> > {
> > @@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
> > return count;
> > }
> >
> > +/* Size(8-byte words) of the log buffer. */
> > +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> > +
> > +static int mlxbf_rsh_log_sem_lock(void)
> > +{
> > + unsigned long timeout;
> > +
> > + /* Take the semaphore. */
> > + timeout = jiffies + msecs_to_jiffies(100);
> > + while (readq(mlxbf_rsh_semaphore)) {
> > + if (time_after(jiffies, timeout))
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void mlxbf_rsh_log_sem_unlock(void)
> > +{
> > + writeq(0, mlxbf_rsh_semaphore);
> > +}
> > +
> > +static ssize_t rsh_log_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
> > + u64 data;
> > +
> > + if (!size)
> > + return -EINVAL;
> > +
> > + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> > + return -EOPNOTSUPP;
> > +
> > + /* Ignore line break at the end. */
> > + if (buf[size - 1] == 0xa)
>
> Please replace 0x0a with '\n' .
Updated in v2
>
> > + size--;
> > +
> > + /* Check the message prefix. */
> > + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> > + len = strlen(mlxbf_rsh_log_level[idx]);
> > + if (len + 1 < size &&
> > + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> > + buf += len + 1;
>
> Why len + 1, this seems intended to assume some separator
> between prefix and the actual message? But if that is
> the case please also check that the expected separator
> character is there.
Good catch. Updated in v2 to remove the '+ 1" which causes confusing.
The separator logic is already covered in next code block.
>
> Otherwise this looks good to me.
>
> Regards,
>
> Hans
>
>
>
> > + size -= len + 1;
> > + level = idx;
> > + break;
> > + }
> > + }
> > +
> > + /* Ignore leading spaces. */
> > + while (size > 0 && buf[0] == ' ') {
> > + size--;
> > + buf++;
> > + }
> > +
> > + /* Take the semaphore. */
> > + rc = mlxbf_rsh_log_sem_lock();
> > + if (rc)
> > + return rc;
> > +
> > + /* Calculate how many words are available. */
> > + num = (size + sizeof(u64) - 1) / sizeof(u64);
> > + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> > + 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 = (MLXBF_RSH_LOG_TYPE_MSG <<
> MLXBF_RSH_LOG_TYPE_SHIFT) |
> > + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> > + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > +
> > + /* Write message. */
> > + for (idx = 0, len = size; idx < num && len > 0; idx++) {
> > + if (len <= sizeof(u64)) {
> > + data = 0;
> > + memcpy(&data, buf, len);
> > + len = 0;
> > + } else {
> > + memcpy(&data, buf, sizeof(u64));
> > + len -= sizeof(u64);
> > + buf += sizeof(u64);
> > + }
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > + }
> > +
> > +done:
> > + /* Release the semaphore. */
> > + mlxbf_rsh_log_sem_unlock();
> > +
> > + /* Ignore the rest if no more space. */
> > + return count;
> > +}
> > +
> > static DEVICE_ATTR_RW(post_reset_wdog);
> > static DEVICE_ATTR_RW(reset_action);
> > static DEVICE_ATTR_RW(second_reset_action);
> > static DEVICE_ATTR_RO(lifecycle_state);
> > static DEVICE_ATTR_RO(secure_boot_fuse_state);
> > static DEVICE_ATTR_WO(fw_reset);
> > +static DEVICE_ATTR_WO(rsh_log);
> >
> > static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_post_reset_wdog.attr,
> > @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_lifecycle_state.attr,
> > &dev_attr_secure_boot_fuse_state.attr,
> > &dev_attr_fw_reset.attr,
> > + &dev_attr_rsh_log.attr,
> > NULL
> > };
> >
> > @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t
> *guid,
> > static int mlxbf_bootctl_probe(struct platform_device *pdev)
> > {
> > struct arm_smccc_res res = { 0 };
> > + void __iomem *reg;
> > guid_t guid;
> > int ret;
> >
> > @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct
> platform_device *pdev)
> > if (IS_ERR(mlxbf_rsh_boot_cnt))
> > return PTR_ERR(mlxbf_rsh_boot_cnt);
> >
> > + /* Get the resource of the bootfifo counter register. */
> > + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> > + if (IS_ERR(mlxbf_rsh_semaphore))
> > + return PTR_ERR(mlxbf_rsh_semaphore);
> > +
> > + /* Get the resource of the bootfifo counter register. */
> > + reg = devm_platform_ioremap_resource(pdev, 3);
> > + if (IS_ERR(reg))
> > + return PTR_ERR(reg);
> > + mlxbf_rsh_scratch_buf_ctl = reg +
> MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> > + mlxbf_rsh_scratch_buf_data = reg +
> MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> > +
> > /* Ensure we have the UUID we expect for this service. */
> > arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0,
> &res);
> > guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
On Wed, 10 May 2023, Liming Sun wrote:
> This commit adds sysfs interface to be used to write into the
> boot log which is 1KB HW buffer on BlueField SoC. The same log
> buffer is also used by firmware code like ATF/UEFI, and can be
> displayed by userspace tools or from external host via USB/PCIe.
>
> Signed-off-by: Liming Sun <[email protected]>
> Reviewed-by: Vadim Pasternak <[email protected]>
> ---
> v1->v2:
> Fixes for comments from Hans:
> - Add more details in Documentation about the log levels;
> - Replace 0x0a with '\n';
> - Solve comment 'Why len + 1, this seems intended to assume some
> separator between prefix'. The change is to remove the '+ 1'
> here to avoid confusion. Yes, it's trying to remove the space
> separator. Since the next block 'Ignore leading spaces' already
> has similar logic, no need for the '+ 1" here.
> v1: Initial version.
> ---
> .../testing/sysfs-platform-mellanox-bootctl | 9 ++
> drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> 2 files changed, 150 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> index 9b99a81babb1..4c5c02d8f870 100644
> --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> @@ -75,3 +75,12 @@ KernelVersion: 6.4
> Contact: "Liming Sun <[email protected]>"
> Description:
> The file used to access the BlueField boot fifo.
> +
> +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> +Date: May 2023
> +KernelVersion: 6.4
> +Contact: "Liming Sun <[email protected]>"
> +Description:
> + The file used to write BlueField boot log with the format
> + "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
> + default if not specified.
> diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
> index 1bad1d278672..e88ce68acb89 100644
> --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> @@ -45,10 +45,38 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
> [3] = "RMA",
> };
>
> +/* Log header format. */
> +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
Use GENMASK_ULL() instead here and FIELD_PREP() below.
> +
> +/* Log module ID and type (only MSG type in Linux driver for now). */
> +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> +
> +/* Log ctl/data register offset. */
> +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> +
> +/* Log message levels. */
> +enum {
> + MLXBF_RSH_LOG_INFO,
> + MLXBF_RSH_LOG_WARN,
> + MLXBF_RSH_LOG_ERR
> +};
> +
> /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
> static void __iomem *mlxbf_rsh_boot_data;
> static void __iomem *mlxbf_rsh_boot_cnt;
>
> +/* Mapped pointer for rsh log semaphore/ctrl/data register. */
> +static void __iomem *mlxbf_rsh_semaphore;
> +static void __iomem *mlxbf_rsh_scratch_buf_ctl;
> +static void __iomem *mlxbf_rsh_scratch_buf_data;
> +
> +/* Rsh log levels. */
> +static const char * const mlxbf_rsh_log_level[] = {
> + "INFO", "WARN", "ERR", "ASSERT"};
> +
> /* ARM SMC call which is atomic and no need for lock. */
> static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
> {
> @@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
> return count;
> }
>
> +/* Size(8-byte words) of the log buffer. */
> +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> +
> +static int mlxbf_rsh_log_sem_lock(void)
> +{
> + unsigned long timeout;
> +
> + /* Take the semaphore. */
> + timeout = jiffies + msecs_to_jiffies(100);
> + while (readq(mlxbf_rsh_semaphore)) {
> + if (time_after(jiffies, timeout))
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
readq_poll_timeout()
> +}
> +
> +static void mlxbf_rsh_log_sem_unlock(void)
> +{
> + writeq(0, mlxbf_rsh_semaphore);
> +}
> +
> +static ssize_t rsh_log_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
Why casting to int, why not keep sizes as size_t??
> + u64 data;
> +
> + if (!size)
> + return -EINVAL;
> +
> + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> + return -EOPNOTSUPP;
> +
> + /* Ignore line break at the end. */
> + if (buf[size - 1] == '\n')
> + size--;
> +
> + /* Check the message prefix. */
> + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> + len = strlen(mlxbf_rsh_log_level[idx]);
> + if (len + 1 < size &&
> + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> + buf += len;
> + size -= len;
> + level = idx;
> + break;
> + }
> + }
> +
> + /* Ignore leading spaces. */
> + while (size > 0 && buf[0] == ' ') {
> + size--;
> + buf++;
> + }
> +
> + /* Take the semaphore. */
> + rc = mlxbf_rsh_log_sem_lock();
> + if (rc)
> + return rc;
> +
> + /* Calculate how many words are available. */
> + num = (size + sizeof(u64) - 1) / sizeof(u64);
DIV_ROUND_UP()
> + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> + if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MASK)
> + num = RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1;
min() ?
> + if (num <= 0)
> + goto done;
> +
> + /* Write Header. */
> + data = (MLXBF_RSH_LOG_TYPE_MSG << MLXBF_RSH_LOG_TYPE_SHIFT) |
> + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> +
> + /* Write message. */
> + for (idx = 0, len = size; idx < num && len > 0; idx++) {
size_t -> int conversion, why? Why you need another variable besides size
anyway?
How can len <= 0 occur when idx < num is true?
> + if (len <= sizeof(u64)) {
Why = ?
> + data = 0;
> + memcpy(&data, buf, len);
> + len = 0;
> + } else {
> + memcpy(&data, buf, sizeof(u64));
> + len -= sizeof(u64);
> + buf += sizeof(u64);
> + }
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> + }
> +
> +done:
> + /* Release the semaphore. */
> + mlxbf_rsh_log_sem_unlock();
> +
> + /* Ignore the rest if no more space. */
> + return count;
> +}
> +
> static DEVICE_ATTR_RW(post_reset_wdog);
> static DEVICE_ATTR_RW(reset_action);
> static DEVICE_ATTR_RW(second_reset_action);
> static DEVICE_ATTR_RO(lifecycle_state);
> static DEVICE_ATTR_RO(secure_boot_fuse_state);
> static DEVICE_ATTR_WO(fw_reset);
> +static DEVICE_ATTR_WO(rsh_log);
>
> static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_post_reset_wdog.attr,
> @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_lifecycle_state.attr,
> &dev_attr_secure_boot_fuse_state.attr,
> &dev_attr_fw_reset.attr,
> + &dev_attr_rsh_log.attr,
> NULL
> };
>
> @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
> static int mlxbf_bootctl_probe(struct platform_device *pdev)
> {
> struct arm_smccc_res res = { 0 };
> + void __iomem *reg;
> guid_t guid;
> int ret;
>
> @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct platform_device *pdev)
> if (IS_ERR(mlxbf_rsh_boot_cnt))
> return PTR_ERR(mlxbf_rsh_boot_cnt);
>
> + /* Get the resource of the bootfifo counter register. */
> + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> + if (IS_ERR(mlxbf_rsh_semaphore))
> + return PTR_ERR(mlxbf_rsh_semaphore);
> +
> + /* Get the resource of the bootfifo counter register. */
> + reg = devm_platform_ioremap_resource(pdev, 3);
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> + mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> + mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> +
> /* Ensure we have the UUID we expect for this service. */
> arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
> guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
>
--
i.
> -----Original Message-----
> From: Liming Sun <[email protected]>
> Sent: Wednesday, May 10, 2023 7:55 AM
> To: Vadim Pasternak <[email protected]>; David Thompson
> <[email protected]>; Hans de Goede <[email protected]>; Mark
> Gross <[email protected]>
> Cc: Liming Sun <[email protected]>; [email protected];
> [email protected]
> Subject: [PATCH v2] mlxbf-bootctl: Add sysfs file for BlueField boot log
>
> This commit adds sysfs interface to be used to write into the boot log which is 1KB
> HW buffer on BlueField SoC. The same log buffer is also used by firmware code
> like ATF/UEFI, and can be displayed by userspace tools or from external host via
> USB/PCIe.
>
> Signed-off-by: Liming Sun <[email protected]>
> Reviewed-by: Vadim Pasternak <[email protected]>
> ---
> v1->v2:
> Fixes for comments from Hans:
> - Add more details in Documentation about the log levels;
> - Replace 0x0a with '\n';
> - Solve comment 'Why len + 1, this seems intended to assume some
> separator between prefix'. The change is to remove the '+ 1'
> here to avoid confusion. Yes, it's trying to remove the space
> separator. Since the next block 'Ignore leading spaces' already
> has similar logic, no need for the '+ 1" here.
> v1: Initial version.
> ---
> .../testing/sysfs-platform-mellanox-bootctl | 9 ++
> drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> 2 files changed, 150 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> index 9b99a81babb1..4c5c02d8f870 100644
> --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> @@ -75,3 +75,12 @@ KernelVersion: 6.4
> Contact: "Liming Sun <[email protected]>"
> Description:
> The file used to access the BlueField boot fifo.
> +
> +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> +Date: May 2023
> +KernelVersion: 6.4
> +Contact: "Liming Sun <[email protected]>"
> +Description:
> + The file used to write BlueField boot log with the format
> + "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
> + default if not specified.
> diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c
> b/drivers/platform/mellanox/mlxbf-bootctl.c
> index 1bad1d278672..e88ce68acb89 100644
> --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> @@ -45,10 +45,38 @@ static const char * const mlxbf_bootctl_lifecycle_states[]
> = {
> [3] = "RMA",
> };
>
> +/* Log header format. */
> +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
> +
> +/* Log module ID and type (only MSG type in Linux driver for now). */
> +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> +
> +/* Log ctl/data register offset. */
> +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> +
> +/* Log message levels. */
> +enum {
> + MLXBF_RSH_LOG_INFO,
> + MLXBF_RSH_LOG_WARN,
> + MLXBF_RSH_LOG_ERR
> +};
> +
Should add "MLXBF_RSH_LOG_ASSERT" to enum for completeness
> /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT
> register. */ static void __iomem *mlxbf_rsh_boot_data; static void __iomem
> *mlxbf_rsh_boot_cnt;
>
> +/* Mapped pointer for rsh log semaphore/ctrl/data register. */ static
> +void __iomem *mlxbf_rsh_semaphore; static void __iomem
> +*mlxbf_rsh_scratch_buf_ctl; static void __iomem
> +*mlxbf_rsh_scratch_buf_data;
> +
> +/* Rsh log levels. */
> +static const char * const mlxbf_rsh_log_level[] = {
> + "INFO", "WARN", "ERR", "ASSERT"};
> +
> /* ARM SMC call which is atomic and no need for lock. */ static int
> mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg) { @@ -266,12 +294,111
> @@ static ssize_t fw_reset_store(struct device *dev,
> return count;
> }
>
> +/* Size(8-byte words) of the log buffer. */
> +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> +
> +static int mlxbf_rsh_log_sem_lock(void) {
> + unsigned long timeout;
> +
> + /* Take the semaphore. */
> + timeout = jiffies + msecs_to_jiffies(100);
> + while (readq(mlxbf_rsh_semaphore)) {
> + if (time_after(jiffies, timeout))
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +static void mlxbf_rsh_log_sem_unlock(void) {
> + writeq(0, mlxbf_rsh_semaphore);
> +}
> +
> +static ssize_t rsh_log_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
> + u64 data;
> +
> + if (!size)
> + return -EINVAL;
> +
> + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> + return -EOPNOTSUPP;
> +
> + /* Ignore line break at the end. */
> + if (buf[size - 1] == '\n')
> + size--;
> +
> + /* Check the message prefix. */
> + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> + len = strlen(mlxbf_rsh_log_level[idx]);
> + if (len + 1 < size &&
> + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> + buf += len;
> + size -= len;
> + level = idx;
> + break;
> + }
> + }
> +
> + /* Ignore leading spaces. */
> + while (size > 0 && buf[0] == ' ') {
> + size--;
> + buf++;
> + }
> +
> + /* Take the semaphore. */
> + rc = mlxbf_rsh_log_sem_lock();
> + if (rc)
> + return rc;
> +
> + /* Calculate how many words are available. */
> + num = (size + sizeof(u64) - 1) / sizeof(u64);
> + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> + 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 = (MLXBF_RSH_LOG_TYPE_MSG << MLXBF_RSH_LOG_TYPE_SHIFT)
> |
> + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
Should use FIELD_PREP() macro to generate value in "data"
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> +
> + /* Write message. */
> + for (idx = 0, len = size; idx < num && len > 0; idx++) {
> + if (len <= sizeof(u64)) {
> + data = 0;
> + memcpy(&data, buf, len);
> + len = 0;
> + } else {
> + memcpy(&data, buf, sizeof(u64));
> + len -= sizeof(u64);
> + buf += sizeof(u64);
> + }
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> + }
> +
> +done:
> + /* Release the semaphore. */
> + mlxbf_rsh_log_sem_unlock();
> +
> + /* Ignore the rest if no more space. */
> + return count;
> +}
> +
> static DEVICE_ATTR_RW(post_reset_wdog); static
> DEVICE_ATTR_RW(reset_action); static
> DEVICE_ATTR_RW(second_reset_action);
> static DEVICE_ATTR_RO(lifecycle_state); static
> DEVICE_ATTR_RO(secure_boot_fuse_state);
> static DEVICE_ATTR_WO(fw_reset);
> +static DEVICE_ATTR_WO(rsh_log);
>
> static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_post_reset_wdog.attr,
> @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_lifecycle_state.attr,
> &dev_attr_secure_boot_fuse_state.attr,
> &dev_attr_fw_reset.attr,
> + &dev_attr_rsh_log.attr,
> NULL
> };
>
> @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
> static int mlxbf_bootctl_probe(struct platform_device *pdev) {
> struct arm_smccc_res res = { 0 };
> + void __iomem *reg;
> guid_t guid;
> int ret;
>
> @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct platform_device
> *pdev)
> if (IS_ERR(mlxbf_rsh_boot_cnt))
> return PTR_ERR(mlxbf_rsh_boot_cnt);
>
> + /* Get the resource of the bootfifo counter register. */
Wrong comment, seems like cut-and-paste error
> + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> + if (IS_ERR(mlxbf_rsh_semaphore))
> + return PTR_ERR(mlxbf_rsh_semaphore);
> +
> + /* Get the resource of the bootfifo counter register. */
Wrong comment, seems like cut-and-paste error
> + reg = devm_platform_ioremap_resource(pdev, 3);
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> + mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> + mlxbf_rsh_scratch_buf_data = reg +
> MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> +
> /* Ensure we have the UUID we expect for this service. */
> arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0,
> &res);
> guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
> --
> 2.30.1
> -----Original Message-----
> From: David Thompson <[email protected]>
> Sent: Wednesday, May 10, 2023 7:03 PM
> To: Liming Sun <[email protected]>; Vadim Pasternak
> <[email protected]>; Hans de Goede <[email protected]>; Mark Gross
> <[email protected]>
> Cc: [email protected]; [email protected]
> Subject: RE: [PATCH v2] mlxbf-bootctl: Add sysfs file for BlueField boot log
>
>
>
> > -----Original Message-----
> > From: Liming Sun <[email protected]>
> > Sent: Wednesday, May 10, 2023 7:55 AM
> > To: Vadim Pasternak <[email protected]>; David Thompson
> > <[email protected]>; Hans de Goede <[email protected]>;
> Mark
> > Gross <[email protected]>
> > Cc: Liming Sun <[email protected]>; [email protected];
> > [email protected]
> > Subject: [PATCH v2] mlxbf-bootctl: Add sysfs file for BlueField boot log
> >
> > This commit adds sysfs interface to be used to write into the boot log which
> is 1KB
> > HW buffer on BlueField SoC. The same log buffer is also used by firmware
> code
> > like ATF/UEFI, and can be displayed by userspace tools or from external host
> via
> > USB/PCIe.
> >
> > Signed-off-by: Liming Sun <[email protected]>
> > Reviewed-by: Vadim Pasternak <[email protected]>
> > ---
> > v1->v2:
> > Fixes for comments from Hans:
> > - Add more details in Documentation about the log levels;
> > - Replace 0x0a with '\n';
> > - Solve comment 'Why len + 1, this seems intended to assume some
> > separator between prefix'. The change is to remove the '+ 1'
> > here to avoid confusion. Yes, it's trying to remove the space
> > separator. Since the next block 'Ignore leading spaces' already
> > has similar logic, no need for the '+ 1" here.
> > v1: Initial version.
> > ---
> > .../testing/sysfs-platform-mellanox-bootctl | 9 ++
> > drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> > 2 files changed, 150 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > index 9b99a81babb1..4c5c02d8f870 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > @@ -75,3 +75,12 @@ KernelVersion: 6.4
> > Contact: "Liming Sun <[email protected]>"
> > Description:
> > The file used to access the BlueField boot fifo.
> > +
> > +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> > +Date: May 2023
> > +KernelVersion: 6.4
> > +Contact: "Liming Sun <[email protected]>"
> > +Description:
> > + The file used to write BlueField boot log with the format
> > + "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
> > + default if not specified.
> > diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c
> > b/drivers/platform/mellanox/mlxbf-bootctl.c
> > index 1bad1d278672..e88ce68acb89 100644
> > --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> > +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> > @@ -45,10 +45,38 @@ static const char * const
> mlxbf_bootctl_lifecycle_states[]
> > = {
> > [3] = "RMA",
> > };
> >
> > +/* Log header format. */
> > +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> > +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> > +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
> > +
> > +/* Log module ID and type (only MSG type in Linux driver for now). */
> > +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> > +
> > +/* Log ctl/data register offset. */
> > +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> > +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> > +
> > +/* Log message levels. */
> > +enum {
> > + MLXBF_RSH_LOG_INFO,
> > + MLXBF_RSH_LOG_WARN,
> > + MLXBF_RSH_LOG_ERR
> > +};
> > +
>
> Should add "MLXBF_RSH_LOG_ASSERT" to enum for completeness
Updated in v3.
>
> > /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT
> > register. */ static void __iomem *mlxbf_rsh_boot_data; static void
> __iomem
> > *mlxbf_rsh_boot_cnt;
> >
> > +/* Mapped pointer for rsh log semaphore/ctrl/data register. */ static
> > +void __iomem *mlxbf_rsh_semaphore; static void __iomem
> > +*mlxbf_rsh_scratch_buf_ctl; static void __iomem
> > +*mlxbf_rsh_scratch_buf_data;
> > +
> > +/* Rsh log levels. */
> > +static const char * const mlxbf_rsh_log_level[] = {
> > + "INFO", "WARN", "ERR", "ASSERT"};
> > +
> > /* ARM SMC call which is atomic and no need for lock. */ static int
> > mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg) { @@ -266,12
> +294,111
> > @@ static ssize_t fw_reset_store(struct device *dev,
> > return count;
> > }
> >
> > +/* Size(8-byte words) of the log buffer. */
> > +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> > +
> > +static int mlxbf_rsh_log_sem_lock(void) {
> > + unsigned long timeout;
> > +
> > + /* Take the semaphore. */
> > + timeout = jiffies + msecs_to_jiffies(100);
> > + while (readq(mlxbf_rsh_semaphore)) {
> > + if (time_after(jiffies, timeout))
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void mlxbf_rsh_log_sem_unlock(void) {
> > + writeq(0, mlxbf_rsh_semaphore);
> > +}
> > +
> > +static ssize_t rsh_log_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
> > + u64 data;
> > +
> > + if (!size)
> > + return -EINVAL;
> > +
> > + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> > + return -EOPNOTSUPP;
> > +
> > + /* Ignore line break at the end. */
> > + if (buf[size - 1] == '\n')
> > + size--;
> > +
> > + /* Check the message prefix. */
> > + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> > + len = strlen(mlxbf_rsh_log_level[idx]);
> > + if (len + 1 < size &&
> > + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> > + buf += len;
> > + size -= len;
> > + level = idx;
> > + break;
> > + }
> > + }
> > +
> > + /* Ignore leading spaces. */
> > + while (size > 0 && buf[0] == ' ') {
> > + size--;
> > + buf++;
> > + }
> > +
> > + /* Take the semaphore. */
> > + rc = mlxbf_rsh_log_sem_lock();
> > + if (rc)
> > + return rc;
> > +
> > + /* Calculate how many words are available. */
> > + num = (size + sizeof(u64) - 1) / sizeof(u64);
> > + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> > + 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 = (MLXBF_RSH_LOG_TYPE_MSG <<
> MLXBF_RSH_LOG_TYPE_SHIFT)
> > |
> > + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> > + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
>
> Should use FIELD_PREP() macro to generate value in "data"
Updated in v3.
>
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > +
> > + /* Write message. */
> > + for (idx = 0, len = size; idx < num && len > 0; idx++) {
> > + if (len <= sizeof(u64)) {
> > + data = 0;
> > + memcpy(&data, buf, len);
> > + len = 0;
> > + } else {
> > + memcpy(&data, buf, sizeof(u64));
> > + len -= sizeof(u64);
> > + buf += sizeof(u64);
> > + }
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > + }
> > +
> > +done:
> > + /* Release the semaphore. */
> > + mlxbf_rsh_log_sem_unlock();
> > +
> > + /* Ignore the rest if no more space. */
> > + return count;
> > +}
> > +
> > static DEVICE_ATTR_RW(post_reset_wdog); static
> > DEVICE_ATTR_RW(reset_action); static
> > DEVICE_ATTR_RW(second_reset_action);
> > static DEVICE_ATTR_RO(lifecycle_state); static
> > DEVICE_ATTR_RO(secure_boot_fuse_state);
> > static DEVICE_ATTR_WO(fw_reset);
> > +static DEVICE_ATTR_WO(rsh_log);
> >
> > static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_post_reset_wdog.attr,
> > @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_lifecycle_state.attr,
> > &dev_attr_secure_boot_fuse_state.attr,
> > &dev_attr_fw_reset.attr,
> > + &dev_attr_rsh_log.attr,
> > NULL
> > };
> >
> > @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t
> *guid,
> > static int mlxbf_bootctl_probe(struct platform_device *pdev) {
> > struct arm_smccc_res res = { 0 };
> > + void __iomem *reg;
> > guid_t guid;
> > int ret;
> >
> > @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct
> platform_device
> > *pdev)
> > if (IS_ERR(mlxbf_rsh_boot_cnt))
> > return PTR_ERR(mlxbf_rsh_boot_cnt);
> >
> > + /* Get the resource of the bootfifo counter register. */
>
> Wrong comment, seems like cut-and-paste error
Updated in v3.
>
> > + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> > + if (IS_ERR(mlxbf_rsh_semaphore))
> > + return PTR_ERR(mlxbf_rsh_semaphore);
> > +
> > + /* Get the resource of the bootfifo counter register. */
>
> Wrong comment, seems like cut-and-paste error
Updated in v3.
>
> > + reg = devm_platform_ioremap_resource(pdev, 3);
> > + if (IS_ERR(reg))
> > + return PTR_ERR(reg);
> > + mlxbf_rsh_scratch_buf_ctl = reg +
> MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> > + mlxbf_rsh_scratch_buf_data = reg +
> > MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> > +
> > /* Ensure we have the UUID we expect for this service. */
> > arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0,
> > &res);
> > guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
> > --
> > 2.30.1
This commit adds sysfs interface to be used to write into the
boot log which is 1KB HW buffer on BlueField SoC. The same log
buffer is also used by firmware code like ATF/UEFI, and can be
displayed by userspace tools or from external host via USB/PCIe.
Signed-off-by: Liming Sun <[email protected]>
Reviewed-by: Vadim Pasternak <[email protected]>
---
v2->v3:
Fixes for comments from David:
- Add MLXBF_RSH_LOG_ASSERT;
- Use FIELD_PREP generate value in "data";
- Fix cut-and-paste comments;
Fixes for comments from Ilpo
- Use GENMASK_ULL();
- Use readq_poll_timeout() in mlxbf_rsh_log_sem_lock();
- No need to cast 'count' to int, keep 'size_t' instead;
- Use DIV_ROUND_UP;
- Use min() when calculating 'num' of words to write;
- Simplify the 'Write message' loop and removed 'len' from
the loop.
v1->v2:
Fixes for comments from Hans:
- Add more details in Documentation about the log levels;
- Replace 0x0a with '\n';
- Solve comment 'Why len + 1, this seems intended to assume some
separator between prefix'. The change is to remove the '+ 1'
here to avoid confusion. Yes, it's trying to remove the space
separator. Since the next block 'Ignore leading spaces' already
has similar logic, no need for the '+ 1" here.
v1: Initial version.
---
.../testing/sysfs-platform-mellanox-bootctl | 9 ++
drivers/platform/mellanox/mlxbf-bootctl.c | 144 +++++++++++++++++-
2 files changed, 151 insertions(+), 2 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
index 9b99a81babb1..4c5c02d8f870 100644
--- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
+++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
@@ -75,3 +75,12 @@ KernelVersion: 6.4
Contact: "Liming Sun <[email protected]>"
Description:
The file used to access the BlueField boot fifo.
+
+What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
+Date: May 2023
+KernelVersion: 6.4
+Contact: "Liming Sun <[email protected]>"
+Description:
+ The file used to write BlueField boot log with the format
+ "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
+ default if not specified.
diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
index 1bad1d278672..fb9f7815c6cd 100644
--- a/drivers/platform/mellanox/mlxbf-bootctl.c
+++ b/drivers/platform/mellanox/mlxbf-bootctl.c
@@ -11,6 +11,7 @@
#include <linux/acpi.h>
#include <linux/arm-smccc.h>
#include <linux/delay.h>
+#include <linux/iopoll.h>
#include <linux/module.h>
#include <linux/platform_device.h>
@@ -45,10 +46,39 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
[3] = "RMA",
};
+/* Log header format. */
+#define MLXBF_RSH_LOG_TYPE_MASK GENMASK_ULL(59, 56)
+#define MLXBF_RSH_LOG_LEN_MASK GENMASK_ULL(54, 48)
+#define MLXBF_RSH_LOG_LEVEL_MASK GENMASK_ULL(7, 0)
+
+/* Log module ID and type (only MSG type in Linux driver for now). */
+#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
+
+/* Log ctl/data register offset. */
+#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
+#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
+
+/* Log message levels. */
+enum {
+ MLXBF_RSH_LOG_INFO,
+ MLXBF_RSH_LOG_WARN,
+ MLXBF_RSH_LOG_ERR,
+ MLXBF_RSH_LOG_ASSERT
+};
+
/* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
static void __iomem *mlxbf_rsh_boot_data;
static void __iomem *mlxbf_rsh_boot_cnt;
+/* Mapped pointer for rsh log semaphore/ctrl/data register. */
+static void __iomem *mlxbf_rsh_semaphore;
+static void __iomem *mlxbf_rsh_scratch_buf_ctl;
+static void __iomem *mlxbf_rsh_scratch_buf_data;
+
+/* Rsh log levels. */
+static const char * const mlxbf_rsh_log_level[] = {
+ "INFO", "WARN", "ERR", "ASSERT"};
+
/* ARM SMC call which is atomic and no need for lock. */
static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
{
@@ -266,12 +296,108 @@ static ssize_t fw_reset_store(struct device *dev,
return count;
}
+/* Size(8-byte words) of the log buffer. */
+#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
+
+/* 100ms timeout */
+#define RSH_SCRATCH_BUF_POLL_TIMEOUT 100000
+
+static int mlxbf_rsh_log_sem_lock(void)
+{
+ unsigned long reg;
+
+ return readq_poll_timeout(mlxbf_rsh_semaphore, reg, !reg, 0,
+ RSH_SCRATCH_BUF_POLL_TIMEOUT);
+}
+
+static void mlxbf_rsh_log_sem_unlock(void)
+{
+ writeq(0, mlxbf_rsh_semaphore);
+}
+
+static ssize_t rsh_log_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int rc, idx, num, len, level = MLXBF_RSH_LOG_INFO;
+ size_t size = count;
+ u64 data;
+
+ if (!size)
+ return -EINVAL;
+
+ if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
+ return -EOPNOTSUPP;
+
+ /* Ignore line break at the end. */
+ if (buf[size - 1] == '\n')
+ size--;
+
+ /* Check the message prefix. */
+ for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
+ len = strlen(mlxbf_rsh_log_level[idx]);
+ if (len + 1 < size &&
+ !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
+ buf += len;
+ size -= len;
+ level = idx;
+ break;
+ }
+ }
+
+ /* Ignore leading spaces. */
+ while (size > 0 && buf[0] == ' ') {
+ size--;
+ buf++;
+ }
+
+ /* Take the semaphore. */
+ rc = mlxbf_rsh_log_sem_lock();
+ if (rc)
+ return rc;
+
+ /* Calculate how many words are available. */
+ idx = readq(mlxbf_rsh_scratch_buf_ctl);
+ num = min((int)DIV_ROUND_UP(size, sizeof(u64)),
+ RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1);
+ if (num <= 0)
+ goto done;
+
+ /* Write Header. */
+ data = FIELD_PREP(MLXBF_RSH_LOG_TYPE_MASK, MLXBF_RSH_LOG_TYPE_MSG);
+ data |= FIELD_PREP(MLXBF_RSH_LOG_LEN_MASK, num);
+ data |= FIELD_PREP(MLXBF_RSH_LOG_LEVEL_MASK, level);
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+
+ /* Write message. */
+ for (idx = 0; idx < num && size > 0; idx++) {
+ if (size < sizeof(u64)) {
+ data = 0;
+ memcpy(&data, buf, size);
+ size = 0;
+ } else {
+ memcpy(&data, buf, sizeof(u64));
+ size -= sizeof(u64);
+ buf += sizeof(u64);
+ }
+ writeq(data, mlxbf_rsh_scratch_buf_data);
+ }
+
+done:
+ /* Release the semaphore. */
+ mlxbf_rsh_log_sem_unlock();
+
+ /* Ignore the rest if no more space. */
+ return count;
+}
+
static DEVICE_ATTR_RW(post_reset_wdog);
static DEVICE_ATTR_RW(reset_action);
static DEVICE_ATTR_RW(second_reset_action);
static DEVICE_ATTR_RO(lifecycle_state);
static DEVICE_ATTR_RO(secure_boot_fuse_state);
static DEVICE_ATTR_WO(fw_reset);
+static DEVICE_ATTR_WO(rsh_log);
static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_post_reset_wdog.attr,
@@ -280,6 +406,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
&dev_attr_lifecycle_state.attr,
&dev_attr_secure_boot_fuse_state.attr,
&dev_attr_fw_reset.attr,
+ &dev_attr_rsh_log.attr,
NULL
};
@@ -345,19 +472,32 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
static int mlxbf_bootctl_probe(struct platform_device *pdev)
{
struct arm_smccc_res res = { 0 };
+ void __iomem *reg;
guid_t guid;
int ret;
- /* Get the resource of the bootfifo data register. */
+ /* Map the resource of the bootfifo data register. */
mlxbf_rsh_boot_data = devm_platform_ioremap_resource(pdev, 0);
if (IS_ERR(mlxbf_rsh_boot_data))
return PTR_ERR(mlxbf_rsh_boot_data);
- /* Get the resource of the bootfifo counter register. */
+ /* Map the resource of the bootfifo counter register. */
mlxbf_rsh_boot_cnt = devm_platform_ioremap_resource(pdev, 1);
if (IS_ERR(mlxbf_rsh_boot_cnt))
return PTR_ERR(mlxbf_rsh_boot_cnt);
+ /* Map the resource of the rshim semaphore register. */
+ mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(mlxbf_rsh_semaphore))
+ return PTR_ERR(mlxbf_rsh_semaphore);
+
+ /* Map the resource of the scratch buffer (log) registers. */
+ reg = devm_platform_ioremap_resource(pdev, 3);
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+ mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
+ mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
+
/* Ensure we have the UUID we expect for this service. */
arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
--
2.30.1
Thanks for the comments.
> -----Original Message-----
> From: Ilpo J?rvinen <[email protected]>
> Sent: Wednesday, May 10, 2023 10:05 AM
> To: Liming Sun <[email protected]>
> Cc: Vadim Pasternak <[email protected]>; David Thompson
> <[email protected]>; Hans de Goede <[email protected]>; Mark
> Gross <[email protected]>; [email protected]; linux-
> [email protected]
> Subject: Re: [PATCH v2] mlxbf-bootctl: Add sysfs file for BlueField boot log
>
> On Wed, 10 May 2023, Liming Sun wrote:
>
> > This commit adds sysfs interface to be used to write into the
> > boot log which is 1KB HW buffer on BlueField SoC. The same log
> > buffer is also used by firmware code like ATF/UEFI, and can be
> > displayed by userspace tools or from external host via USB/PCIe.
> >
> > Signed-off-by: Liming Sun <[email protected]>
> > Reviewed-by: Vadim Pasternak <[email protected]>
> > ---
> > v1->v2:
> > Fixes for comments from Hans:
> > - Add more details in Documentation about the log levels;
> > - Replace 0x0a with '\n';
> > - Solve comment 'Why len + 1, this seems intended to assume some
> > separator between prefix'. The change is to remove the '+ 1'
> > here to avoid confusion. Yes, it's trying to remove the space
> > separator. Since the next block 'Ignore leading spaces' already
> > has similar logic, no need for the '+ 1" here.
> > v1: Initial version.
> > ---
> > .../testing/sysfs-platform-mellanox-bootctl | 9 ++
> > drivers/platform/mellanox/mlxbf-bootctl.c | 141 ++++++++++++++++++
> > 2 files changed, 150 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > index 9b99a81babb1..4c5c02d8f870 100644
> > --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> > @@ -75,3 +75,12 @@ KernelVersion: 6.4
> > Contact: "Liming Sun <[email protected]>"
> > Description:
> > The file used to access the BlueField boot fifo.
> > +
> > +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> > +Date: May 2023
> > +KernelVersion: 6.4
> > +Contact: "Liming Sun <[email protected]>"
> > +Description:
> > + The file used to write BlueField boot log with the format
> > + "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
> > + default if not specified.
> > diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c
> b/drivers/platform/mellanox/mlxbf-bootctl.c
> > index 1bad1d278672..e88ce68acb89 100644
> > --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> > +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> > @@ -45,10 +45,38 @@ static const char * const
> mlxbf_bootctl_lifecycle_states[] = {
> > [3] = "RMA",
> > };
> >
> > +/* Log header format. */
> > +#define MLXBF_RSH_LOG_TYPE_SHIFT 56
> > +#define MLXBF_RSH_LOG_LEN_SHIFT 48
> > +#define MLXBF_RSH_LOG_LEVEL_SHIFT 0
>
> Use GENMASK_ULL() instead here and FIELD_PREP() below.
Updated in v3.
>
> > +
> > +/* Log module ID and type (only MSG type in Linux driver for now). */
> > +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> > +
> > +/* Log ctl/data register offset. */
> > +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> > +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> > +
> > +/* Log message levels. */
> > +enum {
> > + MLXBF_RSH_LOG_INFO,
> > + MLXBF_RSH_LOG_WARN,
> > + MLXBF_RSH_LOG_ERR
> > +};
> > +
> > /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT
> register. */
> > static void __iomem *mlxbf_rsh_boot_data;
> > static void __iomem *mlxbf_rsh_boot_cnt;
> >
> > +/* Mapped pointer for rsh log semaphore/ctrl/data register. */
> > +static void __iomem *mlxbf_rsh_semaphore;
> > +static void __iomem *mlxbf_rsh_scratch_buf_ctl;
> > +static void __iomem *mlxbf_rsh_scratch_buf_data;
> > +
> > +/* Rsh log levels. */
> > +static const char * const mlxbf_rsh_log_level[] = {
> > + "INFO", "WARN", "ERR", "ASSERT"};
> > +
> > /* ARM SMC call which is atomic and no need for lock. */
> > static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
> > {
> > @@ -266,12 +294,111 @@ static ssize_t fw_reset_store(struct device *dev,
> > return count;
> > }
> >
> > +/* Size(8-byte words) of the log buffer. */
> > +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> > +
> > +static int mlxbf_rsh_log_sem_lock(void)
> > +{
> > + unsigned long timeout;
> > +
> > + /* Take the semaphore. */
> > + timeout = jiffies + msecs_to_jiffies(100);
> > + while (readq(mlxbf_rsh_semaphore)) {
> > + if (time_after(jiffies, timeout))
> > + return -ETIMEDOUT;
> > + }
> > +
> > + return 0;
>
> readq_poll_timeout()
Updated in v3.
>
> > +}
> > +
> > +static void mlxbf_rsh_log_sem_unlock(void)
> > +{
> > + writeq(0, mlxbf_rsh_semaphore);
> > +}
> > +
> > +static ssize_t rsh_log_store(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + int rc, idx, num, len, size = (int)count, level = MLXBF_RSH_LOG_INFO;
>
> Why casting to int, why not keep sizes as size_t??
Updated in v3 to keep type 'size_t'.
>
> > + u64 data;
> > +
> > + if (!size)
> > + return -EINVAL;
> > +
> > + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> > + return -EOPNOTSUPP;
> > +
> > + /* Ignore line break at the end. */
> > + if (buf[size - 1] == '\n')
> > + size--;
> > +
> > + /* Check the message prefix. */
> > + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> > + len = strlen(mlxbf_rsh_log_level[idx]);
> > + if (len + 1 < size &&
> > + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> > + buf += len;
> > + size -= len;
> > + level = idx;
> > + break;
> > + }
> > + }
> > +
> > + /* Ignore leading spaces. */
> > + while (size > 0 && buf[0] == ' ') {
> > + size--;
> > + buf++;
> > + }
> > +
> > + /* Take the semaphore. */
> > + rc = mlxbf_rsh_log_sem_lock();
> > + if (rc)
> > + return rc;
> > +
> > + /* Calculate how many words are available. */
> > + num = (size + sizeof(u64) - 1) / sizeof(u64);
>
> DIV_ROUND_UP()
Updated in v3 to use DIV_ROUND_UP().
>
> > + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> > + if (idx + num + 1 >= RSH_SCRATCH_BUF_CTL_IDX_MASK)
> > + num = RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1;
>
> min() ?
Updated in v3 to use min().
>
> > + if (num <= 0)
> > + goto done;
> > +
> > + /* Write Header. */
> > + data = (MLXBF_RSH_LOG_TYPE_MSG <<
> MLXBF_RSH_LOG_TYPE_SHIFT) |
> > + ((u64)num << MLXBF_RSH_LOG_LEN_SHIFT) |
> > + ((u64)level << MLXBF_RSH_LOG_LEVEL_SHIFT);
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > +
> > + /* Write message. */
> > + for (idx = 0, len = size; idx < num && len > 0; idx++) {
>
> size_t -> int conversion, why? Why you need another variable besides size
> anyway?
>
> How can len <= 0 occur when idx < num is true?
Updated in v3. Simplified the code to remove the 'len' from this loop.
>
> > + if (len <= sizeof(u64)) {
>
> Why = ?
Updated in v3. '<' or '<=' will have the same effect in this block.
But changed it to '<' to avoid confusion.
>
> > + data = 0;
> > + memcpy(&data, buf, len);
> > + len = 0;
> > + } else {
> > + memcpy(&data, buf, sizeof(u64));
> > + len -= sizeof(u64);
> > + buf += sizeof(u64);
> > + }
> > + writeq(data, mlxbf_rsh_scratch_buf_data);
> > + }
> > +
> > +done:
> > + /* Release the semaphore. */
> > + mlxbf_rsh_log_sem_unlock();
> > +
> > + /* Ignore the rest if no more space. */
> > + return count;
> > +}
> > +
> > static DEVICE_ATTR_RW(post_reset_wdog);
> > static DEVICE_ATTR_RW(reset_action);
> > static DEVICE_ATTR_RW(second_reset_action);
> > static DEVICE_ATTR_RO(lifecycle_state);
> > static DEVICE_ATTR_RO(secure_boot_fuse_state);
> > static DEVICE_ATTR_WO(fw_reset);
> > +static DEVICE_ATTR_WO(rsh_log);
> >
> > static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_post_reset_wdog.attr,
> > @@ -280,6 +407,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> > &dev_attr_lifecycle_state.attr,
> > &dev_attr_secure_boot_fuse_state.attr,
> > &dev_attr_fw_reset.attr,
> > + &dev_attr_rsh_log.attr,
> > NULL
> > };
> >
> > @@ -345,6 +473,7 @@ static bool mlxbf_bootctl_guid_match(const guid_t
> *guid,
> > static int mlxbf_bootctl_probe(struct platform_device *pdev)
> > {
> > struct arm_smccc_res res = { 0 };
> > + void __iomem *reg;
> > guid_t guid;
> > int ret;
> >
> > @@ -358,6 +487,18 @@ static int mlxbf_bootctl_probe(struct
> platform_device *pdev)
> > if (IS_ERR(mlxbf_rsh_boot_cnt))
> > return PTR_ERR(mlxbf_rsh_boot_cnt);
> >
> > + /* Get the resource of the bootfifo counter register. */
> > + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> > + if (IS_ERR(mlxbf_rsh_semaphore))
> > + return PTR_ERR(mlxbf_rsh_semaphore);
> > +
> > + /* Get the resource of the bootfifo counter register. */
> > + reg = devm_platform_ioremap_resource(pdev, 3);
> > + if (IS_ERR(reg))
> > + return PTR_ERR(reg);
> > + mlxbf_rsh_scratch_buf_ctl = reg +
> MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> > + mlxbf_rsh_scratch_buf_data = reg +
> MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> > +
> > /* Ensure we have the UUID we expect for this service. */
> > arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0,
> &res);
> > guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);
> >
>
> --
> i.
Hi,
On 5/11/23 16:49, Liming Sun wrote:
> This commit adds sysfs interface to be used to write into the
> boot log which is 1KB HW buffer on BlueField SoC. The same log
> buffer is also used by firmware code like ATF/UEFI, and can be
> displayed by userspace tools or from external host via USB/PCIe.
>
> Signed-off-by: Liming Sun <[email protected]>
> Reviewed-by: Vadim Pasternak <[email protected]>
Thank you for your patch, I've applied this patch to my review-hans
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans
Note it will show up in my review-hans branch once I've pushed my
local branch there, which might take a while.
Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.
Regards,
Hans
> ---
> v2->v3:
> Fixes for comments from David:
> - Add MLXBF_RSH_LOG_ASSERT;
> - Use FIELD_PREP generate value in "data";
> - Fix cut-and-paste comments;
> Fixes for comments from Ilpo
> - Use GENMASK_ULL();
> - Use readq_poll_timeout() in mlxbf_rsh_log_sem_lock();
> - No need to cast 'count' to int, keep 'size_t' instead;
> - Use DIV_ROUND_UP;
> - Use min() when calculating 'num' of words to write;
> - Simplify the 'Write message' loop and removed 'len' from
> the loop.
> v1->v2:
> Fixes for comments from Hans:
> - Add more details in Documentation about the log levels;
> - Replace 0x0a with '\n';
> - Solve comment 'Why len + 1, this seems intended to assume some
> separator between prefix'. The change is to remove the '+ 1'
> here to avoid confusion. Yes, it's trying to remove the space
> separator. Since the next block 'Ignore leading spaces' already
> has similar logic, no need for the '+ 1" here.
> v1: Initial version.
> ---
> .../testing/sysfs-platform-mellanox-bootctl | 9 ++
> drivers/platform/mellanox/mlxbf-bootctl.c | 144 +++++++++++++++++-
> 2 files changed, 151 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> index 9b99a81babb1..4c5c02d8f870 100644
> --- a/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> +++ b/Documentation/ABI/testing/sysfs-platform-mellanox-bootctl
> @@ -75,3 +75,12 @@ KernelVersion: 6.4
> Contact: "Liming Sun <[email protected]>"
> Description:
> The file used to access the BlueField boot fifo.
> +
> +What: /sys/bus/platform/devices/MLNXBF04:00/rsh_log
> +Date: May 2023
> +KernelVersion: 6.4
> +Contact: "Liming Sun <[email protected]>"
> +Description:
> + The file used to write BlueField boot log with the format
> + "[INFO|WARN|ERR|ASSERT ]<msg>". Log level 'INFO' is used by
> + default if not specified.
> diff --git a/drivers/platform/mellanox/mlxbf-bootctl.c b/drivers/platform/mellanox/mlxbf-bootctl.c
> index 1bad1d278672..fb9f7815c6cd 100644
> --- a/drivers/platform/mellanox/mlxbf-bootctl.c
> +++ b/drivers/platform/mellanox/mlxbf-bootctl.c
> @@ -11,6 +11,7 @@
> #include <linux/acpi.h>
> #include <linux/arm-smccc.h>
> #include <linux/delay.h>
> +#include <linux/iopoll.h>
> #include <linux/module.h>
> #include <linux/platform_device.h>
>
> @@ -45,10 +46,39 @@ static const char * const mlxbf_bootctl_lifecycle_states[] = {
> [3] = "RMA",
> };
>
> +/* Log header format. */
> +#define MLXBF_RSH_LOG_TYPE_MASK GENMASK_ULL(59, 56)
> +#define MLXBF_RSH_LOG_LEN_MASK GENMASK_ULL(54, 48)
> +#define MLXBF_RSH_LOG_LEVEL_MASK GENMASK_ULL(7, 0)
> +
> +/* Log module ID and type (only MSG type in Linux driver for now). */
> +#define MLXBF_RSH_LOG_TYPE_MSG 0x04ULL
> +
> +/* Log ctl/data register offset. */
> +#define MLXBF_RSH_SCRATCH_BUF_CTL_OFF 0
> +#define MLXBF_RSH_SCRATCH_BUF_DATA_OFF 0x10
> +
> +/* Log message levels. */
> +enum {
> + MLXBF_RSH_LOG_INFO,
> + MLXBF_RSH_LOG_WARN,
> + MLXBF_RSH_LOG_ERR,
> + MLXBF_RSH_LOG_ASSERT
> +};
> +
> /* Mapped pointer for RSH_BOOT_FIFO_DATA and RSH_BOOT_FIFO_COUNT register. */
> static void __iomem *mlxbf_rsh_boot_data;
> static void __iomem *mlxbf_rsh_boot_cnt;
>
> +/* Mapped pointer for rsh log semaphore/ctrl/data register. */
> +static void __iomem *mlxbf_rsh_semaphore;
> +static void __iomem *mlxbf_rsh_scratch_buf_ctl;
> +static void __iomem *mlxbf_rsh_scratch_buf_data;
> +
> +/* Rsh log levels. */
> +static const char * const mlxbf_rsh_log_level[] = {
> + "INFO", "WARN", "ERR", "ASSERT"};
> +
> /* ARM SMC call which is atomic and no need for lock. */
> static int mlxbf_bootctl_smc(unsigned int smc_op, int smc_arg)
> {
> @@ -266,12 +296,108 @@ static ssize_t fw_reset_store(struct device *dev,
> return count;
> }
>
> +/* Size(8-byte words) of the log buffer. */
> +#define RSH_SCRATCH_BUF_CTL_IDX_MASK 0x7f
> +
> +/* 100ms timeout */
> +#define RSH_SCRATCH_BUF_POLL_TIMEOUT 100000
> +
> +static int mlxbf_rsh_log_sem_lock(void)
> +{
> + unsigned long reg;
> +
> + return readq_poll_timeout(mlxbf_rsh_semaphore, reg, !reg, 0,
> + RSH_SCRATCH_BUF_POLL_TIMEOUT);
> +}
> +
> +static void mlxbf_rsh_log_sem_unlock(void)
> +{
> + writeq(0, mlxbf_rsh_semaphore);
> +}
> +
> +static ssize_t rsh_log_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int rc, idx, num, len, level = MLXBF_RSH_LOG_INFO;
> + size_t size = count;
> + u64 data;
> +
> + if (!size)
> + return -EINVAL;
> +
> + if (!mlxbf_rsh_semaphore || !mlxbf_rsh_scratch_buf_ctl)
> + return -EOPNOTSUPP;
> +
> + /* Ignore line break at the end. */
> + if (buf[size - 1] == '\n')
> + size--;
> +
> + /* Check the message prefix. */
> + for (idx = 0; idx < ARRAY_SIZE(mlxbf_rsh_log_level); idx++) {
> + len = strlen(mlxbf_rsh_log_level[idx]);
> + if (len + 1 < size &&
> + !strncmp(buf, mlxbf_rsh_log_level[idx], len)) {
> + buf += len;
> + size -= len;
> + level = idx;
> + break;
> + }
> + }
> +
> + /* Ignore leading spaces. */
> + while (size > 0 && buf[0] == ' ') {
> + size--;
> + buf++;
> + }
> +
> + /* Take the semaphore. */
> + rc = mlxbf_rsh_log_sem_lock();
> + if (rc)
> + return rc;
> +
> + /* Calculate how many words are available. */
> + idx = readq(mlxbf_rsh_scratch_buf_ctl);
> + num = min((int)DIV_ROUND_UP(size, sizeof(u64)),
> + RSH_SCRATCH_BUF_CTL_IDX_MASK - idx - 1);
> + if (num <= 0)
> + goto done;
> +
> + /* Write Header. */
> + data = FIELD_PREP(MLXBF_RSH_LOG_TYPE_MASK, MLXBF_RSH_LOG_TYPE_MSG);
> + data |= FIELD_PREP(MLXBF_RSH_LOG_LEN_MASK, num);
> + data |= FIELD_PREP(MLXBF_RSH_LOG_LEVEL_MASK, level);
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> +
> + /* Write message. */
> + for (idx = 0; idx < num && size > 0; idx++) {
> + if (size < sizeof(u64)) {
> + data = 0;
> + memcpy(&data, buf, size);
> + size = 0;
> + } else {
> + memcpy(&data, buf, sizeof(u64));
> + size -= sizeof(u64);
> + buf += sizeof(u64);
> + }
> + writeq(data, mlxbf_rsh_scratch_buf_data);
> + }
> +
> +done:
> + /* Release the semaphore. */
> + mlxbf_rsh_log_sem_unlock();
> +
> + /* Ignore the rest if no more space. */
> + return count;
> +}
> +
> static DEVICE_ATTR_RW(post_reset_wdog);
> static DEVICE_ATTR_RW(reset_action);
> static DEVICE_ATTR_RW(second_reset_action);
> static DEVICE_ATTR_RO(lifecycle_state);
> static DEVICE_ATTR_RO(secure_boot_fuse_state);
> static DEVICE_ATTR_WO(fw_reset);
> +static DEVICE_ATTR_WO(rsh_log);
>
> static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_post_reset_wdog.attr,
> @@ -280,6 +406,7 @@ static struct attribute *mlxbf_bootctl_attrs[] = {
> &dev_attr_lifecycle_state.attr,
> &dev_attr_secure_boot_fuse_state.attr,
> &dev_attr_fw_reset.attr,
> + &dev_attr_rsh_log.attr,
> NULL
> };
>
> @@ -345,19 +472,32 @@ static bool mlxbf_bootctl_guid_match(const guid_t *guid,
> static int mlxbf_bootctl_probe(struct platform_device *pdev)
> {
> struct arm_smccc_res res = { 0 };
> + void __iomem *reg;
> guid_t guid;
> int ret;
>
> - /* Get the resource of the bootfifo data register. */
> + /* Map the resource of the bootfifo data register. */
> mlxbf_rsh_boot_data = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(mlxbf_rsh_boot_data))
> return PTR_ERR(mlxbf_rsh_boot_data);
>
> - /* Get the resource of the bootfifo counter register. */
> + /* Map the resource of the bootfifo counter register. */
> mlxbf_rsh_boot_cnt = devm_platform_ioremap_resource(pdev, 1);
> if (IS_ERR(mlxbf_rsh_boot_cnt))
> return PTR_ERR(mlxbf_rsh_boot_cnt);
>
> + /* Map the resource of the rshim semaphore register. */
> + mlxbf_rsh_semaphore = devm_platform_ioremap_resource(pdev, 2);
> + if (IS_ERR(mlxbf_rsh_semaphore))
> + return PTR_ERR(mlxbf_rsh_semaphore);
> +
> + /* Map the resource of the scratch buffer (log) registers. */
> + reg = devm_platform_ioremap_resource(pdev, 3);
> + if (IS_ERR(reg))
> + return PTR_ERR(reg);
> + mlxbf_rsh_scratch_buf_ctl = reg + MLXBF_RSH_SCRATCH_BUF_CTL_OFF;
> + mlxbf_rsh_scratch_buf_data = reg + MLXBF_RSH_SCRATCH_BUF_DATA_OFF;
> +
> /* Ensure we have the UUID we expect for this service. */
> arm_smccc_smc(MLXBF_BOOTCTL_SIP_SVC_UID, 0, 0, 0, 0, 0, 0, 0, &res);
> guid_parse(mlxbf_bootctl_svc_uuid_str, &guid);