Qualcomm SMEM V12 Support

drivers/soc/qcom/smem.c +1 -0

+	default:
  		dev_err(&pdev->dev, "Unsupported SMEM version 0x%x\n", version);
  		return -EINVAL;
  	}

In V12 SMEM, SBL writes SMEM parameter information after the TOC. Use the SBL provided item count as the max item number.

Signed-off-by Chris Lew <[email protected]>

---
  drivers/soc/qcom/smem.c | 41 +++++++++++++++++++++++++++++++++++++++--
  1 file changed, 39 insertions(+), 2 deletions(-)

drivers/soc/qcom/smem.c +3 -0

index fed2934d6bda..a51f4ba42173 100644
@@ -225,6 +225,24 @@ struct smem_private_entry {
  #define SMEM_PRIVATE_CANARY	0xa5a5
 
  /**
+ * struct smem_info - smem region info located after the table of contents
+ * @magic:	magic number, must be SMEM_INFO_MAGIC
+ * @size:	size of the smem region

drivers/soc/qcom/smem.c +7 -0

+};
+
+static const u8 SMEM_INFO_MAGIC[] = { 0x53, 0x49, 0x49, 0x49 }; /* SIII */
+
+/**
   * struct smem_region - representation of a chunk of memory used for smem
   * @aux_base:	identifier of aux_mem base
   * @virt_base:	virtual base address of memory with this aux_mem identifier

@@ -245,6 +263,7 @@ struct smem_region {
   * @partitions:	list of pointers to partitions affecting the current
   *		processor/host
   * @cacheline:	list of cacheline sizes for each host
+ * @item_count: max accepted item number
   * @num_regions: number of @regions
   * @regions:	list of the memory regions defining the shared memory
   */

@@ -257,6 +276,7 @@ struct qcom_smem {
  	size_t global_cacheline;
  	struct smem_partition_header *partitions[SMEM_HOST_COUNT];
  	size_t cacheline[SMEM_HOST_COUNT];
+	u32 item_count;
 
  	unsigned num_regions;
  	struct smem_region regions[0];

drivers/soc/qcom/smem.c +18 -1

  		return -EINVAL;
 
  	header = smem->regions[0].virt_base;

@@ -473,7 +493,7 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
  	u32 aux_base;
  	unsigned i;
 
-	if (WARN_ON(item >= SMEM_ITEM_COUNT))
+	if (WARN_ON(item >= smem->item_count))
  		return ERR_PTR(-EINVAL);
 
  	header = smem->regions[0].virt_base;

@@ -640,6 +660,19 @@ static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
  	return le32_to_cpu(versions[SMEM_MASTER_SBL_VERSION_INDEX]);
  }
 
+static u32 qcom_smem_get_dynamic_item(struct qcom_smem *smem)
+{
+	struct smem_ptable *ptable;
+	struct smem_info *info;
+
+	ptable = smem->regions[0].virt_base + smem->regions[0].size - SZ_4K;
+	info = (struct smem_info *)&ptable->entry[ptable->num_entries];
+	if (memcmp(info->magic, SMEM_INFO_MAGIC, sizeof(info->magic)))
+		return SMEM_ITEM_COUNT;
+
+	return le32_to_cpu(info->num_items);
+}
+
  static int qcom_smem_set_global_partition(struct qcom_smem *smem,
  				struct smem_ptable_entry *entry)
  {

@@ -857,7 +890,11 @@ static int qcom_smem_probe(struct platform_device *pdev)
  	version = qcom_smem_get_sbl_version(smem);
  	switch (version >> 16) {
  	case SMEM_GLOBAL_PART_VERSION:
+		smem->item_count = qcom_smem_get_dynamic_item(smem);
+		break;
  	case SMEM_GLOBAL_HEAP_VERSION:
+		smem->item_count = SMEM_ITEM_COUNT;
+		break;
  	default:
  		dev_err(&pdev->dev, "Unsupported SMEM version 0x%x\n", version);
  		return -EINVAL;

SMEM V12 creates a global partition to allocate global smem items from instead of a global heap. The global partition has the same structure as a private partition.

Signed-off-by Chris Lew <[email protected]>

---
 drivers/soc/qcom/smem.c | 134 +++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 105 insertions(+), 29 deletions(-)

drivers/soc/qcom/smem.c +0 -0

index c28275be0038..fed2934d6bda 100644

drivers/soc/qcom/smem.c +0 -0

 	if (!__smem)
 		return -EPROBE_DEFER;

drivers/soc/qcom/smem.c +1 -1

@@ -604,21 +631,61 @@ int qcom_smem_get_free_space(unsigned host)
 
 static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
 {
+	struct smem_header *header;
 	__le32 *versions;
-	size_t size;

drivers/soc/qcom/smem.c +17 -3

+
+static int qcom_smem_set_global_partition(struct qcom_smem *smem,
+				struct smem_ptable_entry *entry)
+{
+	struct smem_partition_header *header;
+	u32  host0, host1;
+
+	if (!le32_to_cpu(entry->offset) || !le32_to_cpu(entry->size)) {
+		dev_err(smem->dev, "Invalid entry for gloabl partition\n");
+		return -EINVAL;
 	}
 
-	if (size < sizeof(unsigned) * SMEM_MASTER_SBL_VERSION_INDEX) {
-		dev_err(smem->dev, "Version item is too small\n");
+	if (smem->global_partition) {
+		dev_err(smem->dev, "Already found the global partition\n");
 		return -EINVAL;
 	}
+	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+	host0 = le16_to_cpu(header->host0);
+	host1 = le16_to_cpu(header->host1);
 
-	return le32_to_cpu(versions[SMEM_MASTER_SBL_VERSION_INDEX]);
+	if (memcmp(header->magic, SMEM_PART_MAGIC,
+		    sizeof(header->magic))) {

drivers/soc/qcom/smem.c +9 -0

+		dev_err(smem->dev,
+			"Global partition has invalid free pointer\n");
+		return -EINVAL;
+	}
+
+	smem->global_partition = header;
+	smem->global_cacheline = le32_to_cpu(entry->cacheline);
+
+	return 0;
 }
 
 static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,

In V12 SMEM, SBL writes SMEM parameter information after the TOC. Use the SBL provided item count as the max item number.

Signed-off-by Chris Lew <[email protected]>

---
 drivers/soc/qcom/smem.c | 41 +++++++++++++++++++++++++++++++++++++++--
 1 file changed, 39 insertions(+), 2 deletions(-)

drivers/soc/qcom/smem.c +0 -0

index fed2934d6bda..a51f4ba42173 100644

drivers/soc/qcom/smem.c +11 -2

+	__le32 reserved;
+	__le32 num_items;
+};
+
+static const u8 SMEM_INFO_MAGIC[] = { 0x53, 0x49, 0x49, 0x49 }; /* SIII */
+
+/**
  * struct smem_region - representation of a chunk of memory used for smem
  * @aux_base:	identifier of aux_mem base
  * @virt_base:	virtual base address of memory with this aux_mem identifier

@@ -245,6 +263,7 @@ struct smem_region {
  * @partitions:	list of pointers to partitions affecting the current
  *		processor/host
  * @cacheline:	list of cacheline sizes for each host
+ * @item_count: max accepted item number
  * @num_regions: number of @regions
  * @regions:	list of the memory regions defining the shared memory
  */

@@ -257,6 +276,7 @@ struct qcom_smem {
 	size_t global_cacheline;
 	struct smem_partition_header *partitions[SMEM_HOST_COUNT];
 	size_t cacheline[SMEM_HOST_COUNT];
+	u32 item_count;
 
 	unsigned num_regions;
 	struct smem_region regions[0];

@@ -388,7 +408,7 @@ static int qcom_smem_alloc_global(struct qcom_smem *smem,
 	struct smem_header *header;
 	struct smem_global_entry *entry;
 
-	if (WARN_ON(item >= SMEM_ITEM_COUNT))
+	if (WARN_ON(item >= smem->item_count))
 		return -EINVAL;
 
 	header = smem->regions[0].virt_base;

@@ -473,7 +493,7 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 	u32 aux_base;
 	unsigned i;
 
-	if (WARN_ON(item >= SMEM_ITEM_COUNT))
+	if (WARN_ON(item >= smem->item_count))
 		return ERR_PTR(-EINVAL);
 
 	header = smem->regions[0].virt_base;

Increase the maximum number of hosts in a system to 10.

Signed-off-by Chris Lew <[email protected]>

---
 drivers/soc/qcom/smem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/soc/qcom/smem.c +1 -1

index a51f4ba42173..4385f3b4bca9 100644
@@ -93,7 +93,7 @@
 #define SMEM_HOST_APPS		0
 
 /* Max number of processors/hosts in a system */
-#define SMEM_HOST_COUNT		9
+#define SMEM_HOST_COUNT		10
 
 /**
   * struct smem_proc_comm - proc_comm communication struct (legacy)

Signature · 2 lines

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

drivers/soc/qcom/smem.c line 79

Quoted 10 lines

  #define  SMEM_MASTER_SBL_VERSION_INDEX	7

drivers/soc/qcom/smem.c +8 -1

-#define  SMEM_EXPECTED_VERSION		11
+#define  SMEM_GLOBAL_HEAP_VERSION	11
+
+/*
+ * Version 12 (SMEM_GLOBAL_PART_VERSION) changes the item alloc/get procedure
+ * for the global heap. A new global partition is created from the global heap
+ * region with partition type (SMEM_GLOBAL_HOST) and the max smem item count is
+ * set by the bootloader.
+ */

Please incorporate this paragraph in the larger comment at the top of the file, so we keep that up to date.

[..]

drivers/soc/qcom/smem.c int qcom_smem_alloc(unsigned host, unsigned item, size_t size) · line 433

Quoted 5 lines

drivers/soc/qcom/smem.c +1 -0

@@ -419,6 +430,7 @@ int qcom_smem_alloc(unsigned host, unsigned item, size_t size)
  {
  	unsigned long flags;
  	int ret;
+	struct smem_partition_header *phdr;

Sorry for my OCD, but please maintain my reverse XMAS tree (put this declaration first, as it's the longest).

Quoted 3 lines

drivers/soc/qcom/smem.c +0 -0

  	if (!__smem)
  		return -EPROBE_DEFER;

[..]

Quoted 8 lines

drivers/soc/qcom/smem.c +1 -1

@@ -604,21 +631,61 @@ int qcom_smem_get_free_space(unsigned host)
 
  static int qcom_smem_get_sbl_version(struct qcom_smem *smem)
  {
+	struct smem_header *header;
  	__le32 *versions;
-	size_t size;
 

drivers/soc/qcom/smem.c int qcom_smem_get_free_space(unsigned host) · line 641

Quoted 9 lines

drivers/soc/qcom/smem.c +5 -4

-	versions = qcom_smem_get_global(smem, SMEM_ITEM_VERSION, &size);
-	if (IS_ERR(versions)) {
-		dev_err(smem->dev, "Unable to read the version item\n");
-		return -ENOENT;
+	header = smem->regions[0].virt_base;
+	versions = header->version;
+
+	return le32_to_cpu(versions[SMEM_MASTER_SBL_VERSION_INDEX]);
+}

I would prefer if you split the change of this function into its own patch, just to clarify that it's unrelated to the new version support.

Quoted 22 lines

drivers/soc/qcom/smem.c +15 -2

+
+static int qcom_smem_set_global_partition(struct qcom_smem *smem,
+				struct smem_ptable_entry *entry)
+{
+	struct smem_partition_header *header;
+	u32  host0, host1;
+
+	if (!le32_to_cpu(entry->offset) || !le32_to_cpu(entry->size)) {
+		dev_err(smem->dev, "Invalid entry for gloabl partition\n");
+		return -EINVAL;
  	}
 
-	if (size < sizeof(unsigned) * SMEM_MASTER_SBL_VERSION_INDEX) {
-		dev_err(smem->dev, "Version item is too small\n");
+	if (smem->global_partition) {
+		dev_err(smem->dev, "Already found the global partition\n");
  		return -EINVAL;
  	}
+	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+	host0 = le16_to_cpu(header->host0);
+	host1 = le16_to_cpu(header->host1);
 

drivers/soc/qcom/smem.c int qcom_smem_get_free_space(unsigned host) · line 673

Quoted 13 lines

drivers/soc/qcom/smem.c +12 -1

-	return le32_to_cpu(versions[SMEM_MASTER_SBL_VERSION_INDEX]);
+	if (memcmp(header->magic, SMEM_PART_MAGIC,
+		    sizeof(header->magic))) {
+		dev_err(smem->dev, "Gloal partition has invalid magic\n");
+		return -EINVAL;
+	}
+
+	if (host0 != SMEM_GLOBAL_HOST && host1 != SMEM_GLOBAL_HOST) {
+		dev_err(smem->dev, "Global partition hosts are invalid\n");
+		return -EINVAL;
+	}
+
+	if (header->size != entry->size) {

This happens to work, because they are both in the same endian. But please sprinkle some le32_to_cpu() here as well.

drivers/soc/qcom/smem.c int qcom_smem_get_free_space(unsigned host) · line 679

Quoted 6 lines

drivers/soc/qcom/smem.c +6 -0

+		dev_err(smem->dev, "Global partition has invalid size\n");
+		return -EINVAL;
+	}
+
+	if (le32_to_cpu(header->offset_free_uncached) >
+			le32_to_cpu(header->size)) {

Consider using local variables in favor of wrapping lines.

Quoted 16 lines

drivers/soc/qcom/smem.c +9 -0

+		dev_err(smem->dev,
+			"Global partition has invalid free pointer\n");
+		return -EINVAL;
+	}
+
+	smem->global_partition = header;
+	smem->global_cacheline = le32_to_cpu(entry->cacheline);
+
+	return 0;
  }
 
  static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,

@@ -647,6 +714,12 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
  		host0 = le16_to_cpu(entry->host0);
  		host1 = le16_to_cpu(entry->host1);
 

drivers/soc/qcom/smem.c static int qcom_smem_enumerate_partitions(struct qcom_smem *smem, · line 722

Quoted 6 lines

drivers/soc/qcom/smem.c +6 -0

+		if (host0 == SMEM_GLOBAL_HOST && host0 == host1) {
+			if (qcom_smem_set_global_partition(smem, entry))
+				return -EINVAL;
+			continue;
+		}
+

As you're not able to leverage any of the checks from this loop I think it's cleaner to duplicate the traversal of the partition table in both functions and call the "search for global partition" directly from probe - if the version indicates there should be one.

Quoted 6 lines

drivers/soc/qcom/smem.c +0 -0

  		if (host0 != local_host && host1 != local_host)
  			continue;
 

@@ -782,7 +855,10 @@ static int qcom_smem_probe(struct platform_device *pdev)
  	}
 

drivers/soc/qcom/smem.c static int qcom_smem_probe(struct platform_device *pdev) · line 860

Quoted 5 lines

drivers/soc/qcom/smem.c +3 -1

  	version = qcom_smem_get_sbl_version(smem);
-	if (version >> 16 != SMEM_EXPECTED_VERSION) {
+	switch (version >> 16) {
+	case SMEM_GLOBAL_PART_VERSION:
+	case SMEM_GLOBAL_HEAP_VERSION:

As Arun pointed out, there should be a "break" here.

On 8/21/2017 10:17 AM, Bjorn Andersson wrote:

Quoted 3 lines

Quoted 3 lines

drivers/soc/qcom/smem.c +3 -0

+static int qcom_smem_set_global_partition(struct qcom_smem *smem,
+				struct smem_ptable_entry *entry)
+{

Quoted 5 lines

drivers/soc/qcom/smem.c int qcom_smem_get_free_space(unsigned host) · line 673

Quoted 1 line

drivers/soc/qcom/smem.c +1 -0

+	if (header->size != entry->size) {

This happens to work, because they are both in the same endian. But please sprinkle some le32_to_cpu() here as well.

These checks mimic the sanity checks being done in enumerate_partitions. Should I create a patch to increase le32_to_cpu usage in qcom_smem_enumerate_partitions?

Quoted 6 lines

Quoted 6 lines

drivers/soc/qcom/smem.c +3 -0

@@ -647,6 +714,12 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
  		host0 = le16_to_cpu(entry->host0);
  		host1 = le16_to_cpu(entry->host1);
+		if (host0 == SMEM_GLOBAL_HOST && host0 == host1) {
+			if (qcom_smem_set_global_partition(smem, entry))
+				return -EINVAL;

Quoted 9 lines

drivers/soc/qcom/smem.c static int qcom_smem_enumerate_partitions(struct qcom_smem *smem, · line 722

Quoted 3 lines

drivers/soc/qcom/smem.c +3 -0

+			continue;
+		}
+

As you're not able to leverage any of the checks from this loop I think it's cleaner to duplicate the traversal of the partition table in both functions and call the "search for global partition" directly from probe - if the version indicates there should be one.

Ok, will set the global partition in the version case statement and error out of the probe if finding the global partition fails since it is not optional in the new version.