diff mbox series

[16/21] dt-bindings: reserved-memory: introduce designated-movable-block

Message ID 20220913195508.3511038-17-opendmb@gmail.com (mailing list archive)
State New
Headers show
Series mm: introduce Designated Movable Blocks | expand

Commit Message

Doug Berger Sept. 13, 2022, 7:55 p.m. UTC
Introduce designated-movable-block.yaml to document the
devicetree binding for Designated Movable Block children of the
reserved-memory node.

Signed-off-by: Doug Berger <opendmb@gmail.com>
---
 .../designated-movable-block.yaml             | 51 +++++++++++++++++++
 1 file changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml

Comments

Rob Herring Sept. 14, 2022, 2:55 p.m. UTC | #1
On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
> Introduce designated-movable-block.yaml to document the
> devicetree binding for Designated Movable Block children of the
> reserved-memory node.

What is a Designated Movable Block? This patch needs to stand on its 
own.

Why does this belong or need to be in DT?

> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
>  .../designated-movable-block.yaml             | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> new file mode 100644
> index 000000000000..42f846069a2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/designated-movable-block.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /reserved-memory Designated Movable Block node binding
> +
> +maintainers:
> +  - devicetree-spec@vger.kernel.org
> +
> +allOf:
> +  - $ref: "reserved-memory.yaml"
> +
> +properties:
> +  compatible:
> +    const: designated-movable-block
> +    description:
> +      This indicates a region of memory meant to be placed into
> +      ZONE_MOVABLE.

Don't put Linuxisms into bindings.

> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reusable
> +
> +examples:
> +  - |
> +      reserved-memory {
> +          #address-cells = <0x2>;
> +          #size-cells = <0x2>;
> +
> +          DMB0@10800000 {
> +               compatible = "designated-movable-block";
> +               reusable;
> +               reg = <0x0 0x10800000 0x0 0x2d800000>;
> +          };
> +
> +          DMB1@40000000 {
> +               compatible = "designated-movable-block";
> +               reusable;
> +               reg = <0x0 0x40000000 0x0 0x30000000>;
> +          };
> +
> +          DMB2@80000000 {
> +               compatible = "designated-movable-block";
> +               reusable;
> +               reg = <0x0 0x80000000 0x0 0x2fc00000>;
> +          };
> +      };
> -- 
> 2.25.1
> 
>
Doug Berger Sept. 14, 2022, 5:13 p.m. UTC | #2
On 9/14/2022 7:55 AM, Rob Herring wrote:
> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>> Introduce designated-movable-block.yaml to document the
>> devicetree binding for Designated Movable Block children of the
>> reserved-memory node.
> 
> What is a Designated Movable Block? This patch needs to stand on its
> own.
As noted in my reply to your [PATCH 00/21] comment, my intention in 
submitting the entire patch set (and specifically PATCH 00/21]) was to 
communicate this context. Now that I believe I understand that only this 
patch should have been submitted to the devicetree-spec mailing list, I 
will strive harder to make it more self contained.

> 
> Why does this belong or need to be in DT?
While my preferred method of declaring Designated Movable Blocks is 
through the movablecore kernel parameter, I can conceive that others may 
wish to take advantage of the reserved-memory DT nodes. In particular, 
it has the advantage that a device can claim ownership of the 
reserved-memory via device tree, which is something that has yet to be 
implemented for DMBs defined with movablecore.

> 
>>
>> Signed-off-by: Doug Berger <opendmb@gmail.com>
>> ---
>>   .../designated-movable-block.yaml             | 51 +++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>> new file mode 100644
>> index 000000000000..42f846069a2e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/reserved-memory/designated-movable-block.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: /reserved-memory Designated Movable Block node binding
>> +
>> +maintainers:
>> +  - devicetree-spec@vger.kernel.org
>> +
>> +allOf:
>> +  - $ref: "reserved-memory.yaml"
>> +
>> +properties:
>> +  compatible:
>> +    const: designated-movable-block
>> +    description:
>> +      This indicates a region of memory meant to be placed into
>> +      ZONE_MOVABLE.
> 
> Don't put Linuxisms into bindings.
I will avoid ZONE_MOVABLE if this commit is included in V2 of this patch 
set.
> 
>> +
>> +unevaluatedProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reusable
>> +
>> +examples:
>> +  - |
>> +      reserved-memory {
>> +          #address-cells = <0x2>;
>> +          #size-cells = <0x2>;
>> +
>> +          DMB0@10800000 {
>> +               compatible = "designated-movable-block";
>> +               reusable;
>> +               reg = <0x0 0x10800000 0x0 0x2d800000>;
>> +          };
>> +
>> +          DMB1@40000000 {
>> +               compatible = "designated-movable-block";
>> +               reusable;
>> +               reg = <0x0 0x40000000 0x0 0x30000000>;
>> +          };
>> +
>> +          DMB2@80000000 {
>> +               compatible = "designated-movable-block";
>> +               reusable;
>> +               reg = <0x0 0x80000000 0x0 0x2fc00000>;
>> +          };
>> +      };
>> -- 
>> 2.25.1
>>
>>
Thank you for the review!
-Doug
Krzysztof Kozlowski Sept. 18, 2022, 10:28 a.m. UTC | #3
On 13/09/2022 20:55, Doug Berger wrote:
> Introduce designated-movable-block.yaml to document the
> devicetree binding for Designated Movable Block children of the
> reserved-memory node.
> 
> Signed-off-by: Doug Berger <opendmb@gmail.com>
> ---
>  .../designated-movable-block.yaml             | 51 +++++++++++++++++++
>  1 file changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> new file mode 100644
> index 000000000000..42f846069a2e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
> @@ -0,0 +1,51 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/designated-movable-block.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: /reserved-memory Designated Movable Block node binding

Drop "binding"

> +
> +maintainers:
> +  - devicetree-spec@vger.kernel.org
> +
> +allOf:
> +  - $ref: "reserved-memory.yaml"

Skip quotes

> +
> +properties:
> +  compatible:
> +    const: designated-movable-block
> +    description:
> +      This indicates a region of memory meant to be placed into
> +      ZONE_MOVABLE.
> +
> +unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reusable
> +
> +examples:
> +  - |
> +      reserved-memory {

Use 4 spaces for example indentation.

> +          #address-cells = <0x2>;
> +          #size-cells = <0x2>;
> +
> +          DMB0@10800000 {

The convention for node names is to use lowercase and generic node
names, so just "dmb".



Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 18, 2022, 10:31 a.m. UTC | #4
On 14/09/2022 18:13, Doug Berger wrote:
> On 9/14/2022 7:55 AM, Rob Herring wrote:
>> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>>> Introduce designated-movable-block.yaml to document the
>>> devicetree binding for Designated Movable Block children of the
>>> reserved-memory node.
>>
>> What is a Designated Movable Block? This patch needs to stand on its
>> own.
> As noted in my reply to your [PATCH 00/21] comment, my intention in 
> submitting the entire patch set (and specifically PATCH 00/21]) was to 
> communicate this context. Now that I believe I understand that only this 
> patch should have been submitted to the devicetree-spec mailing list, I 
> will strive harder to make it more self contained.

The submission of entire thread was ok. What is missing is the
explanation in this commit. This commit must be self-explanatory (e.g.
in explaining "Why are you doing it?"), not rely on other commits for
such explanation.

> 
>>
>> Why does this belong or need to be in DT?
> While my preferred method of declaring Designated Movable Blocks is 
> through the movablecore kernel parameter, I can conceive that others may 
> wish to take advantage of the reserved-memory DT nodes. In particular, 
> it has the advantage that a device can claim ownership of the 
> reserved-memory via device tree, which is something that has yet to be 
> implemented for DMBs defined with movablecore.

Rephrasing the question: why OS memory layout and OS behavior is a
property of hardware (DTS)?



Best regards,
Krzysztof
Doug Berger Sept. 18, 2022, 10:41 p.m. UTC | #5
On 9/18/2022 3:28 AM, Krzysztof Kozlowski wrote:
> On 13/09/2022 20:55, Doug Berger wrote:
>> Introduce designated-movable-block.yaml to document the
>> devicetree binding for Designated Movable Block children of the
>> reserved-memory node.
>>
>> Signed-off-by: Doug Berger <opendmb@gmail.com>
>> ---
>>   .../designated-movable-block.yaml             | 51 +++++++++++++++++++
>>   1 file changed, 51 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>> new file mode 100644
>> index 000000000000..42f846069a2e
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
>> @@ -0,0 +1,51 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/reserved-memory/designated-movable-block.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: /reserved-memory Designated Movable Block node binding
> 
> Drop "binding"
> 
>> +
>> +maintainers:
>> +  - devicetree-spec@vger.kernel.org
>> +
>> +allOf:
>> +  - $ref: "reserved-memory.yaml"
> 
> Skip quotes
> 
>> +
>> +properties:
>> +  compatible:
>> +    const: designated-movable-block
>> +    description:
>> +      This indicates a region of memory meant to be placed into
>> +      ZONE_MOVABLE.
>> +
>> +unevaluatedProperties: false
>> +
>> +required:
>> +  - compatible
>> +  - reusable
>> +
>> +examples:
>> +  - |
>> +      reserved-memory {
> 
> Use 4 spaces for example indentation.
> 
>> +          #address-cells = <0x2>;
>> +          #size-cells = <0x2>;
>> +
>> +          DMB0@10800000 {
> 
> The convention for node names is to use lowercase and generic node
> names, so just "dmb".
> 
> 
> 
> Best regards,
> Krzysztof
Thanks for taking the time to review and provide feedback on this patch.
-Doug
Doug Berger Sept. 18, 2022, 11:12 p.m. UTC | #6
On 9/18/2022 3:31 AM, Krzysztof Kozlowski wrote:
> On 14/09/2022 18:13, Doug Berger wrote:
>> On 9/14/2022 7:55 AM, Rob Herring wrote:
>>> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>>>> Introduce designated-movable-block.yaml to document the
>>>> devicetree binding for Designated Movable Block children of the
>>>> reserved-memory node.
>>>
>>> What is a Designated Movable Block? This patch needs to stand on its
>>> own.
>> As noted in my reply to your [PATCH 00/21] comment, my intention in
>> submitting the entire patch set (and specifically PATCH 00/21]) was to
>> communicate this context. Now that I believe I understand that only this
>> patch should have been submitted to the devicetree-spec mailing list, I
>> will strive harder to make it more self contained.
> 
> The submission of entire thread was ok. What is missing is the
> explanation in this commit. This commit must be self-explanatory (e.g.
> in explaining "Why are you doing it?"), not rely on other commits for
> such explanation.
> 
>>
>>>
>>> Why does this belong or need to be in DT?
>> While my preferred method of declaring Designated Movable Blocks is
>> through the movablecore kernel parameter, I can conceive that others may
>> wish to take advantage of the reserved-memory DT nodes. In particular,
>> it has the advantage that a device can claim ownership of the
>> reserved-memory via device tree, which is something that has yet to be
>> implemented for DMBs defined with movablecore.
> 
> Rephrasing the question: why OS memory layout and OS behavior is a
> property of hardware (DTS)?
I would say the premise is fundamentally the same as the existing 
reserved-memory child node.

I've been rethinking how this should be specified. I am now thinking 
that it may be better to introduce a new Reserved Memory property that 
serves as a modifier to the 'reusable' property. The 'reusable' property 
allows the OS to use memory that has been reserved for a device and 
therefore requires the device driver to reclaim the memory prior to its 
use. However, an OS may have multiple ways of implementing such reuse 
and reclamation.

I am considering introducing the vendor specific 'linux,dmb' property 
that is dependent on the 'reusable' property to allow both the OS and 
the device driver to identify the method used by the Linux OS to support 
reuse and reclamation of the reserved-memory child node.

Such a property would remove any need for new compatible strings to the 
device tree. Does that approach seem reasonable to you?

> 
> Best regards,
> Krzysztof
Thanks again for taking the time,
-Doug
Krzysztof Kozlowski Sept. 19, 2022, 11:03 a.m. UTC | #7
On 19/09/2022 01:12, Doug Berger wrote:
> On 9/18/2022 3:31 AM, Krzysztof Kozlowski wrote:
>> On 14/09/2022 18:13, Doug Berger wrote:
>>> On 9/14/2022 7:55 AM, Rob Herring wrote:
>>>> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>>>>> Introduce designated-movable-block.yaml to document the
>>>>> devicetree binding for Designated Movable Block children of the
>>>>> reserved-memory node.
>>>>
>>>> What is a Designated Movable Block? This patch needs to stand on its
>>>> own.
>>> As noted in my reply to your [PATCH 00/21] comment, my intention in
>>> submitting the entire patch set (and specifically PATCH 00/21]) was to
>>> communicate this context. Now that I believe I understand that only this
>>> patch should have been submitted to the devicetree-spec mailing list, I
>>> will strive harder to make it more self contained.
>>
>> The submission of entire thread was ok. What is missing is the
>> explanation in this commit. This commit must be self-explanatory (e.g.
>> in explaining "Why are you doing it?"), not rely on other commits for
>> such explanation.
>>
>>>
>>>>
>>>> Why does this belong or need to be in DT?
>>> While my preferred method of declaring Designated Movable Blocks is
>>> through the movablecore kernel parameter, I can conceive that others may
>>> wish to take advantage of the reserved-memory DT nodes. In particular,
>>> it has the advantage that a device can claim ownership of the
>>> reserved-memory via device tree, which is something that has yet to be
>>> implemented for DMBs defined with movablecore.
>>
>> Rephrasing the question: why OS memory layout and OS behavior is a
>> property of hardware (DTS)?
> I would say the premise is fundamentally the same as the existing 
> reserved-memory child node.

I don't think it is fundamentally the same.

The existing reserved-memory node describes memory used by hardware - by
other devices. The OS way of handling this memory - movable, reclaimable
etc - is not part of it.

So no, it is not the same.

> 
> I've been rethinking how this should be specified. I am now thinking 
> that it may be better to introduce a new Reserved Memory property that 
> serves as a modifier to the 'reusable' property. The 'reusable' property 
> allows the OS to use memory that has been reserved for a device and 
> therefore requires the device driver to reclaim the memory prior to its 
> use. However, an OS may have multiple ways of implementing such reuse 
> and reclamation.

... and I repeat the question - why OS way of implementing reuse and
reclamation is relevant to DT?

> I am considering introducing the vendor specific 'linux,dmb' property 
> that is dependent on the 'reusable' property to allow both the OS and 
> the device driver to identify the method used by the Linux OS to support 
> reuse and reclamation of the reserved-memory child node.

Sure, but why? Why OS and Linux driver specific pieces should be in DT?
> Such a property would remove any need for new compatible strings to the 
> device tree. Does that approach seem reasonable to you?

No, because you did not explain original question. At all.

Best regards,
Krzysztof
Doug Berger Sept. 21, 2022, 12:14 a.m. UTC | #8
On 9/19/2022 4:03 AM, Krzysztof Kozlowski wrote:
> On 19/09/2022 01:12, Doug Berger wrote:
>> On 9/18/2022 3:31 AM, Krzysztof Kozlowski wrote:
>>> On 14/09/2022 18:13, Doug Berger wrote:
>>>> On 9/14/2022 7:55 AM, Rob Herring wrote:
>>>>> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>>>>>> Introduce designated-movable-block.yaml to document the
>>>>>> devicetree binding for Designated Movable Block children of the
>>>>>> reserved-memory node.
>>>>>
>>>>> What is a Designated Movable Block? This patch needs to stand on its
>>>>> own.
>>>> As noted in my reply to your [PATCH 00/21] comment, my intention in
>>>> submitting the entire patch set (and specifically PATCH 00/21]) was to
>>>> communicate this context. Now that I believe I understand that only this
>>>> patch should have been submitted to the devicetree-spec mailing list, I
>>>> will strive harder to make it more self contained.
>>>
>>> The submission of entire thread was ok. What is missing is the
>>> explanation in this commit. This commit must be self-explanatory (e.g.
>>> in explaining "Why are you doing it?"), not rely on other commits for
>>> such explanation.
>>>
>>>>
>>>>>
>>>>> Why does this belong or need to be in DT?
>>>> While my preferred method of declaring Designated Movable Blocks is
>>>> through the movablecore kernel parameter, I can conceive that others may
>>>> wish to take advantage of the reserved-memory DT nodes. In particular,
>>>> it has the advantage that a device can claim ownership of the
>>>> reserved-memory via device tree, which is something that has yet to be
>>>> implemented for DMBs defined with movablecore.
>>>
>>> Rephrasing the question: why OS memory layout and OS behavior is a
>>> property of hardware (DTS)?
>> I would say the premise is fundamentally the same as the existing
>> reserved-memory child node.
> 
> I don't think it is fundamentally the same.
> 
> The existing reserved-memory node describes memory used by hardware - by
> other devices. The OS way of handling this memory - movable, reclaimable
> etc - is not part of it.
> 
> So no, it is not the same.
> 
>>
>> I've been rethinking how this should be specified. I am now thinking
>> that it may be better to introduce a new Reserved Memory property that
>> serves as a modifier to the 'reusable' property. The 'reusable' property
>> allows the OS to use memory that has been reserved for a device and
>> therefore requires the device driver to reclaim the memory prior to its
>> use. However, an OS may have multiple ways of implementing such reuse
>> and reclamation.
> 
> ... and I repeat the question - why OS way of implementing reuse and
> reclamation is relevant to DT?
> 
>> I am considering introducing the vendor specific 'linux,dmb' property
>> that is dependent on the 'reusable' property to allow both the OS and
>> the device driver to identify the method used by the Linux OS to support
>> reuse and reclamation of the reserved-memory child node.
> 
> Sure, but why? Why OS and Linux driver specific pieces should be in DT?
>> Such a property would remove any need for new compatible strings to the
>> device tree. Does that approach seem reasonable to you?
> 
> No, because you did not explain original question. At all.
I apologize if I have somehow offended you, but please recognize that my 
apparent inability to answer your question does not come from an 
unwillingness to do so.

I believe an example of the reserved-memory node being used the way you 
indicate (though there are other uses) can be expressed with device tree 
nodes like these:

reserved-memory {
	#address-cells = <0x1>;
	#size-cells = <0x1>;
	ranges;

	multimedia_reserved: multimedia@80000000 {
		reg = <0x80000000 0x10000000>;
	};
};

decoder@8012000 {
	memory-region = <&multimedia_reserved>;
	/* ... */
};

Here a 256MB chunk of memory is reserved for use by a hardware decoder 
as part of rendering a video stream. In this case the memory is reserved 
for the exclusive use of the decoder device and its associated device 
driver.

The Devicetree Specification includes a property named 'reusable' that 
could be applied to the multimedia node to allow the OS to "use the 
memory in this region with the limitation that the device driver(s) 
owning the region need to be able to reclaim it back". This is a good 
idea, because this memory could probably be put to good use when the 
decoder is not active. Unfortunately, the methods for reusing this 
memory are not defined for Linux so the multimedia reserved memory would 
not be reused even though the devicetree indicates that it is allowed.

The notion behind this commit was to introduce the 
'designated-movable-block' compatible string that could be added to the 
multimedia node to allow the Client Program (i.e. Linux) to select a 
device driver that knows how to reclaim reserved memory back from the OS 
when it is needed by the decoder device and release it back to the OS 
when the decoder no longer needs it. In this way, the purpose of the 
multimedia node remains the same (i.e. to reserve memory for use by a 
device), but a new compatible string is defined to allow for selection 
of an appropriate device driver and allow successful reuse of the memory 
for the benefit of the system.

 From Rob's feedback it is clear that 'designated-movable-block' is not 
an appropriate name, but maybe 'linux,dmb' might have been. However, it 
would be more flexible if a 'linux,dmb' property could be introduced as 
a modifier to the existing 'reusable' property to provide a general 
mechanism for clarifying how 'reusable' should be supported by the 
Client Software and its device drivers.

Such a property is not directly relevant to hardware, but the devicetree 
is not wholly concerned with hardware. Reserved memory node children 
include support for 'linux,cma-default' and 'linux,dma-default' 
properties that signal behavioral intent to the Linux OS. Some aspects 
of the devicetree (e.g. the /chosen node and 'reusable' property) are 
for the benefit of the Client Program.

> 
> Best regards,
> Krzysztof
I hope this is closer to the answer you seek, but I may simply not 
understand the question being asked,
-Doug
Krzysztof Kozlowski Sept. 21, 2022, 6:35 a.m. UTC | #9
On 21/09/2022 02:14, Doug Berger wrote:
> On 9/19/2022 4:03 AM, Krzysztof Kozlowski wrote:
>> On 19/09/2022 01:12, Doug Berger wrote:
>>> On 9/18/2022 3:31 AM, Krzysztof Kozlowski wrote:
>>>> On 14/09/2022 18:13, Doug Berger wrote:
>>>>> On 9/14/2022 7:55 AM, Rob Herring wrote:
>>>>>> On Tue, Sep 13, 2022 at 12:55:03PM -0700, Doug Berger wrote:
>>>>>>> Introduce designated-movable-block.yaml to document the
>>>>>>> devicetree binding for Designated Movable Block children of the
>>>>>>> reserved-memory node.
>>>>>>
>>>>>> What is a Designated Movable Block? This patch needs to stand on its
>>>>>> own.
>>>>> As noted in my reply to your [PATCH 00/21] comment, my intention in
>>>>> submitting the entire patch set (and specifically PATCH 00/21]) was to
>>>>> communicate this context. Now that I believe I understand that only this
>>>>> patch should have been submitted to the devicetree-spec mailing list, I
>>>>> will strive harder to make it more self contained.
>>>>
>>>> The submission of entire thread was ok. What is missing is the
>>>> explanation in this commit. This commit must be self-explanatory (e.g.
>>>> in explaining "Why are you doing it?"), not rely on other commits for
>>>> such explanation.
>>>>
>>>>>
>>>>>>
>>>>>> Why does this belong or need to be in DT?
>>>>> While my preferred method of declaring Designated Movable Blocks is
>>>>> through the movablecore kernel parameter, I can conceive that others may
>>>>> wish to take advantage of the reserved-memory DT nodes. In particular,
>>>>> it has the advantage that a device can claim ownership of the
>>>>> reserved-memory via device tree, which is something that has yet to be
>>>>> implemented for DMBs defined with movablecore.
>>>>
>>>> Rephrasing the question: why OS memory layout and OS behavior is a
>>>> property of hardware (DTS)?
>>> I would say the premise is fundamentally the same as the existing
>>> reserved-memory child node.
>>
>> I don't think it is fundamentally the same.
>>
>> The existing reserved-memory node describes memory used by hardware - by
>> other devices. The OS way of handling this memory - movable, reclaimable
>> etc - is not part of it.
>>
>> So no, it is not the same.
>>
>>>
>>> I've been rethinking how this should be specified. I am now thinking
>>> that it may be better to introduce a new Reserved Memory property that
>>> serves as a modifier to the 'reusable' property. The 'reusable' property
>>> allows the OS to use memory that has been reserved for a device and
>>> therefore requires the device driver to reclaim the memory prior to its
>>> use. However, an OS may have multiple ways of implementing such reuse
>>> and reclamation.
>>
>> ... and I repeat the question - why OS way of implementing reuse and
>> reclamation is relevant to DT?
>>
>>> I am considering introducing the vendor specific 'linux,dmb' property
>>> that is dependent on the 'reusable' property to allow both the OS and
>>> the device driver to identify the method used by the Linux OS to support
>>> reuse and reclamation of the reserved-memory child node.
>>
>> Sure, but why? Why OS and Linux driver specific pieces should be in DT?
>>> Such a property would remove any need for new compatible strings to the
>>> device tree. Does that approach seem reasonable to you?
>>
>> No, because you did not explain original question. At all.
> I apologize if I have somehow offended you, but please recognize that my 
> apparent inability to answer your question does not come from an 
> unwillingness to do so.
> 
> I believe an example of the reserved-memory node being used the way you 
> indicate (though there are other uses) can be expressed with device tree 
> nodes like these:
> 
> reserved-memory {
> 	#address-cells = <0x1>;
> 	#size-cells = <0x1>;
> 	ranges;
> 
> 	multimedia_reserved: multimedia@80000000 {
> 		reg = <0x80000000 0x10000000>;
> 	};
> };
> 
> decoder@8012000 {
> 	memory-region = <&multimedia_reserved>;
> 	/* ... */
> };
> 
> Here a 256MB chunk of memory is reserved for use by a hardware decoder 
> as part of rendering a video stream. In this case the memory is reserved 
> for the exclusive use of the decoder device and its associated device 
> driver.
> 
> The Devicetree Specification includes a property named 'reusable' that 
> could be applied to the multimedia node to allow the OS to "use the 
> memory in this region with the limitation that the device driver(s) 
> owning the region need to be able to reclaim it back". 

Indeed, there is such.... and should be used instead. :)

> This is a good 
> idea, because this memory could probably be put to good use when the 
> decoder is not active. Unfortunately, the methods for reusing this 
> memory are not defined for Linux so the multimedia reserved memory would 
> not be reused even though the devicetree indicates that it is allowed.

Then rather implementation has to be changed, not Devicetree bindings.

> 
> The notion behind this commit was to introduce the 
> 'designated-movable-block' compatible string that could be added to the 
> multimedia node to allow the Client Program (i.e. Linux) to select a 
> device driver that knows how to reclaim reserved memory back from the OS 
> when it is needed by the decoder device and release it back to the OS 
> when the decoder no longer needs it. In this way, the purpose of the 
> multimedia node remains the same (i.e. to reserve memory for use by a 
> device), but a new compatible string is defined to allow for selection 
> of an appropriate device driver and allow successful reuse of the memory 
> for the benefit of the system.

We don't need a new compatible for it but use that existing property.

> 
>  From Rob's feedback it is clear that 'designated-movable-block' is not 
> an appropriate name, but maybe 'linux,dmb' might have been. However, it 
> would be more flexible if a 'linux,dmb' property could be introduced as 
> a modifier to the existing 'reusable' property to provide a general 
> mechanism for clarifying how 'reusable' should be supported by the 
> Client Software and its device drivers.
> 
> Such a property is not directly relevant to hardware, but the devicetree 
> is not wholly concerned with hardware. Reserved memory node children 
> include support for 'linux,cma-default' and 'linux,dma-default' 
> properties that signal behavioral intent to the Linux OS. Some aspects 
> of the devicetree (e.g. the /chosen node and 'reusable' property) are 
> for the benefit of the Client Program.

Fair enough, although there is difference between generic property for
reusable/reclaimable memory and a property describing one of Linux
memory-management zones.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
new file mode 100644
index 000000000000..42f846069a2e
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/designated-movable-block.yaml
@@ -0,0 +1,51 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/designated-movable-block.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: /reserved-memory Designated Movable Block node binding
+
+maintainers:
+  - devicetree-spec@vger.kernel.org
+
+allOf:
+  - $ref: "reserved-memory.yaml"
+
+properties:
+  compatible:
+    const: designated-movable-block
+    description:
+      This indicates a region of memory meant to be placed into
+      ZONE_MOVABLE.
+
+unevaluatedProperties: false
+
+required:
+  - compatible
+  - reusable
+
+examples:
+  - |
+      reserved-memory {
+          #address-cells = <0x2>;
+          #size-cells = <0x2>;
+
+          DMB0@10800000 {
+               compatible = "designated-movable-block";
+               reusable;
+               reg = <0x0 0x10800000 0x0 0x2d800000>;
+          };
+
+          DMB1@40000000 {
+               compatible = "designated-movable-block";
+               reusable;
+               reg = <0x0 0x40000000 0x0 0x30000000>;
+          };
+
+          DMB2@80000000 {
+               compatible = "designated-movable-block";
+               reusable;
+               reg = <0x0 0x80000000 0x0 0x2fc00000>;
+          };
+      };