When writing the SMEM implementation no public kernel had a consumer specifying
the "cached" flag, so I ommitted this part of the implementation. On MSM8996
this has changed, where we have one user of this - namely the GLINK RX FIFO.
The remote is supposedly scanning both lists for the GLINK descriptors and TX
FIFO, so we don't need to support allocating from this list at this point. This
has been confirmed in initial testing (by booting the ADSP)
Bjorn Andersson (2):
soc: qcom: smem: Rename "uncached" accessors
soc: qcom: smem: Support getting cached entries
drivers/soc/qcom/smem.c | 93 ++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 73 insertions(+), 20 deletions(-)
--
2.12.0
On msm8996 cached SMEM items are used for storing the GLINK FIFOs, so
for items not found in the uncached list we need to also search the
cased list for these items.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Changes since v1:
- phdr_to_first_cached_entry() endian conversion
- e->size is a le32, not le16
drivers/soc/qcom/smem.c | 69 +++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index b451dbc4aa39..c28275be0038 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -52,7 +52,8 @@
*
* Items in the non-cached region are allocated from the start of the partition
* while items in the cached region are allocated from the end. The free area
- * is hence the region between the cached and non-cached offsets.
+ * is hence the region between the cached and non-cached offsets. The header of
+ * cached items comes after the data.
*
*
* To synchronize allocations in the shared memory heaps a remote spinlock must
@@ -140,6 +141,7 @@ struct smem_header {
* @flags: flags for the partition (currently unused)
* @host0: first processor/host with access to this partition
* @host1: second processor/host with access to this partition
+ * @cacheline: alignment for "cached" entries
* @reserved: reserved entries for later use
*/
struct smem_ptable_entry {
@@ -148,7 +150,8 @@ struct smem_ptable_entry {
__le32 flags;
__le16 host0;
__le16 host1;
- __le32 reserved[8];
+ __le32 cacheline;
+ __le32 reserved[7];
};
/**
@@ -230,6 +233,7 @@ struct smem_region {
* @hwlock: reference to a hwspinlock
* @partitions: list of pointers to partitions affecting the current
* processor/host
+ * @cacheline: list of cacheline sizes for each host
* @num_regions: number of @regions
* @regions: list of the memory regions defining the shared memory
*/
@@ -239,6 +243,7 @@ struct qcom_smem {
struct hwspinlock *hwlock;
struct smem_partition_header *partitions[SMEM_HOST_COUNT];
+ size_t cacheline[SMEM_HOST_COUNT];
unsigned num_regions;
struct smem_region regions[0];
@@ -252,6 +257,14 @@ phdr_to_last_uncached_entry(struct smem_partition_header *phdr)
return p + le32_to_cpu(phdr->offset_free_uncached);
}
+static void *phdr_to_first_cached_entry(struct smem_partition_header *phdr,
+ size_t cacheline)
+{
+ void *p = phdr;
+
+ return p + le32_to_cpu(phdr->size) - ALIGN(sizeof(*phdr), cacheline);
+}
+
static void *phdr_to_last_cached_entry(struct smem_partition_header *phdr)
{
void *p = phdr;
@@ -276,6 +289,14 @@ uncached_entry_next(struct smem_private_entry *e)
le32_to_cpu(e->size);
}
+static struct smem_private_entry *
+cached_entry_next(struct smem_private_entry *e, size_t cacheline)
+{
+ void *p = e;
+
+ return p - le32_to_cpu(e->size) - ALIGN(sizeof(*e), cacheline);
+}
+
static void *uncached_entry_to_item(struct smem_private_entry *e)
{
void *p = e;
@@ -283,6 +304,13 @@ static void *uncached_entry_to_item(struct smem_private_entry *e)
return p + sizeof(*e) + le16_to_cpu(e->padding_hdr);
}
+static void *cached_entry_to_item(struct smem_private_entry *e)
+{
+ void *p = e;
+
+ return p - le32_to_cpu(e->size);
+}
+
/* Pointer to the one and only smem handle */
static struct qcom_smem *__smem;
@@ -458,18 +486,17 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
{
struct smem_partition_header *phdr;
struct smem_private_entry *e, *end;
+ size_t cacheline;
phdr = smem->partitions[host];
+ cacheline = smem->cacheline[host];
+
e = phdr_to_first_uncached_entry(phdr);
end = phdr_to_last_uncached_entry(phdr);
while (e < end) {
- if (e->canary != SMEM_PRIVATE_CANARY) {
- dev_err(smem->dev,
- "Found invalid canary in host %d partition\n",
- host);
- return ERR_PTR(-EINVAL);
- }
+ if (e->canary != SMEM_PRIVATE_CANARY)
+ goto invalid_canary;
if (le16_to_cpu(e->item) == item) {
if (size != NULL)
@@ -482,7 +509,32 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
e = uncached_entry_next(e);
}
+ /* Item was not found in the uncached list, search the cached list */
+
+ e = phdr_to_first_cached_entry(phdr, cacheline);
+ end = phdr_to_last_cached_entry(phdr);
+
+ while (e > end) {
+ if (e->canary != SMEM_PRIVATE_CANARY)
+ goto invalid_canary;
+
+ if (le16_to_cpu(e->item) == item) {
+ if (size != NULL)
+ *size = le32_to_cpu(e->size) -
+ le16_to_cpu(e->padding_data);
+
+ return cached_entry_to_item(e);
+ }
+
+ e = cached_entry_next(e, cacheline);
+ }
+
return ERR_PTR(-ENOENT);
+
+invalid_canary:
+ dev_err(smem->dev, "Found invalid canary in host %d partition\n", host);
+
+ return ERR_PTR(-EINVAL);
}
/**
@@ -659,6 +711,7 @@ static int qcom_smem_enumerate_partitions(struct qcom_smem *smem,
}
smem->partitions[remote_host] = header;
+ smem->cacheline[remote_host] = le32_to_cpu(entry->cacheline);
}
return 0;
--
2.12.0
In preparation for adding accessors for "cached" entries rename the
"uncached" accessors to. Also rename "first" cached entry to "last", as
the cached list grows backwards.
Signed-off-by: Bjorn Andersson <[email protected]>
---
Chances since v1:
- None
drivers/soc/qcom/smem.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/drivers/soc/qcom/smem.c b/drivers/soc/qcom/smem.c
index 18ec52f2078a..b451dbc4aa39 100644
--- a/drivers/soc/qcom/smem.c
+++ b/drivers/soc/qcom/smem.c
@@ -245,14 +245,14 @@ struct qcom_smem {
};
static struct smem_private_entry *
-phdr_to_last_private_entry(struct smem_partition_header *phdr)
+phdr_to_last_uncached_entry(struct smem_partition_header *phdr)
{
void *p = phdr;
return p + le32_to_cpu(phdr->offset_free_uncached);
}
-static void *phdr_to_first_cached_entry(struct smem_partition_header *phdr)
+static void *phdr_to_last_cached_entry(struct smem_partition_header *phdr)
{
void *p = phdr;
@@ -260,7 +260,7 @@ static void *phdr_to_first_cached_entry(struct smem_partition_header *phdr)
}
static struct smem_private_entry *
-phdr_to_first_private_entry(struct smem_partition_header *phdr)
+phdr_to_first_uncached_entry(struct smem_partition_header *phdr)
{
void *p = phdr;
@@ -268,7 +268,7 @@ phdr_to_first_private_entry(struct smem_partition_header *phdr)
}
static struct smem_private_entry *
-private_entry_next(struct smem_private_entry *e)
+uncached_entry_next(struct smem_private_entry *e)
{
void *p = e;
@@ -276,7 +276,7 @@ private_entry_next(struct smem_private_entry *e)
le32_to_cpu(e->size);
}
-static void *entry_to_item(struct smem_private_entry *e)
+static void *uncached_entry_to_item(struct smem_private_entry *e)
{
void *p = e;
@@ -300,9 +300,9 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem,
void *cached;
phdr = smem->partitions[host];
- hdr = phdr_to_first_private_entry(phdr);
- end = phdr_to_last_private_entry(phdr);
- cached = phdr_to_first_cached_entry(phdr);
+ hdr = phdr_to_first_uncached_entry(phdr);
+ end = phdr_to_last_uncached_entry(phdr);
+ cached = phdr_to_last_cached_entry(phdr);
while (hdr < end) {
if (hdr->canary != SMEM_PRIVATE_CANARY) {
@@ -315,7 +315,7 @@ static int qcom_smem_alloc_private(struct qcom_smem *smem,
if (le16_to_cpu(hdr->item) == item)
return -EEXIST;
- hdr = private_entry_next(hdr);
+ hdr = uncached_entry_next(hdr);
}
/* Check that we don't grow into the cached region */
@@ -460,8 +460,8 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
struct smem_private_entry *e, *end;
phdr = smem->partitions[host];
- e = phdr_to_first_private_entry(phdr);
- end = phdr_to_last_private_entry(phdr);
+ e = phdr_to_first_uncached_entry(phdr);
+ end = phdr_to_last_uncached_entry(phdr);
while (e < end) {
if (e->canary != SMEM_PRIVATE_CANARY) {
@@ -476,10 +476,10 @@ static void *qcom_smem_get_private(struct qcom_smem *smem,
*size = le32_to_cpu(e->size) -
le16_to_cpu(e->padding_data);
- return entry_to_item(e);
+ return uncached_entry_to_item(e);
}
- e = private_entry_next(e);
+ e = uncached_entry_next(e);
}
return ERR_PTR(-ENOENT);
--
2.12.0
On 08/14, Bjorn Andersson wrote:
> In preparation for adding accessors for "cached" entries rename the
> "uncached" accessors to. Also rename "first" cached entry to "last", as
s/to/too? or to "uncached"?
> the cached list grows backwards.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
Otherwise,
Reviewed-by: Stephen Boyd <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On 08/14, Bjorn Andersson wrote:
> On msm8996 cached SMEM items are used for storing the GLINK FIFOs, so
> for items not found in the uncached list we need to also search the
> cased list for these items.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
Reviewed-by: Stephen Boyd <[email protected]>
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
On Fri 15 Sep 12:19 PDT 2017, Stephen Boyd wrote:
> On 08/14, Bjorn Andersson wrote:
> > In preparation for adding accessors for "cached" entries rename the
> > "uncached" accessors to. Also rename "first" cached entry to "last", as
>
> s/to/too? or to "uncached"?
>
Doesn't it make more sense if we ju just make it: rename the "uncached"
accessors? I.e just drop the "to".
> > the cached list grows backwards.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
>
>
> Otherwise,
>
> Reviewed-by: Stephen Boyd <[email protected]>
>
Thanks,
Bjorn
On 09/15, Bjorn Andersson wrote:
> On Fri 15 Sep 12:19 PDT 2017, Stephen Boyd wrote:
>
> > On 08/14, Bjorn Andersson wrote:
> > > In preparation for adding accessors for "cached" entries rename the
> > > "uncached" accessors to. Also rename "first" cached entry to "last", as
> >
> > s/to/too? or to "uncached"?
> >
>
> Doesn't it make more sense if we ju just make it: rename the "uncached"
> accessors? I.e just drop the "to".
>
Yes.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project