Linux ARM-MSM sub-architecture
 help / color / mirror / Atom feed
* [PATCH 00/12] soc: qcom: smem: some refactoring
@ 2018-05-02  1:10 Alex Elder
  2018-05-02  1:10 ` [PATCH 01/12] soc: qcom: smem: rename variable in qcom_smem_get_global() Alex Elder
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

This series contains a few general cleanups in the Qualcomm SMEM
code, reorganizing some things so memory partitions are treated
more consistently, and to avoid some duplication.  A few patches
change existing behavior, as highlighted below.  Each of these
changes has been tested and verified to function with available
hardware.

					-Alex

Alex Elder (12):
  soc: qcom: smem: rename variable in qcom_smem_get_global()
  soc: qcom: smem: initialize region struct only when successful
	These two are simple cleanups.

  soc: qcom: smem: always ignore partitions with 0 offset or size
  	This patch makes partitions with a zero offset always be
	ignored.  Previously this condition was flagged as an error
	in one place and ignored in another.

  soc: qcom: smem: small refactor in qcom_smem_enumerate_partitions()
  	This just simplifies a little logic in one spot.

  soc: qcom: smem: verify both host ids in partition header
  	This patch makes it so *both* IDs, not just one or the
	other, must use the special value SMEM_GLOBAL_HOST to
	signify the special global partition.

  soc: qcom: smem: require order of host ids to match
	A memory partition is described by an entry in a partition
	table, which identifies two hosts permitted to access the
	partition.  A partition also embeds a header, which also
	identifies the hosts.  Previously the host ids could be
	in any order; now the two host ids are required to be
	listed in the same order in the partition table and header.

  soc: qcom: smem: introduce qcom_smem_partition_header()
  soc: qcom: smem: verify partition header size
  soc: qcom: smem: verify partition offset_free_uncached
  soc: qcom: smem: small change in global entry loop
  soc: qcom: smem: verify partition host ids match
  	These patches together introduce a single function
	to locate a partition header in memory, and to validate its
	contents.  The common function is used in two place and
	removes some duplication.  The patches build up the function
	in steps to facilitate review.

  soc: qcom: smem: a few last cleanups
  	This patch gathers a small set of trivial cleanups.

 drivers/soc/qcom/smem.c | 173 +++++++++++++++++++++++-------------------------
 1 file changed, 82 insertions(+), 91 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 01/12] soc: qcom: smem: rename variable in qcom_smem_get_global()
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 02/12] soc: qcom: smem: initialize region struct only when successful Alex Elder
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Rename the variable "area" to be "region" in qcom_smem_get_global(),
so its name better matches its type.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 70b2ee80d6bd..c46bb43c0f3d 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -277,7 +277,7 @@ struct qcom_smem {
 	u32 item_count;
 
 	unsigned num_regions;
-	struct smem_region regions[0];
+	struct smem_region regions[];
 };
 
 static void *
@@ -494,7 +494,7 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 				  size_t *size)
 {
 	struct smem_header *header;
-	struct smem_region *area;
+	struct smem_region *region;
 	struct smem_global_entry *entry;
 	u32 aux_base;
 	unsigned i;
@@ -507,12 +507,12 @@ static void *qcom_smem_get_global(struct qcom_smem *smem,
 	aux_base = le32_to_cpu(entry->aux_base) & AUX_BASE_MASK;
 
 	for (i = 0; i < smem->num_regions; i++) {
-		area = &smem->regions[i];
+		region = &smem->regions[i];
 
-		if (area->aux_base == aux_base || !aux_base) {
+		if (region->aux_base == aux_base || !aux_base) {
 			if (size != NULL)
 				*size = le32_to_cpu(entry->size);
-			return area->virt_base + le32_to_cpu(entry->offset);
+			return region->virt_base + le32_to_cpu(entry->offset);
 		}
 	}
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 02/12] soc: qcom: smem: initialize region struct only when successful
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
  2018-05-02  1:10 ` [PATCH 01/12] soc: qcom: smem: rename variable in qcom_smem_get_global() Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 03/12] soc: qcom: smem: always ignore partitions with 0 offset or size Alex Elder
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Hold off initializing anything for the array entry representing a
memory region in qcom_smem_map_memory() until we know we've
successfully mapped it.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index c46bb43c0f3d..d8008bf480a4 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -887,6 +887,7 @@ static int qcom_smem_map_memory(struct qcom_smem *smem, struct device *dev,
 {
 	struct device_node *np;
 	struct resource r;
+	resource_size_t size;
 	int ret;
 
 	np = of_parse_phandle(dev->of_node, name, 0);
@@ -899,12 +900,13 @@ static int qcom_smem_map_memory(struct qcom_smem *smem, struct device *dev,
 	of_node_put(np);
 	if (ret)
 		return ret;
+	size = resource_size(&r);
 
-	smem->regions[i].aux_base = (u32)r.start;
-	smem->regions[i].size = resource_size(&r);
-	smem->regions[i].virt_base = devm_ioremap_wc(dev, r.start, resource_size(&r));
+	smem->regions[i].virt_base = devm_ioremap_wc(dev, r.start, size);
 	if (!smem->regions[i].virt_base)
 		return -ENOMEM;
+	smem->regions[i].aux_base = (u32)r.start;
+	smem->regions[i].size = size;
 
 	return 0;
 }
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 03/12] soc: qcom: smem: always ignore partitions with 0 offset or size
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
  2018-05-02  1:10 ` [PATCH 01/12] soc: qcom: smem: rename variable in qcom_smem_get_global() Alex Elder
  2018-05-02  1:10 ` [PATCH 02/12] soc: qcom: smem: initialize region struct only when successful Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 04/12] soc: qcom: smem: small refactor in qcom_smem_enumerate_partitions() Alex Elder
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

In qcom_smem_enumerate_partitions(), any partition table entry
having a zero offset or size field is ignored.  Move those checks
earlier in the loop, because there's no sense in examining the
host fields for those entries.

Add the same checks in qcom_smem_set_global_partition(), so the
scan for the global partition skips over these invalid entries.
This allows a later check for zero size or offset once the global
entry is found to be eliminated.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index d8008bf480a4..83ba549cfea0 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -742,9 +742,13 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 
 	for (i = 0; i < le32_to_cpu(ptable->num_entries); i++) {
 		entry = &ptable->entry[i];
+		if (!le32_to_cpu(entry->offset))
+			continue;
+		if (!le32_to_cpu(entry->size))
+			continue;
+
 		host0 = le16_to_cpu(entry->host0);
 		host1 = le16_to_cpu(entry->host1);
-
 		if (host0 == SMEM_GLOBAL_HOST && host0 == host1) {
 			found = true;
 			break;
@@ -756,11 +760,6 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	if (!le32_to_cpu(entry->offset) || !le32_to_cpu(entry->size)) {
-		dev_err(smem->dev, "Invalid entry for 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);
@@ -809,18 +808,16 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 
 	for (i = 0; i < le32_to_cpu(ptable->num_entries); i++) {
 		entry = &ptable->entry[i];
+		if (!le32_to_cpu(entry->offset))
+			continue;
+		if (!le32_to_cpu(entry->size))
+			continue;
+
 		host0 = le16_to_cpu(entry->host0);
 		host1 = le16_to_cpu(entry->host1);
-
 		if (host0 != local_host && host1 != local_host)
 			continue;
 
-		if (!le32_to_cpu(entry->offset))
-			continue;
-
-		if (!le32_to_cpu(entry->size))
-			continue;
-
 		if (host0 == local_host)
 			remote_host = host1;
 		else
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 04/12] soc: qcom: smem: small refactor in qcom_smem_enumerate_partitions()
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (2 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 03/12] soc: qcom: smem: always ignore partitions with 0 offset or size Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 05/12] soc: qcom: smem: verify both host ids in partition header Alex Elder
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Combine the code that checks whether a partition table entry is
associated with the local host with the assignment of the remote
host id value.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 83ba549cfea0..5d3ed510e54b 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -815,13 +815,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 != local_host && host1 != local_host)
-			continue;
-
 		if (host0 == local_host)
 			remote_host = host1;
-		else
+		else if (host1 == local_host)
 			remote_host = host0;
+		else
+			continue;
 
 		if (remote_host >= SMEM_HOST_COUNT) {
 			dev_err(smem->dev,
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 05/12] soc: qcom: smem: verify both host ids in partition header
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (3 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 04/12] soc: qcom: smem: small refactor in qcom_smem_enumerate_partitions() Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 06/12] soc: qcom: smem: require order of host ids to match Alex Elder
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

The global partition is indicated by having both host values in its
table of contents entry equal SMEM_GLOBAL_HOST=0xfffe.

In qcom_smem_set_global_partition(), we check whether the header
structure at the beginning of the partition contains that host
value, but the check only verifies *one* of them.  Change the check
so the partition header must have SMEM_GLOBAL_HOST for *both* its
host fields.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 5d3ed510e54b..6931602d9a9e 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -769,7 +769,7 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	if (host0 != SMEM_GLOBAL_HOST && host1 != SMEM_GLOBAL_HOST) {
+	if (host0 != SMEM_GLOBAL_HOST || host1 != SMEM_GLOBAL_HOST) {
 		dev_err(smem->dev, "Global partition hosts are invalid\n");
 		return -EINVAL;
 	}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 06/12] soc: qcom: smem: require order of host ids to match
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (4 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 05/12] soc: qcom: smem: verify both host ids in partition header Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 07/12] soc: qcom: smem: introduce qcom_smem_partition_header() Alex Elder
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

In qcom_smem_enumerate_partitions(), we find all partitions that
have a given local host id in either its host0 or its host1 field
in the partition table entry.  We then verify that the header
structure at the start of each partition also contains the same two
host ids as is found in the table of contents.

There is no requirement that the order of the two host ids be the
same in the table of contents and in the partition header.

This patch changes that, requiring host0 to in the partition table
entry to equal host0 in the partition header structure (and similar
for the host1 values).

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 6931602d9a9e..9e20c6b1b4c2 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -837,9 +837,6 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 		}
 
 		header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
-		host0 = le16_to_cpu(header->host0);
-		host1 = le16_to_cpu(header->host1);
-
 		if (memcmp(header->magic, SMEM_PART_MAGIC,
 			    sizeof(header->magic))) {
 			dev_err(smem->dev,
@@ -847,15 +844,10 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			return -EINVAL;
 		}
 
-		if (host0 != local_host && host1 != local_host) {
+		if (host0 != le16_to_cpu(header->host0) ||
+				host1 != le16_to_cpu(header->host1)) {
 			dev_err(smem->dev,
-				"Partition %d hosts are invalid\n", i);
-			return -EINVAL;
-		}
-
-		if (host0 != remote_host && host1 != remote_host) {
-			dev_err(smem->dev,
-				"Partition %d hosts are invalid\n", i);
+				"Partition %d hosts don't match\n", i);
 			return -EINVAL;
 		}
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 07/12] soc: qcom: smem: introduce qcom_smem_partition_header()
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (5 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 06/12] soc: qcom: smem: require order of host ids to match Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 08/12] soc: qcom: smem: verify partition header size Alex Elder
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Create a new function qcom_smem_partition_header() to encapsulate
validating the header information found in a partition.  This will
be built up over a few commits to make it more obvious how the
common function is replacing duplicated code elsewhere.  Initially
it just verifies the header has the right magic number.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 9e20c6b1b4c2..d52a282f7338 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -727,6 +727,29 @@ static u32 qcom_smem_get_item_count(struct qcom_smem *smem)
 	return le16_to_cpu(info->num_items);
 }
 
+/*
+ * Validate the partition header for a partition whose partition
+ * table entry is supplied.  Returns a pointer to its header if
+ * valid, or a null pointer otherwise.
+ */
+static struct smem_partition_header *
+qcom_smem_partition_header(struct qcom_smem *smem,
+		struct smem_ptable_entry *entry)
+{
+	struct smem_partition_header *header;
+
+	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+
+	if (memcmp(header->magic, SMEM_PART_MAGIC, sizeof(header->magic))) {
+		dev_err(smem->dev, "bad partition magic %02x %02x %02x %02x\n",
+			header->magic[0], header->magic[1],
+			header->magic[2], header->magic[3]);
+		return NULL;
+	}
+
+	return header;
+}
+
 static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 {
 	struct smem_partition_header *header;
@@ -760,15 +783,13 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
+	header = qcom_smem_partition_header(smem, entry);
+	if (!header)
+		return -EINVAL;
+
 	host0 = le16_to_cpu(header->host0);
 	host1 = le16_to_cpu(header->host1);
 
-	if (memcmp(header->magic, SMEM_PART_MAGIC, sizeof(header->magic))) {
-		dev_err(smem->dev, "Global 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;
@@ -836,13 +857,9 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			return -EINVAL;
 		}
 
-		header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
-		if (memcmp(header->magic, SMEM_PART_MAGIC,
-			    sizeof(header->magic))) {
-			dev_err(smem->dev,
-				"Partition %d has invalid magic\n", i);
+		header = qcom_smem_partition_header(smem, entry);
+		if (!header)
 			return -EINVAL;
-		}
 
 		if (host0 != le16_to_cpu(header->host0) ||
 				host1 != le16_to_cpu(header->host1)) {
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 08/12] soc: qcom: smem: verify partition header size
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (6 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 07/12] soc: qcom: smem: introduce qcom_smem_partition_header() Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 09/12] soc: qcom: smem: verify partition offset_free_uncached Alex Elder
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Add verification in qcom_smem_partition_header() that the size in a
partition's header structure matches the size in its partition table
entry.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index d52a282f7338..f1e6e4609ccb 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -737,6 +737,7 @@ qcom_smem_partition_header(struct qcom_smem *smem,
 		struct smem_ptable_entry *entry)
 {
 	struct smem_partition_header *header;
+	u32 size;
 
 	header = smem->regions[0].virt_base + le32_to_cpu(entry->offset);
 
@@ -747,6 +748,13 @@ qcom_smem_partition_header(struct qcom_smem *smem,
 		return NULL;
 	}
 
+	size = le32_to_cpu(header->size);
+	if (size != le32_to_cpu(entry->size)) {
+		dev_err(smem->dev, "bad partition size (%u != %u)\n",
+			size, le32_to_cpu(entry->size));
+		return NULL;
+	}
+
 	return header;
 }
 
@@ -795,11 +803,6 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
-		dev_err(smem->dev, "Global partition has invalid size\n");
-		return -EINVAL;
-	}
-
 	size = le32_to_cpu(header->offset_free_uncached);
 	if (size > le32_to_cpu(header->size)) {
 		dev_err(smem->dev,
@@ -868,12 +871,6 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			return -EINVAL;
 		}
 
-		if (le32_to_cpu(header->size) != le32_to_cpu(entry->size)) {
-			dev_err(smem->dev,
-				"Partition %d has invalid size\n", i);
-			return -EINVAL;
-		}
-
 		if (le32_to_cpu(header->offset_free_uncached) > le32_to_cpu(header->size)) {
 			dev_err(smem->dev,
 				"Partition %d has invalid free pointer\n", i);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 09/12] soc: qcom: smem: verify partition offset_free_uncached
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (7 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 08/12] soc: qcom: smem: verify partition header size Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 10/12] soc: qcom: smem: small change in global entry loop Alex Elder
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Add verification in qcom_smem_partition_header() that the
offset_free_uncached field in a partition's header structure does
not exceed the partition's size.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index f1e6e4609ccb..7ef415c4d62e 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -755,6 +755,12 @@ qcom_smem_partition_header(struct qcom_smem *smem,
 		return NULL;
 	}
 
+	if (le32_to_cpu(header->offset_free_uncached) > size) {
+		dev_err(smem->dev, "bad partition free uncached (%u > %u)\n",
+			le32_to_cpu(header->offset_free_uncached), size);
+		return NULL;
+	}
+
 	return header;
 }
 
@@ -763,7 +769,7 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 	struct smem_partition_header *header;
 	struct smem_ptable_entry *entry;
 	struct smem_ptable *ptable;
-	u32 host0, host1, size;
+	u32 host0, host1;
 	bool found = false;
 	int i;
 
@@ -803,13 +809,6 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	size = le32_to_cpu(header->offset_free_uncached);
-	if (size > le32_to_cpu(header->size)) {
-		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);
 
@@ -871,12 +870,6 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			return -EINVAL;
 		}
 
-		if (le32_to_cpu(header->offset_free_uncached) > le32_to_cpu(header->size)) {
-			dev_err(smem->dev,
-				"Partition %d has invalid free pointer\n", i);
-			return -EINVAL;
-		}
-
 		smem->partitions[remote_host] = header;
 		smem->cacheline[remote_host] = le32_to_cpu(entry->cacheline);
 	}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 10/12] soc: qcom: smem: small change in global entry loop
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (8 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 09/12] soc: qcom: smem: verify partition offset_free_uncached Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 11/12] soc: qcom: smem: verify partition host ids match Alex Elder
  2018-05-02  1:10 ` [PATCH 12/12] soc: qcom: smem: a few last cleanups Alex Elder
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Change the logic in the loop that finds that global host entry in
the partition table not require the host0 and host1 local variables.
The next patch will remove them.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 7ef415c4d62e..54c504e41508 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -784,9 +784,10 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		if (!le32_to_cpu(entry->size))
 			continue;
 
-		host0 = le16_to_cpu(entry->host0);
-		host1 = le16_to_cpu(entry->host1);
-		if (host0 == SMEM_GLOBAL_HOST && host0 == host1) {
+		if (le16_to_cpu(entry->host0) != SMEM_GLOBAL_HOST)
+			continue;
+
+		if (le16_to_cpu(entry->host1) == SMEM_GLOBAL_HOST) {
 			found = true;
 			break;
 		}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 11/12] soc: qcom: smem: verify partition host ids match
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (9 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 10/12] soc: qcom: smem: small change in global entry loop Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  2018-05-02  1:10 ` [PATCH 12/12] soc: qcom: smem: a few last cleanups Alex Elder
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

Add verification in qcom_smem_partition_header() that the host ids
found in a partition's header structure match those in its partition
table entry.

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 34 +++++++++++++++-------------------
 1 file changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 54c504e41508..7383a0e1b468 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -734,7 +734,7 @@ static u32 qcom_smem_get_item_count(struct qcom_smem *smem)
  */
 static struct smem_partition_header *
 qcom_smem_partition_header(struct qcom_smem *smem,
-		struct smem_ptable_entry *entry)
+		struct smem_ptable_entry *entry, u16 host0, u16 host1)
 {
 	struct smem_partition_header *header;
 	u32 size;
@@ -748,6 +748,17 @@ qcom_smem_partition_header(struct qcom_smem *smem,
 		return NULL;
 	}
 
+	if (host0 != le16_to_cpu(header->host0)) {
+		dev_err(smem->dev, "bad host0 (%hu != %hu)\n",
+				host0, le16_to_cpu(header->host0));
+		return NULL;
+	}
+	if (host1 != le16_to_cpu(header->host1)) {
+		dev_err(smem->dev, "bad host1 (%hu != %hu)\n",
+				host1, le16_to_cpu(header->host1));
+		return NULL;
+	}
+
 	size = le32_to_cpu(header->size);
 	if (size != le32_to_cpu(entry->size)) {
 		dev_err(smem->dev, "bad partition size (%u != %u)\n",
@@ -769,7 +780,6 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 	struct smem_partition_header *header;
 	struct smem_ptable_entry *entry;
 	struct smem_ptable *ptable;
-	u32 host0, host1;
 	bool found = false;
 	int i;
 
@@ -798,18 +808,11 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 		return -EINVAL;
 	}
 
-	header = qcom_smem_partition_header(smem, entry);
+	header = qcom_smem_partition_header(smem, entry,
+				SMEM_GLOBAL_HOST, SMEM_GLOBAL_HOST);
 	if (!header)
 		return -EINVAL;
 
-	host0 = le16_to_cpu(header->host0);
-	host1 = le16_to_cpu(header->host1);
-
-	if (host0 != SMEM_GLOBAL_HOST || host1 != SMEM_GLOBAL_HOST) {
-		dev_err(smem->dev, "Global partition hosts are invalid\n");
-		return -EINVAL;
-	}
-
 	smem->global_partition = header;
 	smem->global_cacheline = le32_to_cpu(entry->cacheline);
 
@@ -860,17 +863,10 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			return -EINVAL;
 		}
 
-		header = qcom_smem_partition_header(smem, entry);
+		header = qcom_smem_partition_header(smem, entry, host0, host1);
 		if (!header)
 			return -EINVAL;
 
-		if (host0 != le16_to_cpu(header->host0) ||
-				host1 != le16_to_cpu(header->host1)) {
-			dev_err(smem->dev,
-				"Partition %d hosts don't match\n", i);
-			return -EINVAL;
-		}
-
 		smem->partitions[remote_host] = header;
 		smem->cacheline[remote_host] = le32_to_cpu(entry->cacheline);
 	}
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 12/12] soc: qcom: smem: a few last cleanups
  2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
                   ` (10 preceding siblings ...)
  2018-05-02  1:10 ` [PATCH 11/12] soc: qcom: smem: verify partition host ids match Alex Elder
@ 2018-05-02  1:10 ` Alex Elder
  11 siblings, 0 replies; 13+ messages in thread
From: Alex Elder @ 2018-05-02  1:10 UTC (permalink / raw)
  To: andy.gross
  Cc: clew, aneela, david.brown, linux-arm-msm, linux-soc, linux-kernel

This patch contains several small cleanups:
  - In qcom_smem_enumerate_partitions(), change the "local_host"
    argument to have 16 bit unsigned type
  - Also in qcom_smem_enumerate_partitions(), change the type of
    the "host0" and "host1" local variables to be u16
  - Fix error messages reporting host ids to use the right format
    specifier
  - Shorten the error messages as well, to fit on one line
  - Add a compile-time check to ensure the local host value passed
    to qcom_smem_enumerate_partitions() is in range

Signed-off-by: Alex Elder <elder@linaro.org>
---
 drivers/soc/qcom/smem.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 7383a0e1b468..0d7f369c3f7a 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -819,14 +819,14 @@ static int qcom_smem_set_global_partition(struct qcom_smem *smem)
 	return 0;
 }
 
-static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
-					  unsigned int local_host)
+static int
+qcom_smem_enumerate_partitions(struct qcom_smem *smem, u16 local_host)
 {
 	struct smem_partition_header *header;
 	struct smem_ptable_entry *entry;
 	struct smem_ptable *ptable;
 	unsigned int remote_host;
-	u32 host0, host1;
+	u16 host0, host1;
 	int i;
 
 	ptable = qcom_smem_get_ptable(smem);
@@ -850,16 +850,12 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
 			continue;
 
 		if (remote_host >= SMEM_HOST_COUNT) {
-			dev_err(smem->dev,
-				"Invalid remote host %d\n",
-				remote_host);
+			dev_err(smem->dev, "bad host %hu\n", remote_host);
 			return -EINVAL;
 		}
 
 		if (smem->partitions[remote_host]) {
-			dev_err(smem->dev,
-				"Already found a partition for host %d\n",
-				remote_host);
+			dev_err(smem->dev, "duplicate host %hu\n", remote_host);
 			return -EINVAL;
 		}
 
@@ -956,6 +952,7 @@ static int qcom_smem_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
+	BUILD_BUG_ON(SMEM_HOST_APPS >= SMEM_HOST_COUNT);
 	ret = qcom_smem_enumerate_partitions(smem, SMEM_HOST_APPS);
 	if (ret < 0 && ret != -ENOENT)
 		return ret;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2018-05-02  1:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-02  1:10 [PATCH 00/12] soc: qcom: smem: some refactoring Alex Elder
2018-05-02  1:10 ` [PATCH 01/12] soc: qcom: smem: rename variable in qcom_smem_get_global() Alex Elder
2018-05-02  1:10 ` [PATCH 02/12] soc: qcom: smem: initialize region struct only when successful Alex Elder
2018-05-02  1:10 ` [PATCH 03/12] soc: qcom: smem: always ignore partitions with 0 offset or size Alex Elder
2018-05-02  1:10 ` [PATCH 04/12] soc: qcom: smem: small refactor in qcom_smem_enumerate_partitions() Alex Elder
2018-05-02  1:10 ` [PATCH 05/12] soc: qcom: smem: verify both host ids in partition header Alex Elder
2018-05-02  1:10 ` [PATCH 06/12] soc: qcom: smem: require order of host ids to match Alex Elder
2018-05-02  1:10 ` [PATCH 07/12] soc: qcom: smem: introduce qcom_smem_partition_header() Alex Elder
2018-05-02  1:10 ` [PATCH 08/12] soc: qcom: smem: verify partition header size Alex Elder
2018-05-02  1:10 ` [PATCH 09/12] soc: qcom: smem: verify partition offset_free_uncached Alex Elder
2018-05-02  1:10 ` [PATCH 10/12] soc: qcom: smem: small change in global entry loop Alex Elder
2018-05-02  1:10 ` [PATCH 11/12] soc: qcom: smem: verify partition host ids match Alex Elder
2018-05-02  1:10 ` [PATCH 12/12] soc: qcom: smem: a few last cleanups Alex Elder

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox