diff mbox series

[v4,03/21] soc: qcom: Add qcom_minidump_smem module

Message ID 1687955688-20809-4-git-send-email-quic_mojha@quicinc.com (mailing list archive)
State Handled Elsewhere
Headers show
Series [v4,01/21] docs: qcom: Add qualcomm minidump guide | expand

Commit Message

Mukesh Ojha June 28, 2023, 12:34 p.m. UTC
Add qcom_minidump_smem module in a preparation to remove smem
based minidump specific code from driver/remoteproc/qcom_common.c
and provide needed exported API, this abstract minidump specific
data layout from qualcomm's remoteproc driver.

Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
---
 drivers/soc/qcom/Kconfig              |   8 ++
 drivers/soc/qcom/qcom_minidump_smem.c | 147 ++++++++++++++++++++++++++++++++++
 include/soc/qcom/qcom_minidump.h      |  24 ++++++
 3 files changed, 179 insertions(+)
 create mode 100644 drivers/soc/qcom/qcom_minidump_smem.c
 create mode 100644 include/soc/qcom/qcom_minidump.h

Comments

Andy Shevchenko June 28, 2023, 1:31 p.m. UTC | #1
On Wed, Jun 28, 2023 at 3:35 PM Mukesh Ojha <quic_mojha@quicinc.com> wrote:
>
> Add qcom_minidump_smem module in a preparation to remove smem
> based minidump specific code from driver/remoteproc/qcom_common.c
> and provide needed exported API, this abstract minidump specific
> data layout from qualcomm's remoteproc driver.

...

> +#include <linux/kernel.h>

Why?

Missing headers:
err.h
export.h
string.h
types.h

byteorder/generic.h

> +#include <linux/module.h>
> +#include <linux/io.h>

Can you have them ordered?

...

> + * Return: On success, it returns iomapped base segment address, otherwise NULL on error.

IO mapped (or MMIO?)

...

> +       return ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr),

Why casting?

> +                       *seg_cnt * sizeof(struct minidump_region));

Something from overflow.h?

...

> +       /* If it is not valid region, skip it */

if region is not valid, skip it

...

> +#ifndef _QCOM_MINIDUMP_H_
> +#define _QCOM_MINIDUMP_H_

This header uses EINVAL, IS_ENABLED() and (to some extent) dma_addr_t.
So do you need respective headers to be included?

> +#endif /* _QCOM_MINIDUMP_H_ */
Greg KH June 28, 2023, 3:47 p.m. UTC | #2
On Wed, Jun 28, 2023 at 06:04:30PM +0530, Mukesh Ojha wrote:
> Add qcom_minidump_smem module in a preparation to remove smem
> based minidump specific code from driver/remoteproc/qcom_common.c
> and provide needed exported API, this abstract minidump specific
> data layout from qualcomm's remoteproc driver.
> 
> Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com>
> ---
>  drivers/soc/qcom/Kconfig              |   8 ++
>  drivers/soc/qcom/qcom_minidump_smem.c | 147 ++++++++++++++++++++++++++++++++++
>  include/soc/qcom/qcom_minidump.h      |  24 ++++++
>  3 files changed, 179 insertions(+)
>  create mode 100644 drivers/soc/qcom/qcom_minidump_smem.c
>  create mode 100644 include/soc/qcom/qcom_minidump.h
> 
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index a491718f8064..982310b5a1cb 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -279,4 +279,12 @@ config QCOM_INLINE_CRYPTO_ENGINE
>  	tristate
>  	select QCOM_SCM
>  
> +config QCOM_MINIDUMP_SMEM
> +	tristate "QCOM Minidump SMEM (as backend) Support"
> +	depends on ARCH_QCOM
> +	depends on QCOM_SMEM
> +	help
> +	  Enablement of core minidump feature is controlled from boot firmware
> +	  side, and this config allow linux to query minidump segments associated
> +	  with the remote processor and check its validity.

I can not understand this help text, sorry.  Also, what is the module
name?

And why is this only with ARCH_QCOM?  Why are we doing ARCH_PLATFORM
symbols still?  why is that a thing for a generic cpu type?

And don't you want build coverage?  Why not allow that?

thanks,

greg k-h
diff mbox series

Patch

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index a491718f8064..982310b5a1cb 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -279,4 +279,12 @@  config QCOM_INLINE_CRYPTO_ENGINE
 	tristate
 	select QCOM_SCM
 
+config QCOM_MINIDUMP_SMEM
+	tristate "QCOM Minidump SMEM (as backend) Support"
+	depends on ARCH_QCOM
+	depends on QCOM_SMEM
+	help
+	  Enablement of core minidump feature is controlled from boot firmware
+	  side, and this config allow linux to query minidump segments associated
+	  with the remote processor and check its validity.
 endmenu
diff --git a/drivers/soc/qcom/qcom_minidump_smem.c b/drivers/soc/qcom/qcom_minidump_smem.c
new file mode 100644
index 000000000000..b588833ea26e
--- /dev/null
+++ b/drivers/soc/qcom/qcom_minidump_smem.c
@@ -0,0 +1,147 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <linux/soc/qcom/smem.h>
+#include <soc/qcom/qcom_minidump.h>
+
+#define MAX_NUM_OF_SS           10
+#define MAX_REGION_NAME_LENGTH  16
+#define SBL_MINIDUMP_SMEM_ID	602
+#define MINIDUMP_REGION_VALID	('V' << 24 | 'A' << 16 | 'L' << 8 | 'I' << 0)
+#define MINIDUMP_SS_ENCR_DONE	('D' << 24 | 'O' << 16 | 'N' << 8 | 'E' << 0)
+#define MINIDUMP_SS_ENABLED	('E' << 24 | 'N' << 16 | 'B' << 8 | 'L' << 0)
+
+/**
+ * struct minidump_region - Minidump region
+ * @name		: Name of the region to be dumped
+ * @seq_num:		: Use to differentiate regions with same name.
+ * @valid		: This entry to be dumped (if set to 1)
+ * @address		: Physical address of region to be dumped
+ * @size		: Size of the region
+ */
+struct minidump_region {
+	char	name[MAX_REGION_NAME_LENGTH];
+	__le32	seq_num;
+	__le32	valid;
+	__le64	address;
+	__le64	size;
+};
+
+/**
+ * struct minidump_subsystem - Subsystem's SMEM Table of content
+ * @status : Subsystem toc init status
+ * @enabled : if set to 1, this region would be copied during coredump
+ * @encryption_status: Encryption status for this subsystem
+ * @encryption_required : Decides to encrypt the subsystem regions or not
+ * @region_count : Number of regions added in this subsystem toc
+ * @regions_baseptr : regions base pointer of the subsystem
+ */
+struct minidump_subsystem {
+	__le32	status;
+	__le32	enabled;
+	__le32	encryption_status;
+	__le32	encryption_required;
+	__le32	region_count;
+	__le64	regions_baseptr;
+};
+
+/**
+ * struct minidump_global_toc - Global Table of Content
+ * @status : Global Minidump init status
+ * @md_revision : Minidump revision
+ * @enabled : Minidump enable status
+ * @subsystems : Array of subsystems toc
+ */
+struct minidump_global_toc {
+	__le32				status;
+	__le32				md_revision;
+	__le32				enabled;
+	struct minidump_subsystem	subsystems[MAX_NUM_OF_SS];
+};
+
+/**
+ * qcom_ss_md_mapped_base() - For getting subsystem iomapped base segment address
+ * for given minidump index.
+ *
+ * @minidump_id: Subsystem minidump index.
+ * @seg_cnt:	 Segment count for a given minidump index
+ *
+ * Return: On success, it returns iomapped base segment address, otherwise NULL on error.
+ */
+void *qcom_ss_md_mapped_base(unsigned int minidump_id, int *seg_cnt)
+{
+	struct minidump_global_toc *toc;
+	struct minidump_subsystem *subsystem;
+
+	/*
+	 * Get global minidump ToC and check if global table
+	 * pointer exists and init is set.
+	 */
+	toc = qcom_smem_get(QCOM_SMEM_HOST_ANY, SBL_MINIDUMP_SMEM_ID, NULL);
+	if (IS_ERR(toc) || !toc->status)
+		return NULL;
+
+	/* Get subsystem table of contents using the minidump id */
+	subsystem = &toc->subsystems[minidump_id];
+
+	/**
+	 * Collect minidump if SS ToC is valid and segment table
+	 * is initialized in memory and encryption status is set.
+	 */
+	if (subsystem->regions_baseptr == 0 ||
+	    le32_to_cpu(subsystem->status) != 1 ||
+	    le32_to_cpu(subsystem->enabled) != MINIDUMP_SS_ENABLED ||
+	    le32_to_cpu(subsystem->encryption_status) != MINIDUMP_SS_ENCR_DONE)
+		return NULL;
+
+	*seg_cnt = le32_to_cpu(subsystem->region_count);
+
+	return ioremap((unsigned long)le64_to_cpu(subsystem->regions_baseptr),
+			*seg_cnt * sizeof(struct minidump_region));
+}
+EXPORT_SYMBOL_GPL(qcom_ss_md_mapped_base);
+
+/**
+ * qcom_ss_valid_segment_info() - Checks segment sanity and gets it's details
+ *
+ * @ptr:  Segment IO-mapped base address.
+ * @i:	  Segment index.
+ * @name: Segment name.
+ * @da:	  Segment physical address.
+ * @size: Segment size.
+ *
+ * Return: It returns 0 on success and 1 if the segment is invalid and negative
+ *	   value on error.
+ */
+int qcom_ss_valid_segment_info(void *ptr, int i, char **name, dma_addr_t *da, size_t *size)
+{
+	struct minidump_region __iomem *tmp;
+	struct minidump_region region;
+
+	if (!ptr)
+		return -EINVAL;
+
+	tmp = ptr;
+	memcpy_fromio(&region, tmp + i, sizeof(region));
+
+	/* If it is not valid region, skip it */
+	if (le32_to_cpu(region.valid) != MINIDUMP_REGION_VALID)
+		return 1;
+
+	*name = kstrndup(region.name, MAX_REGION_NAME_LENGTH - 1, GFP_KERNEL);
+	if (!*name)
+		return -ENOMEM;
+
+	*da = le64_to_cpu(region.address);
+	*size = le64_to_cpu(region.size);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_ss_valid_segment_info);
+
+MODULE_DESCRIPTION("Qualcomm Minidump SMEM as backend driver");
+MODULE_LICENSE("GPL");
diff --git a/include/soc/qcom/qcom_minidump.h b/include/soc/qcom/qcom_minidump.h
new file mode 100644
index 000000000000..d7747c27fd45
--- /dev/null
+++ b/include/soc/qcom/qcom_minidump.h
@@ -0,0 +1,24 @@ 
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ */
+
+#ifndef _QCOM_MINIDUMP_H_
+#define _QCOM_MINIDUMP_H_
+
+#if IS_ENABLED(CONFIG_QCOM_MINIDUMP_SMEM)
+void *qcom_ss_md_mapped_base(unsigned int minidump_id, int *seg_cnt);
+int qcom_ss_valid_segment_info(void *ptr, int i, char **name,
+			       dma_addr_t *da, size_t *size);
+#else
+static inline void *qcom_ss_md_mapped_base(unsigned int minidump_id, int *seg_cnt)
+{
+	return NULL;
+}
+static inline int qcom_ss_valid_segment_info(void *ptr, int i, char **name,
+					     dma_addr_t *da, size_t *size)
+{
+	return -EINVAL;
+}
+#endif /* CONFIG_QCOM_MINIDUMP_SMEM */
+#endif /* _QCOM_MINIDUMP_H_ */