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

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

[Bjorn Andersson]

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

[PATCH v2 1/3] dt-bindings: reserved-memory: rmtfs: Allow guard pages

[Bjorn Andersson]

On some Qualcomm platforms it's required that the rmtfs memory is not
placed adjacent to allocations performed by other clients. Some
DeviceTree authors have solved this by reserving the space around
the region, but this prevents such author to use rely on the OS to place
the region, through the use of "size" (instead of a fixed location).

So introduce a flag to indicate that guard pages should be carved at the
beginning and end of the memory region. The user shall account for the
two 4k blocks in the defined size.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---

Changes since v1:
- Drop qcom,alloc-size in favour of using reserved-memory/size
- Introduce explicit property to signal that guard pages should be
  carved out from this region (rather than always do it in the dynamic
  case).

 .../bindings/reserved-memory/qcom,rmtfs-mem.yaml           | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
index bab982f00485..26e24e7b08cf 100644
--- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
@@ -26,6 +26,13 @@ properties:
     description: >
       identifier of the client to use this region for buffers
 
+  qcom,use-guard-pages:
+    type: boolean
+    description: >
+      Indicates that the OS should ignore the first and last 4k block of the
+      memory region, for the purpose of ensuring that the allocation is not
+      adjacent to other protected regions.
+
   qcom,vmid:
     $ref: /schemas/types.yaml#/definitions/uint32-array
     description: >
-- 
2.25.1

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

[Bjorn Andersson]

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 <quic_bjorande@quicinc.com>
---

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

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

[Bjorn Andersson]

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 <quic_bjorande@quicinc.com>
---

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

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

[Caleb Connolly]

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 <quic_bjorande@quicinc.com>
> ---
> 
> 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)

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

[Caleb Connolly]

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 <quic_bjorande@quicinc.com>

Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>>
>> 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)

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

[Stephan Gerhold]

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 <quic_bjorande@quicinc.com>
> ---
> 
> 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

Re: [PATCH v2 1/3] dt-bindings: reserved-memory: rmtfs: Allow guard pages

[Stephan Gerhold]

On Tue, May 30, 2023 at 04:36:41PM -0700, Bjorn Andersson wrote:
> On some Qualcomm platforms it's required that the rmtfs memory is not
> placed adjacent to allocations performed by other clients. Some
> DeviceTree authors have solved this by reserving the space around
> the region, but this prevents such author to use rely on the OS to place
> the region, through the use of "size" (instead of a fixed location).
> 
> So introduce a flag to indicate that guard pages should be carved at the
> beginning and end of the memory region. The user shall account for the
> two 4k blocks in the defined size.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> 
> Changes since v1:
> - Drop qcom,alloc-size in favour of using reserved-memory/size
> - Introduce explicit property to signal that guard pages should be
>   carved out from this region (rather than always do it in the dynamic
>   case).
> 
>  .../bindings/reserved-memory/qcom,rmtfs-mem.yaml           | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> index bab982f00485..26e24e7b08cf 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> @@ -26,6 +26,13 @@ properties:
>      description: >
>        identifier of the client to use this region for buffers
>  
> +  qcom,use-guard-pages:
> +    type: boolean
> +    description: >
> +      Indicates that the OS should ignore the first and last 4k block of the
> +      memory region, for the purpose of ensuring that the allocation is not
> +      adjacent to other protected regions.
> +

The name "page(s)" is kind of ambiguous nowadays given that systems are
configured with a variety of page sizes other than 4k.

I wonder if it would be more clear to add the actual size of the guard
pages to the device tree, e.g.

	rmtfs {
		compatible = "qcom,rmtfs-mem";
		size = <(2*1024*1024 + 2*4096)>;
		qcom,guard-size = <4096>;
	};

This could also handle a different padding - in the unlikely case that
this is needed on some platform...

It's probably also fine the way you added it though, this is just
a random thought I had. :)

Thanks,
Stephan

Re: [PATCH v2 1/3] dt-bindings: reserved-memory: rmtfs: Allow guard pages

[Krzysztof Kozlowski]

On 31/05/2023 01:36, Bjorn Andersson wrote:
> On some Qualcomm platforms it's required that the rmtfs memory is not
> placed adjacent to allocations performed by other clients. Some
> DeviceTree authors have solved this by reserving the space around
> the region, but this prevents such author to use rely on the OS to place
> the region, through the use of "size" (instead of a fixed location).
> 
> So introduce a flag to indicate that guard pages should be carved at the
> beginning and end of the memory region. The user shall account for the
> two 4k blocks in the defined size.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
> 
> Changes since v1:
> - Drop qcom,alloc-size in favour of using reserved-memory/size
> - Introduce explicit property to signal that guard pages should be
>   carved out from this region (rather than always do it in the dynamic
>   case).
> 
>  .../bindings/reserved-memory/qcom,rmtfs-mem.yaml           | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> index bab982f00485..26e24e7b08cf 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> @@ -26,6 +26,13 @@ properties:
>      description: >
>        identifier of the client to use this region for buffers
>  
> +  qcom,use-guard-pages:
> +    type: boolean
> +    description: >
> +      Indicates that the OS should ignore the first and last 4k block of the
> +      memory region, for the purpose of ensuring that the allocation is not
> +      adjacent to other protected regions.

Property name and description: do not describe the OS behavior. Describe
the hardware or firmware. Describe the actual problem or characteristic
which will be then interpreted by OS to do what you want. DT is not for
instructing OS what to do.

Best regards,
Krzysztof