2023-05-30 23:52:06

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v2 0/3] soc: qcom: rmtfs: Support dynamic allocation

Some platforms have laxed requirements on the placement of the rmtfs
memory region, introduce support for guard pages to allow the DeviceTree
author to rely on the OS/Linux for placement of the region.

Bjorn Andersson (3):
dt-bindings: reserved-memory: rmtfs: Allow guard pages
soc: qcom: rmtfs: Support discarding guard pages
soc: qcom: rtmfs: Handle reserved-memory allocation issues

.../bindings/reserved-memory/qcom,rmtfs-mem.yaml | 7 +++++++
drivers/soc/qcom/rmtfs_mem.c | 12 +++++++++++-
2 files changed, 18 insertions(+), 1 deletion(-)

--
2.25.1



2023-05-30 23:54:25

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v2 2/3] soc: qcom: rmtfs: Support discarding guard pages

In some configurations, the exact placement of the rmtfs shared memory
region isn't so strict. The DeviceTree author can then choose to use the
"size" property and rely on the OS for placement (in combination with
"alloc-ranges", if desired).

But on some platforms the rmtfs memory region may not be allocated
adjacent to regions allocated by other clients. Add support for
discarding the first and last 4k block in the region, if
qcom,use-guard-pages is specified in DeviceTree.

Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v1:
- Drop the dma_alloc_coherent() based approach and just add support for
the guard pages.

drivers/soc/qcom/rmtfs_mem.c | 10 ++++++++++
1 file changed, 10 insertions(+)

diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
index f83811f51175..28238974d913 100644
--- a/drivers/soc/qcom/rmtfs_mem.c
+++ b/drivers/soc/qcom/rmtfs_mem.c
@@ -213,6 +213,16 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
goto put_device;
}

+ /*
+ * If requested, discard the first and last 4k block in order to ensure
+ * that the rmtfs region isn't adjacent to other protected regions.
+ */
+ if (of_property_present(node, "qcom,use-guard-pages")) {
+ rmtfs_mem->addr += SZ_4K;
+ rmtfs_mem->base += SZ_4K;
+ rmtfs_mem->size -= 2 * SZ_4K;
+ }
+
cdev_init(&rmtfs_mem->cdev, &qcom_rmtfs_mem_fops);
rmtfs_mem->cdev.owner = THIS_MODULE;

--
2.25.1


2023-05-30 23:55:04

by Bjorn Andersson

[permalink] [raw]
Subject: [PATCH v2 3/3] soc: qcom: rtmfs: Handle reserved-memory allocation issues

In the even that Linux failed to allocate the reserved memory range
specified in the DeviceTree, the size of the reserved_mem will be 0,
which results in a oops when memory remapping is attempted.

Detect this and report that the memory region was not found instead.

Signed-off-by: Bjorn Andersson <[email protected]>
---

Changes since v1:
- New patch

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

diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
index 28238974d913..e3a55fa041f9 100644
--- a/drivers/soc/qcom/rmtfs_mem.c
+++ b/drivers/soc/qcom/rmtfs_mem.c
@@ -180,7 +180,7 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
int ret, i;

rmem = of_reserved_mem_lookup(node);
- if (!rmem) {
+ if (!rmem || !rmem->size) {
dev_err(&pdev->dev, "failed to acquire memory region\n");
return -EINVAL;
}
--
2.25.1


2023-05-31 00:33:30

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] soc: qcom: rmtfs: Support discarding guard pages



On 31/05/2023 00:36, Bjorn Andersson wrote:
> In some configurations, the exact placement of the rmtfs shared memory
> region isn't so strict. The DeviceTree author can then choose to use the
> "size" property and rely on the OS for placement (in combination with
> "alloc-ranges", if desired).
>
> But on some platforms the rmtfs memory region may not be allocated
> adjacent to regions allocated by other clients. Add support for
> discarding the first and last 4k block in the region, if
> qcom,use-guard-pages is specified in DeviceTree.

Oh nice!
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> Changes since v1:
> - Drop the dma_alloc_coherent() based approach and just add support for
> the guard pages.
>
> drivers/soc/qcom/rmtfs_mem.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
> index f83811f51175..28238974d913 100644
> --- a/drivers/soc/qcom/rmtfs_mem.c
> +++ b/drivers/soc/qcom/rmtfs_mem.c
> @@ -213,6 +213,16 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
> goto put_device;
> }
>
> + /*
> + * If requested, discard the first and last 4k block in order to ensure
> + * that the rmtfs region isn't adjacent to other protected regions.
> + */
> + if (of_property_present(node, "qcom,use-guard-pages")) {
> + rmtfs_mem->addr += SZ_4K;
> + rmtfs_mem->base += SZ_4K;
> + rmtfs_mem->size -= 2 * SZ_4K;
> + }
> +
> cdev_init(&rmtfs_mem->cdev, &qcom_rmtfs_mem_fops);
> rmtfs_mem->cdev.owner = THIS_MODULE;
>

--
// Caleb (they/them)

2023-05-31 00:34:20

by Caleb Connolly

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] soc: qcom: rmtfs: Support discarding guard pages



On 31/05/2023 01:08, Caleb Connolly wrote:
>
>
> On 31/05/2023 00:36, Bjorn Andersson wrote:
>> In some configurations, the exact placement of the rmtfs shared memory
>> region isn't so strict. The DeviceTree author can then choose to use the
>> "size" property and rely on the OS for placement (in combination with
>> "alloc-ranges", if desired).
>>
>> But on some platforms the rmtfs memory region may not be allocated
>> adjacent to regions allocated by other clients. Add support for
>> discarding the first and last 4k block in the region, if
>> qcom,use-guard-pages is specified in DeviceTree.
>
> Oh nice!
... Bit eager on the enter key there
>>
>> Signed-off-by: Bjorn Andersson <[email protected]>

Reviewed-by: Caleb Connolly <[email protected]>
>> ---
>>
>> Changes since v1:
>> - Drop the dma_alloc_coherent() based approach and just add support for
>> the guard pages.
>>
>> drivers/soc/qcom/rmtfs_mem.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
>> index f83811f51175..28238974d913 100644
>> --- a/drivers/soc/qcom/rmtfs_mem.c
>> +++ b/drivers/soc/qcom/rmtfs_mem.c
>> @@ -213,6 +213,16 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
>> goto put_device;
>> }
>>
>> + /*
>> + * If requested, discard the first and last 4k block in order to ensure
>> + * that the rmtfs region isn't adjacent to other protected regions.
>> + */
>> + if (of_property_present(node, "qcom,use-guard-pages")) {
>> + rmtfs_mem->addr += SZ_4K;
>> + rmtfs_mem->base += SZ_4K;
>> + rmtfs_mem->size -= 2 * SZ_4K;
>> + }
>> +
>> cdev_init(&rmtfs_mem->cdev, &qcom_rmtfs_mem_fops);
>> rmtfs_mem->cdev.owner = THIS_MODULE;
>>
>

--
// Caleb (they/them)

2023-06-01 12:56:58

by Stephan Gerhold

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] soc: qcom: rmtfs: Support discarding guard pages

On Tue, May 30, 2023 at 04:36:42PM -0700, Bjorn Andersson wrote:
> In some configurations, the exact placement of the rmtfs shared memory
> region isn't so strict. The DeviceTree author can then choose to use the
> "size" property and rely on the OS for placement (in combination with
> "alloc-ranges", if desired).
>
> But on some platforms the rmtfs memory region may not be allocated
> adjacent to regions allocated by other clients. Add support for
> discarding the first and last 4k block in the region, if
> qcom,use-guard-pages is specified in DeviceTree.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> Changes since v1:
> - Drop the dma_alloc_coherent() based approach and just add support for
> the guard pages.
>
> drivers/soc/qcom/rmtfs_mem.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
> index f83811f51175..28238974d913 100644
> --- a/drivers/soc/qcom/rmtfs_mem.c
> +++ b/drivers/soc/qcom/rmtfs_mem.c
> @@ -213,6 +213,16 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
> goto put_device;
> }
>
> + /*
> + * If requested, discard the first and last 4k block in order to ensure
> + * that the rmtfs region isn't adjacent to other protected regions.
> + */
> + if (of_property_present(node, "qcom,use-guard-pages")) {
> + rmtfs_mem->addr += SZ_4K;
> + rmtfs_mem->base += SZ_4K;
> + rmtfs_mem->size -= 2 * SZ_4K;
> + }

It probably doesn't make a big difference in practice but I would say
there is no need to even memremap() the guard pages. If you adjust the
->addr and ->size before the memremap() then you don't need to modify
the ->base at all.

Thanks,
Stephan