[PATCH v4 0/2] Retrieve information about DDR from SMEM

[PATCH v4 0/2] Retrieve information about DDR from SMEM

[Konrad Dybcio]

SMEM allows the OS to retrieve information about the DDR memory.
Among that information, is a semi-magic value called 'HBB', or Highest
Bank address Bit, which multimedia drivers (for hardware like Adreno
and MDSS) must retrieve in order to program the IP blocks correctly.

This series introduces an API to retrieve that value, uses it in the
aforementioned programming sequences and exposes available DDR
frequencies in debugfs (to e.g. pass to aoss_qmp debugfs). More
information can be exposed in the future, as needed.

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Changes in v4:
- Rebase on top of Dmitry's cleanups to ubwc_config
- Mark and consume the shared data as little-endian
- Fix compiler optimizing out what it thinks is impossible (odd
  v3_14freq case)
- Rework qcom_ubwc_config_get_data() per discussion to not alter
  hardcoded data
- Remove some unnecessary comments and assignments
- Return -ENODATA instead of -EINVAL for out-of-sensible-range HBB
- Cosmetic/line wrapping fixups
- Drop applied drm/msm patch
- Link to v3: https://lore.kernel.org/r/20260108-topic-smem_dramc-v3-0-6b64df58a017@oss.qualcomm.com

Changes in v3:
- Support v6 and v7 DDRInfo (v7 is used on e.g. Hamoa)
- Handle rare cases of DDRInfo v5 with additional trailing data
- Rebase/adjust to SSoT UBWC
- Expose hbb value in debugfs
- cosmetic changes
- Link to v2: https://lore.kernel.org/r/20250410-topic-smem_dramc-v2-0-dead15264714@oss.qualcomm.com

Changes in v2:
- Avoid checking for < 0 on unsigned types
- Overwrite Adreno UBWC data to keep the data shared with userspace
  coherent with what's programmed into the hardware
- Call get_hbb() in msm_mdss_enable() instead of all UBWC setup
  branches separately
- Pick up Bjorn's rb on patch 1
- Link to v1: https://lore.kernel.org/r/20250409-topic-smem_dramc-v1-0-94d505cd5593@oss.qualcomm.com

To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Kees Cook <kees@kernel.org>
To: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: linux-kernel@vger.kernel.org
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-hardening@vger.kernel.org

---
Konrad Dybcio (2):
      soc: qcom: smem: Expose DDR data from SMEM
      soc: qcom: ubwc: Get HBB from SMEM

 drivers/soc/qcom/Makefile      |   3 +-
 drivers/soc/qcom/smem.c        |  14 +-
 drivers/soc/qcom/smem.h        |   9 +
 drivers/soc/qcom/smem_dramc.c  | 430 +++++++++++++++++++++++++++++++++++++++++
 drivers/soc/qcom/ubwc_config.c |  40 +++-
 include/linux/soc/qcom/smem.h  |   2 +
 6 files changed, 495 insertions(+), 3 deletions(-)
---
base-commit: 3fe08b9796f36ef437ab9328e7dd1e5ff2d66603
change-id: 20250409-topic-smem_dramc-6467187ac865

Best regards,
--  
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

[PATCH v4 1/2] soc: qcom: smem: Expose DDR data from SMEM

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

Most modern Qualcomm platforms (>= SM8150) expose information about the
DDR memory present on the system via SMEM.

Details from this information is used in various scenarios, such as
multimedia drivers configuring the hardware based on the "Highest Bank
address Bit" (hbb), or the list of valid frequencies in validation
scenarios...

Add support for parsing v3-v7 version of the structs. Unforunately,
they are not versioned, so some elbow grease is necessary to determine
which one is present. See for reference:

ver 3: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/1d11897d2cfcc7b85f28ff74c445018dbbecac7a
ver 4: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/f6e9aa549260bbc0bdcb156c2b05f48dc5963203
ver 5: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/617d3297abe8b1b8dd3de3d1dd69c3961e6f343f
ver 5 with 6regions: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/d770e009f9bae58d56d926f7490bbfb45af8341f
ver 6: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/62659b557fdb1551b20fae8073d1d701dfa8a62e
ver 7: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/734d95599c5ebb1ca0d4e1639142e65c590532b7

Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/Makefile     |   3 +-
 drivers/soc/qcom/smem.c       |  14 +-
 drivers/soc/qcom/smem.h       |   9 +
 drivers/soc/qcom/smem_dramc.c | 430 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/soc/qcom/smem.h |   2 +
 5 files changed, 456 insertions(+), 2 deletions(-)

diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index b7f1d2a57367..798643be3590 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -23,7 +23,8 @@ obj-$(CONFIG_QCOM_RPMH)		+= qcom_rpmh.o
 qcom_rpmh-y			+= rpmh-rsc.o
 qcom_rpmh-y			+= rpmh.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= rpm-proc.o smd-rpm.o
-obj-$(CONFIG_QCOM_SMEM) +=	smem.o
+qcom_smem-y			+= smem.o smem_dramc.o
+obj-$(CONFIG_QCOM_SMEM) +=	qcom_smem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 CFLAGS_smp2p.o := -I$(src)
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index afb21a778fe7..4315512d3a1d 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -4,6 +4,7 @@
  * Copyright (c) 2012-2013, The Linux Foundation. All rights reserved.
  */
 
+#include <linux/debugfs.h>
 #include <linux/hwspinlock.h>
 #include <linux/io.h>
 #include <linux/module.h>
@@ -16,6 +17,8 @@
 #include <linux/soc/qcom/smem.h>
 #include <linux/soc/qcom/socinfo.h>
 
+#include "smem.h"
+
 /*
  * The Qualcomm shared memory system is a allocate only heap structure that
  * consists of one of more memory areas that can be accessed by the processors
@@ -281,6 +284,8 @@ struct qcom_smem {
 	struct smem_partition global_partition;
 	struct xarray partitions;
 
+	struct dentry *debugfs_dir;
+
 	unsigned num_regions;
 	struct smem_region regions[] __counted_by(num_regions);
 };
@@ -1239,17 +1244,24 @@ static int qcom_smem_probe(struct platform_device *pdev)
 
 	__smem = smem;
 
+	smem->debugfs_dir = smem_dram_parse(smem->dev);
+
 	smem->socinfo = platform_device_register_data(&pdev->dev, "qcom-socinfo",
 						      PLATFORM_DEVID_NONE, NULL,
 						      0);
-	if (IS_ERR(smem->socinfo))
+	if (IS_ERR(smem->socinfo)) {
+		debugfs_remove_recursive(smem->debugfs_dir);
+
 		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
+	}
 
 	return 0;
 }
 
 static void qcom_smem_remove(struct platform_device *pdev)
 {
+	debugfs_remove_recursive(__smem->debugfs_dir);
+
 	platform_device_unregister(__smem->socinfo);
 
 	xa_destroy(&__smem->partitions);
diff --git a/drivers/soc/qcom/smem.h b/drivers/soc/qcom/smem.h
new file mode 100644
index 000000000000..8bf3f606e1ae
--- /dev/null
+++ b/drivers/soc/qcom/smem.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __QCOM_SMEM_INTERNAL__
+#define __QCOM_SMEM_INTERNAL__
+
+#include <linux/device.h>
+
+struct dentry *smem_dram_parse(struct device *dev);
+
+#endif
diff --git a/drivers/soc/qcom/smem_dramc.c b/drivers/soc/qcom/smem_dramc.c
new file mode 100644
index 000000000000..2998573a6e07
--- /dev/null
+++ b/drivers/soc/qcom/smem_dramc.c
@@ -0,0 +1,430 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/soc/qcom/smem.h>
+#include <linux/units.h>
+
+#include "smem.h"
+
+#define SMEM_DDR_INFO_ID		603
+
+#define MAX_DDR_FREQ_NUM_V3		13
+#define MAX_DDR_FREQ_NUM_V5		14
+
+#define MAX_CHAN_NUM			8
+#define MAX_RANK_NUM			2
+
+#define DDR_HBB_MIN			13
+#define DDR_HBB_MAX			19
+
+#define MAX_SHUB_ENTRIES		8
+
+static struct smem_dram *__dram;
+
+enum ddr_info_version {
+	INFO_UNKNOWN,
+	INFO_V3,
+	INFO_V3_WITH_14_FREQS,
+	INFO_V4,
+	INFO_V5,
+	INFO_V5_WITH_6_REGIONS,
+	INFO_V6, /* INFO_V6 seems to only have shipped with 6 DDR regions, unlike V7 */
+	INFO_V7,
+	INFO_V7_WITH_6_REGIONS,
+};
+
+struct smem_dram {
+	unsigned long frequencies[MAX_DDR_FREQ_NUM_V5];
+	u32 num_frequencies;
+	u8 hbb;
+};
+
+enum ddr_type {
+	DDR_TYPE_NODDR = 0,
+	DDR_TYPE_LPDDR1 = 1,
+	DDR_TYPE_LPDDR2 = 2,
+	DDR_TYPE_PCDDR2 = 3,
+	DDR_TYPE_PCDDR3 = 4,
+	DDR_TYPE_LPDDR3 = 5,
+	DDR_TYPE_LPDDR4 = 6,
+	DDR_TYPE_LPDDR4X = 7,
+	DDR_TYPE_LPDDR5 = 8,
+	DDR_TYPE_LPDDR5X = 9,
+};
+
+/* The data structures below are NOT __packed on purpose! */
+
+/* Structs used across multiple versions */
+struct ddr_part_details {
+	__le16 revision_id1;
+	__le16 revision_id2;
+	__le16 width;
+	__le16 density;
+};
+
+struct ddr_freq_table {
+	__le32 freq_khz;
+	u8 enabled;
+};
+
+/* V3 */
+struct ddr_freq_plan_v3 {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+};
+
+struct ddr_details_v3 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3 ddr_freq_tbl;
+	u8 num_channels;
+};
+
+/* Some V3 structs have an additional frequency level */
+struct ddr_freq_plan_v3_14freqs {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3 + 1];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+};
+
+struct ddr_details_v3_14freqs {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3_14freqs ddr_freq_tbl;
+	u8 num_channels;
+};
+
+/* V4 */
+struct ddr_details_v4 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v3 ddr_freq_tbl;
+	u8 num_channels;
+	u8 num_ranks[MAX_CHAN_NUM];
+	u8 highest_bank_addr_bit[MAX_CHAN_NUM][MAX_RANK_NUM];
+};
+
+/* V5 */
+struct shub_freq_table {
+	u8 enable;
+	__le32 freq_khz;
+};
+
+struct shub_freq_plan_entry {
+	u8 num_shub_freqs;
+	struct shub_freq_table shub_freq[MAX_SHUB_ENTRIES];
+};
+
+struct ddr_xbl2quantum_smem_data {
+	phys_addr_t ssr_cookie_addr;
+	__le32 reserved[10];
+};
+
+struct ddr_freq_plan_v5 {
+	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V5];
+	u8 num_ddr_freqs;
+	phys_addr_t clk_period_address;
+	__le32 max_nom_ddr_freq;
+};
+
+struct ddr_region_v5 {
+	__le64 start_address;
+	__le64 size;
+	__le64 mem_controller_address;
+	__le32 granule_size; /* MiB */
+	u8  ddr_rank;
+#define DDR_RANK_0	BIT(0)
+#define DDR_RANK_1	BIT(1)
+	u8  segments_start_index;
+	__le64 segments_start_offset;
+};
+
+struct ddr_regions_v5 {
+	__le32 ddr_region_num; /* We expect this to always be 4 or 6 */
+	__le64 ddr_rank0_size;
+	__le64 ddr_rank1_size;
+	__le64 ddr_cs0_start_addr;
+	__le64 ddr_cs1_start_addr;
+	__le32 highest_bank_addr_bit;
+	struct ddr_region_v5 ddr_region[] __counted_by_le(ddr_region_num);
+};
+
+struct ddr_details_v5 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v5 ddr_freq_tbl;
+	u8 num_channels;
+	u8 _padding;
+	struct ddr_regions_v5 ddr_regions;
+};
+
+/* V6 */
+struct ddr_misc_info_v6 {
+	__le32 dsf_version;
+	__le32 reserved[10];
+};
+
+/* V7 */
+struct ddr_details_v7 {
+	u8 manufacturer_id;
+	u8 device_type;
+	struct ddr_part_details ddr_params[MAX_CHAN_NUM];
+	struct ddr_freq_plan_v5 ddr_freq_tbl;
+	u8 num_channels;
+	u8 sct_config;
+	struct ddr_regions_v5 ddr_regions;
+};
+
+/**
+ * qcom_smem_dram_get_hbb(): Get the Highest bank address bit
+ *
+ * Context: Check qcom_smem_is_available() before calling this function.
+ * Because __dram * is initialized by smem_dram_parse(), which is in turn
+ * called from * qcom_smem_probe(), __dram will only be NULL if the data
+ * couldn't have been found/interpreted correctly.
+ *
+ * Return: highest bank bit on success, -ENODATA on failure.
+ */
+int qcom_smem_dram_get_hbb(void)
+{
+	if (!__dram || !__dram->hbb)
+		return -ENODATA;
+
+	if (__dram->hbb < DDR_HBB_MIN || __dram->hbb > DDR_HBB_MAX)
+		return -ENODATA;
+
+	return __dram->hbb;
+}
+EXPORT_SYMBOL_GPL(qcom_smem_dram_get_hbb);
+
+static void smem_dram_parse_v3_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v3 *details = data;
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v3_14freqs_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v3_14freqs *details = data;
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3 + 1; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled)
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_entry->freq_khz;
+	}
+}
+
+static void smem_dram_parse_v4_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v4 *details = data;
+
+	/* Rank 0 channel 0 entry holds the correct value */
+	dram->hbb = details->highest_bank_addr_bit[0][0];
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v5_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v5 *details = data;
+	struct ddr_regions_v5 *region = &details->ddr_regions;
+
+	dram->hbb = le32_to_cpu(region[0].highest_bank_addr_bit);
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V5; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+static void smem_dram_parse_v7_data(struct smem_dram *dram, void *data)
+{
+	struct ddr_details_v7 *details = data;
+	struct ddr_regions_v5 *region = &details->ddr_regions;
+
+	dram->hbb = le32_to_cpu(region[0].highest_bank_addr_bit);
+
+	for (int i = 0; i < MAX_DDR_FREQ_NUM_V5; i++) {
+		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
+
+		if (freq_entry->freq_khz && freq_entry->enabled) {
+			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
+			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;
+		}
+	}
+}
+
+/* The structure contains no version field, so we have to perform some guesswork.. */
+static int smem_dram_infer_struct_version(size_t size)
+{
+	/* Some early versions provided less bytes of less useful data */
+	if (size < sizeof(struct ddr_details_v3))
+		return -EINVAL;
+
+	if (size == sizeof(struct ddr_details_v3))
+		return INFO_V3;
+
+	if (size == sizeof(struct ddr_details_v3_14freqs))
+		return INFO_V3_WITH_14_FREQS;
+
+	if (size == sizeof(struct ddr_details_v4))
+		return INFO_V4;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    4 * sizeof(struct ddr_region_v5))
+		return INFO_V5;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    4 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_xbl2quantum_smem_data) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V5;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5))
+		return INFO_V5_WITH_6_REGIONS;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_xbl2quantum_smem_data) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V5_WITH_6_REGIONS;
+
+	if (size == sizeof(struct ddr_details_v5) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V6;
+
+	if (size == sizeof(struct ddr_details_v7) +
+		    4 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V7;
+
+	if (size == sizeof(struct ddr_details_v7) +
+		    6 * sizeof(struct ddr_region_v5) +
+		    sizeof(struct ddr_misc_info_v6) +
+		    sizeof(struct shub_freq_plan_entry))
+		return INFO_V7_WITH_6_REGIONS;
+
+	return INFO_UNKNOWN;
+}
+
+static int smem_dram_frequencies_show(struct seq_file *s, void *unused)
+{
+	struct smem_dram *dram = s->private;
+
+	for (int i = 0; i < dram->num_frequencies; i++)
+		seq_printf(s, "%lu\n", dram->frequencies[i]);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(smem_dram_frequencies);
+
+static int smem_hbb_show(struct seq_file *s, void *unused)
+{
+	struct smem_dram *dram = s->private;
+
+	if (!dram->hbb)
+		return -EINVAL;
+
+	seq_printf(s, "%d\n", dram->hbb);
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(smem_hbb);
+
+struct dentry *smem_dram_parse(struct device *dev)
+{
+	struct dentry *debugfs_dir;
+	enum ddr_info_version ver;
+	struct smem_dram *dram;
+	size_t actual_size;
+	void *data;
+
+	/* No need to check qcom_smem_is_available(), this func is called by the SMEM driver */
+	data = qcom_smem_get(QCOM_SMEM_HOST_ANY, SMEM_DDR_INFO_ID, &actual_size);
+	if (IS_ERR_OR_NULL(data))
+		return ERR_PTR(-ENODATA);
+
+	ver = smem_dram_infer_struct_version(actual_size);
+	if (ver < 0) {
+		/* Some SoCs don't provide data that's useful for us */
+		return ERR_PTR(-ENODATA);
+	} else if (ver == INFO_UNKNOWN) {
+		/* In other cases, we may not have added support for a newer struct revision */
+		dev_err(dev, "Found an unknown type of DRAM info struct (size = %zu)\n",
+			actual_size);
+		return ERR_PTR(-EINVAL);
+	}
+
+	dram = devm_kzalloc(dev, sizeof(*dram), GFP_KERNEL);
+	if (!dram)
+		return ERR_PTR(-ENOMEM);
+
+	switch (ver) {
+	case INFO_V3:
+		smem_dram_parse_v3_data(dram, data);
+		break;
+	case INFO_V3_WITH_14_FREQS:
+		smem_dram_parse_v3_14freqs_data(dram, data);
+		break;
+	case INFO_V4:
+		smem_dram_parse_v4_data(dram, data);
+		break;
+	case INFO_V5:
+	case INFO_V5_WITH_6_REGIONS:
+	case INFO_V6:
+		smem_dram_parse_v5_data(dram, data);
+		break;
+	case INFO_V7:
+	case INFO_V7_WITH_6_REGIONS:
+		smem_dram_parse_v7_data(dram, data);
+		break;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+
+	debugfs_dir = debugfs_create_dir("qcom_smem", NULL);
+	debugfs_create_file("dram_frequencies", 0444, debugfs_dir, dram,
+			    &smem_dram_frequencies_fops);
+	debugfs_create_file("hbb", 0444, debugfs_dir, dram, &smem_hbb_fops);
+
+	__dram = dram;
+
+	return debugfs_dir;
+}
diff --git a/include/linux/soc/qcom/smem.h b/include/linux/soc/qcom/smem.h
index f946e3beca21..3249cbd2016b 100644
--- a/include/linux/soc/qcom/smem.h
+++ b/include/linux/soc/qcom/smem.h
@@ -17,4 +17,6 @@ int qcom_smem_get_feature_code(u32 *code);
 
 int qcom_smem_bust_hwspin_lock_by_host(unsigned int host);
 
+int qcom_smem_dram_get_hbb(void);
+
 #endif

-- 
2.55.0

[PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM

[Konrad Dybcio]

From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

To make sure the correct settings for a given DRAM configuration get
applied, attempt to retrieve that data from SMEM (which happens to be
what the BSP kernel does, albeit with through convoluted means of the
bootloader altering the DT with this data).

Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
 drivers/soc/qcom/ubwc_config.c | 40 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 39 insertions(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
index 0209a02c390f..23901a4c51f7 100644
--- a/drivers/soc/qcom/ubwc_config.c
+++ b/drivers/soc/qcom/ubwc_config.c
@@ -11,6 +11,7 @@
 #include <linux/platform_device.h>
 
 #include <linux/soc/qcom/ubwc.h>
+#include <linux/soc/qcom/smem.h>
 
 static const struct qcom_ubwc_cfg_data no_ubwc_data = {
 	/* no UBWC, no HBB */
@@ -168,9 +169,25 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
 	{ }
 };
 
+static struct qcom_ubwc_cfg_data *cfg;
+static DEFINE_MUTEX(cfg_mutex);
+/**
+ * qcom_ubwc_config_get_data() - Retrieve UBWC data for the platform.
+ *
+ * Return: Pointer to valid struct qcom_ubwc_cfg_data on success, negative
+ * errno on failure. Note that this may return EPROBE_DEFER.
+ */
 const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
 {
 	const struct qcom_ubwc_cfg_data *data;
+	int hbb;
+
+	guard(mutex)(&cfg_mutex);
+	if (cfg)
+		return cfg;
+
+	if (!qcom_smem_is_available())
+		return ERR_PTR(-EPROBE_DEFER);
 
 	data = of_machine_get_match_data(qcom_ubwc_configs);
 	if (!data) {
@@ -178,9 +195,30 @@ const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
 		return ERR_PTR(-EINVAL);
 	}
 
-	return data;
+	hbb = qcom_smem_dram_get_hbb();
+	if (hbb == -ENODATA) {
+		/* Lack of HBB data is OK - it was only introduced later */
+		return data;
+	} else if (hbb < 0) {
+		pr_err("Couldn't get HBB data from SMEM: %d\n", hbb);
+		return ERR_PTR(hbb);
+	}
+
+	cfg = kmemdup(data, sizeof(*data), GFP_KERNEL);
+	if (!cfg)
+		return ERR_PTR(-ENOMEM);
+
+	cfg->highest_bank_bit = hbb;
+
+	return cfg;
 }
 EXPORT_SYMBOL_GPL(qcom_ubwc_config_get_data);
 
+static void __exit ubwc_config_exit(void)
+{
+	kfree(cfg);
+}
+module_exit(ubwc_config_exit);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("UBWC config database for QTI SoCs");

-- 
2.55.0

Re: [PATCH v4 1/2] soc: qcom: smem: Expose DDR data from SMEM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:

New issues:
- [High] The global pointer `__dram` is assigned memory managed by `devm_kzalloc`, but is never cleared to `NULL` upon driver removal, leading to a Use-After-Free when accessed by other drivers.
- [High] Firmware/hardware SMEM structures incorrectly use the configurable `phys_addr_t` type, breaking binary compatibility with the fixed firmware layout.
- [High] `debugfs_remove_recursive()` is called twice on the same dentry if `socinfo` registration fails, leading to a Double-Free/Use-After-Free.
- [Medium] The `__le32` field `freq_khz` is used directly in an arithmetic operation without conversion to native CPU endianness.
- [Medium] Multiplying `freq_khz` by `1000` evaluates as a 32-bit unsigned integer, which will overflow for memory frequencies above ~4.29 GHz.
- [Medium] The global `__smem` pointer is published before `smem_dram_parse()` completes, allowing concurrent callers to `qcom_smem_dram_get_hbb()` to erroneously observe a NULL `__dram`.
- [Medium] The global `__dram` pointer is written and read locklessly without necessary memory barriers, risking exposure of uninitialized struct fields to concurrent readers.

Pre-existing issues:
- [High] `platform_device_unregister()` is unconditionally called with an `ERR_PTR` if `socinfo` registration fails, leading to a crash.
--

--- Patch [1]: [PATCH v4 1/2] soc: qcom: smem: Expose DDR data from SMEM ---
commit 21c7410afeccade097a3fa15bf0554d407b561bb
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

soc: qcom: smem: Expose DDR data from SMEM

Most modern Qualcomm platforms (>= SM8150) expose information about the
DDR memory present on the system via SMEM.

Details from this information is used in various scenarios, such as
multimedia drivers configuring the hardware based on the "Highest Bank
address Bit" (hbb), or the list of valid frequencies in validation
scenarios...

Add support for parsing v3-v7 version of the structs. Unforunately,
they are not versioned, so some elbow grease is necessary to determine
which one is present. See for reference:

ver 3: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/1d11897d2cfcc7b85f28ff74c445018dbbecac7a
ver 4: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/f6e9aa549260bbc0bdcb156c2b05f48dc5963203
ver 5: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/617d3297abe8b1b8dd3de3d1dd69c3961e6f343f
ver 5 with 6regions: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/d770e009f9bae58d56d926f7490bbfb45af8341f
ver 6: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/62659b557fdb1551b20fae8073d1d701dfa8a62e
ver 7: https://git.codelinaro.org/clo/la/abl/tianocore/edk2/-/commit/734d95599c5ebb1ca0d4e1639142e65c590532b7

Reviewed-by: Bjorn Andersson <andersson@kernel.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

> diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
> index afb21a778fe7b..4315512d3a1d4 100644
> --- a/drivers/soc/qcom/smem.c
> +++ b/drivers/soc/qcom/smem.c

[ ... ]

> @@ -1239,17 +1244,24 @@ static int qcom_smem_probe(struct platform_device *pdev)
>  
>  	__smem = smem;
>  
> +	smem->debugfs_dir = smem_dram_parse(smem->dev);

[Severity: Medium]
Is it safe to publish the global __smem pointer before smem_dram_parse
completes?

If a concurrent driver probe calls qcom_smem_is_available, it will return
true, and the driver might then call qcom_smem_dram_get_hbb. Since the
__dram pointer hasn't been initialized yet, wouldn't the caller falsely
receive -ENODATA?

> +
>  	smem->socinfo = platform_device_register_data(&pdev->dev, "qcom-socinfo",
>  						      PLATFORM_DEVID_NONE, NULL,
>  						      0);
> -	if (IS_ERR(smem->socinfo))
> +	if (IS_ERR(smem->socinfo)) {
> +		debugfs_remove_recursive(smem->debugfs_dir);
> +

[Severity: High]
This is a pre-existing issue, but if platform_device_register_data fails, it
returns an ERR_PTR which is stored in smem->socinfo.

Since probe still returns 0 (success) in this case, when the driver is later
removed, qcom_smem_remove will pass this ERR_PTR directly to
platform_device_unregister. Could this cause a crash when the driver core
attempts to dereference it?

[Severity: High]
Additionally, when socinfo registration fails, we call
debugfs_remove_recursive on smem->debugfs_dir, but we leave the pointer
intact.

When the driver is later unbound, qcom_smem_remove calls
debugfs_remove_recursive again on __smem->debugfs_dir. Does this trigger a
double-free or use-after-free in the debugfs core?

>  		dev_dbg(&pdev->dev, "failed to register socinfo device\n");
> +	}
>  
>  	return 0;
>  }
>  
>  static void qcom_smem_remove(struct platform_device *pdev)
>  {
> +	debugfs_remove_recursive(__smem->debugfs_dir);
> +

[Severity: High]
When the driver is unbound, the devm-managed dram memory allocated in
smem_dram_parse will be freed.

However, the global __dram pointer is never cleared to NULL. If an external
driver later calls the exported qcom_smem_dram_get_hbb, will it bypass the
NULL check and dereference the freed memory?

>  	platform_device_unregister(__smem->socinfo);
>  
>  	xa_destroy(&__smem->partitions);

[ ... ]

> diff --git a/drivers/soc/qcom/smem_dramc.c b/drivers/soc/qcom/smem_dramc.c
> new file mode 100644
> index 0000000000000..2998573a6e074
> --- /dev/null
> +++ b/drivers/soc/qcom/smem_dramc.c

[ ... ]

> +/* V3 */
> +struct ddr_freq_plan_v3 {
> +	struct ddr_freq_table ddr_freq[MAX_DDR_FREQ_NUM_V3];
> +	u8 num_ddr_freqs;
> +	phys_addr_t clk_period_address;
> +};

[Severity: High]
Does using phys_addr_t in these firmware structures break binary compatibility?

Since the size of phys_addr_t depends on kernel configuration (e.g., 32-bit vs
64-bit), this could alter the structure layout and offsets. Should these use
explicit fixed-width types like u32 or u64 to match the fixed firmware ABI?
(This also appears to affect ddr_xbl2quantum_smem_data and ddr_freq_plan_v5).

[ ... ]

> +int qcom_smem_dram_get_hbb(void)
> +{
> +	if (!__dram || !__dram->hbb)
> +		return -ENODATA;

[Severity: Medium]
Are memory barriers required here?

Since smem_dram_parse writes to the global __dram pointer without
smp_store_release, and this function reads it without smp_load_acquire, could
CPU or compiler reordering cause concurrent readers to observe a non-NULL
pointer but read uninitialized values from its fields?

[ ... ]

> +static void smem_dram_parse_v3_data(struct smem_dram *dram, void *data)
> +{
> +	struct ddr_details_v3 *details = data;
> +
> +	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3; i++) {
> +		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
> +
> +		if (freq_entry->freq_khz && freq_entry->enabled) {
> +			u32 freq_khz = le32_to_cpu(freq_entry->freq_khz);
> +			dram->frequencies[dram->num_frequencies++] = 1000 * freq_khz;

[Severity: Medium]
Can this multiplication overflow on modern high-speed memory?

The expression 1000 * freq_khz uses a 32-bit unsigned integer multiplication.
If the frequency exceeds 4,294,967 kHz, the value will wrap around before
being implicitly promoted to unsigned long for the array assignment. Would it
help to cast freq_khz to unsigned long first? (This logic appears in
smem_dram_parse_v5_data as well).

> +		}
> +	}
> +}
> +
> +static void smem_dram_parse_v3_14freqs_data(struct smem_dram *dram, void *data)
> +{
> +	struct ddr_details_v3_14freqs *details = data;
> +
> +	for (int i = 0; i < MAX_DDR_FREQ_NUM_V3 + 1; i++) {
> +		struct ddr_freq_table *freq_entry = &details->ddr_freq_tbl.ddr_freq[i];
> +
> +		if (freq_entry->freq_khz && freq_entry->enabled)
> +			dram->frequencies[dram->num_frequencies++] = 1000 * freq_entry->freq_khz;

[Severity: Medium]
Is the little-endian to CPU conversion missing here?

In the other parsing functions, le32_to_cpu is used to read freq_khz. Here it
is read directly, which might result in incorrect frequency calculations on
big-endian systems.

> +	}
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-topic-smem_dramc-v4-0-748f992278a3@oss.qualcomm.com?part=1

Re: [PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A race condition between SMEM probing and `qcom_ubwc_config_get_data()` can cause the driver to bypass probe deferral, incorrectly fall back to default UBWC data, and return mismatched configurations to concurrent callers.
- [Low] Missing Kconfig dependency on QCOM_SMEM causes linker errors when QCOM_UBWC_CONFIG is selected but QCOM_SMEM is disabled or built as a module.
--

--- Patch [2]: [PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM ---
commit e0bf0c57e180329e2b45423901bc2dc1b3492f77
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

soc: qcom: ubwc: Get HBB from SMEM

This commit updates the UBWC configuration driver to dynamically retrieve
the Highest Bank Bit (HBB) from SMEM. If SMEM is not yet available, it
returns -EPROBE_DEFER. Once available, it allocates and updates a copy of
the UBWC configuration data with the retrieved HBB value.

> diff --git a/drivers/soc/qcom/ubwc_config.c b/drivers/soc/qcom/ubwc_config.c
> index 0209a02c390f0..23901a4c51f73 100644
> --- a/drivers/soc/qcom/ubwc_config.c
> +++ b/drivers/soc/qcom/ubwc_config.c
[ ... ]
> @@ -168,9 +169,25 @@ static const struct of_device_id qcom_ubwc_configs[] __maybe_unused = {
>  	{ }
>  };
>  
> +static struct qcom_ubwc_cfg_data *cfg;
> +static DEFINE_MUTEX(cfg_mutex);
> +/**
> + * qcom_ubwc_config_get_data() - Retrieve UBWC data for the platform.
> + *
> + * Return: Pointer to valid struct qcom_ubwc_cfg_data on success, negative
> + * errno on failure. Note that this may return EPROBE_DEFER.
> + */
>  const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
>  {
>  	const struct qcom_ubwc_cfg_data *data;
> +	int hbb;
> +
> +	guard(mutex)(&cfg_mutex);
> +	if (cfg)
> +		return cfg;
> +
> +	if (!qcom_smem_is_available())
> +		return ERR_PTR(-EPROBE_DEFER);

[Severity: Low]
Will this cause linker errors if the kernel is configured with
CONFIG_QCOM_UBWC_CONFIG=y but CONFIG_QCOM_SMEM=m or CONFIG_QCOM_SMEM=n?

Since these SMEM functions are called unconditionally without stub
implementations provided for when SMEM is disabled, it seems dependent drivers
selecting QCOM_UBWC_CONFIG might trigger undefined reference errors during
the build.

>  	data = of_machine_get_match_data(qcom_ubwc_configs);
>  	if (!data) {
> @@ -178,9 +195,30 @@ const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
>  		return ERR_PTR(-EINVAL);
>  	}
>  
> -	return data;
> +	hbb = qcom_smem_dram_get_hbb();

[Severity: High]
Can a race condition occur between SMEM probing and this function, causing
it to bypass probe deferral and use incorrect default data?

In drivers/soc/qcom/smem.c:qcom_smem_probe(), the global __smem pointer is
published before the DRAM data is populated:

drivers/soc/qcom/smem.c:qcom_smem_probe() {
    ...
    __smem = smem;
    ...
    smem->debugfs_dir = smem_dram_parse(smem->dev);
    ...
}

If a driver calls qcom_ubwc_config_get_data() concurrently during this
window, qcom_smem_is_available() would return true, but
qcom_smem_dram_get_hbb() would return -ENODATA because __dram is not
populated yet.

> +	if (hbb == -ENODATA) {
> +		/* Lack of HBB data is OK - it was only introduced later */
> +		return data;

Would this result in interpreting a transient initialization state as a
permanent lack of HBB data, returning mismatched configurations that could
cause issues like graphical corruption or SMMU faults across IP blocks that
share UBWC memory?

> +	} else if (hbb < 0) {
> +		pr_err("Couldn't get HBB data from SMEM: %d\n", hbb);
> +		return ERR_PTR(hbb);
> +	}
> +
> +	cfg = kmemdup(data, sizeof(*data), GFP_KERNEL);
> +	if (!cfg)
> +		return ERR_PTR(-ENOMEM);
> +
> +	cfg->highest_bank_bit = hbb;
> +
> +	return cfg;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-topic-smem_dramc-v4-0-748f992278a3@oss.qualcomm.com?part=2

Re: [PATCH v4 2/2] soc: qcom: ubwc: Get HBB from SMEM

[Dmitry Baryshkov]

On Tue, Jul 21, 2026 at 12:40:05PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] A race condition between SMEM probing and `qcom_ubwc_config_get_data()` can cause the driver to bypass probe deferral, incorrectly fall back to default UBWC data, and return mismatched configurations to concurrent callers.
> - [Low] Missing Kconfig dependency on QCOM_SMEM causes linker errors when QCOM_UBWC_CONFIG is selected but QCOM_SMEM is disabled or built as a module.
> --
> 
> > @@ -178,9 +195,30 @@ const struct qcom_ubwc_cfg_data *qcom_ubwc_config_get_data(void)
> >  		return ERR_PTR(-EINVAL);
> >  	}
> >  
> > -	return data;
> > +	hbb = qcom_smem_dram_get_hbb();
> 
> [Severity: High]
> Can a race condition occur between SMEM probing and this function, causing
> it to bypass probe deferral and use incorrect default data?
> 
> In drivers/soc/qcom/smem.c:qcom_smem_probe(), the global __smem pointer is
> published before the DRAM data is populated:
> 
> drivers/soc/qcom/smem.c:qcom_smem_probe() {
>     ...
>     __smem = smem;
>     ...
>     smem->debugfs_dir = smem_dram_parse(smem->dev);
>     ...
> }
> 
> If a driver calls qcom_ubwc_config_get_data() concurrently during this
> window, qcom_smem_is_available() would return true, but
> qcom_smem_dram_get_hbb() would return -ENODATA because __dram is not
> populated yet.

I think this must be addressed, the DRAM data should be parsed before
setting __semem pointer.


-- 
With best wishes
Dmitry