Re: [RFC PATCH 11/36] dt-bindings: arm: Add MPAM MSC binding

[James Morse <james.morse@arm.com>]

Hi Rob,

On 11/07/2025 22:43, Rob Herring wrote:
> On Fri, Jul 11, 2025 at 06:36:23PM +0000, James Morse wrote:
>> From: Rob Herring <robh@kernel.org>
>>
>> The binding is designed around the assumption that an MSC will be a
>> sub-block of something else such as a memory controller, cache controller,
>> or IOMMU. However, it's certainly possible a design does not have that
>> association or has a mixture of both, so the binding illustrates how we can
>> support that with RIS child nodes.
>>
>> A key part of MPAM is we need to know about all of the MSCs in the system
>> before it can be enabled. This drives the need for the genericish
>> 'arm,mpam-msc' compatible. Though we can't assume an MSC is accessible
>> until a h/w specific driver potentially enables the h/w.

> Is there any DT based h/w using this? I'm not aware of any.

I'm told there is. I'll let them come out of the wood work to confirm it ...


> I would 
> prefer not merging this until there is. I have little insight whether 
> these genericish compatibles will be sufficient, but I have lots of 
> experience to say they won't be. I would also suspect that if anyone has 
> started using this, they've just extended/modified it however they 
> wanted and no feedback got to me.

Sure, what are you looking for here, Reviewed-by tags from someone with a hardware
platform that is going to ship with DT?


>> diff --git a/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
>> new file mode 100644
>> index 000000000000..9d542ecb1a7d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/arm,mpam-msc.yaml
>> @@ -0,0 +1,227 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/arm/arm,mpam-msc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Arm Memory System Resource Partitioning and Monitoring (MPAM)
>> +
>> +description: |
>> +  The Arm MPAM specification can be found here:
>> +
>> +  https://developer.arm.com/documentation/ddi0598/latest
>> +
>> +maintainers:
>> +  - Rob Herring <robh@kernel.org>
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - const: arm,mpam-msc                   # Further details are discoverable
>> +      - const: arm,mpam-memory-controller-msc
>> +
>> +  reg:
>> +    maxItems: 1
>> +    description: A memory region containing registers as defined in the MPAM
>> +      specification.
>> +
>> +  interrupts:
>> +    minItems: 1
>> +    items:
>> +      - description: error (optional)
>> +      - description: overflow (optional, only for monitoring)
>> +
>> +  interrupt-names:
>> +    oneOf:
>> +      - items:
>> +          - enum: [ error, overflow ]
>> +      - items:
>> +          - const: error
>> +          - const: overflow
>> +
>> +  arm,not-ready-us:
>> +    description: The maximum time in microseconds for monitoring data to be
>> +      accurate after a settings change. For more information, see the
>> +      Not-Ready (NRDY) bit description in the MPAM specification.
>> +
>> +  numa-node-id: true # see NUMA binding
>> +
>> +  '#address-cells':
>> +    const: 1
>> +
>> +  '#size-cells':
>> +    const: 0
>> +
>> +patternProperties:
>> +  '^ris@[0-9a-f]$':
>> +    type: object
>> +    additionalProperties: false
>> +    description: |
> 
> '|' can be dropped.
> 
>> +      RIS nodes for each RIS in an MSC. These nodes are required for each RIS
>> +      implementing known MPAM controls
>> +
>> +    properties:
>> +      compatible:
>> +        enum:
>> +            # Bulk storage for cache
>> +          - arm,mpam-cache
>> +            # Memory bandwidth
>> +          - arm,mpam-memory
>> +
>> +      reg:
>> +        minimum: 0
>> +        maximum: 0xf
>> +
>> +      cpus:
>> +        $ref: '/schemas/types.yaml#/definitions/phandle-array'

> Don't need the type. It's in the core schemas now.

(I assume the type is that '$ref' line)


>> +        description:
>> +          Phandle(s) to the CPU node(s) this RIS belongs to. By default, the parent
>> +          device's affinity is used.
>> +
>> +      arm,mpam-device:
>> +        $ref: '/schemas/types.yaml#/definitions/phandle'
> 
> Don't need quotes. This should be a warning, but no testing happened 
> because the DT list and maintainers weren't CCed.

Yup, its an RFC - I only CC'd the folk that have previously expressed an interest for the
first pass. (git send-email put you on CC!)


>> +        description:
>> +          By default, the MPAM enabled device associated with a RIS is the MSC's
>> +          parent node. It is possible for each RIS to be associated with different
>> +          devices in which case 'arm,mpam-device' should be used.
>> +
>> +    required:
>> +      - compatible
>> +      - reg
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +
>> +dependencies:
>> +  interrupts: [ interrupt-names ]
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    /*
>> +    cpus {
>> +        cpu@0 {
>> +            next-level-cache = <&L2_0>;
>> +        };
>> +        cpu@100 {
>> +            next-level-cache = <&L2_1>;
>> +        };
>> +    };
>> +    */
>> +    L2_0: cache-controller-0 {
>> +        compatible = "cache";
>> +        cache-level = <2>;
>> +        cache-unified;
>> +        next-level-cache = <&L3>;
>> +
>> +    };
>> +
>> +    L2_1: cache-controller-1 {
>> +        compatible = "cache";
>> +        cache-level = <2>;
>> +        cache-unified;
>> +        next-level-cache = <&L3>;
>> +
>> +    };
> 
> All the above should be dropped. Not part of this binding.
> 
>> +
>> +    L3: cache-controller@30000000 {
>> +        compatible = "arm,dsu-l3-cache", "cache";
> 
> Pretty sure this is a warning because that compatible doesn't exist.

Not sure what to do with this. I see plenty of DT in the tree with 'cache', and you've got
'foo,a-memory-controller' below ...


>> +        cache-level = <3>;
>> +        cache-unified;
>> +
>> +        ranges = <0x0 0x30000000 0x800000>;
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        msc@10000 {
>> +            compatible = "arm,mpam-msc";
>> +
>> +            /* CPU affinity implied by parent cache node's  */
>> +            reg = <0x10000 0x2000>;
>> +            interrupts = <1>, <2>;
>> +            interrupt-names = "error", "overflow";
>> +            arm,not-ready-us = <1>;
>> +        };
>> +    };
>> +
>> +    mem: memory-controller@20000 {
>> +        compatible = "foo,a-memory-controller";
>> +        reg = <0x20000 0x1000>;
>> +
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +        ranges;
>> +
>> +        msc@21000 {
>> +            compatible = "arm,mpam-memory-controller-msc", "arm,mpam-msc";
>> +            reg = <0x21000 0x1000>;
>> +            interrupts = <3>;
>> +            interrupt-names = "error";
>> +            arm,not-ready-us = <1>;
>> +            numa-node-id = <1>;
>> +        };
>> +    };


James