2023-06-28 12:39:53

by Mukesh Ojha

[permalink] [raw]
Subject: [PATCH v4 03/21] soc: qcom: Add qcom_minidump_smem module

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 <[email protected]>
---
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.
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_ */
--
2.7.4



2023-06-28 13:47:27

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 03/21] soc: qcom: Add qcom_minidump_smem module

On Wed, Jun 28, 2023 at 3:35 PM Mukesh Ojha <[email protected]> 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_ */

--
With Best Regards,
Andy Shevchenko

2023-06-28 16:04:05

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH v4 03/21] soc: qcom: Add qcom_minidump_smem module

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 <[email protected]>
> ---
> 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