All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-21 14:15 ` Conor Dooley
  0 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-21 14:15 UTC (permalink / raw)
  To: conor, robh+dt, krzysztof.kozlowski+dt, palmer, atishp
  Cc: Conor Dooley, devicetree, linux-riscv, linux-kernel, apatel

From: Conor Dooley <conor.dooley@microchip.com>

The SBI PMU extension requires a firmware to be aware of the event to
counter/mhpmevent mappings supported by the hardware. OpenSBI may use
DeviceTree to describe the PMU mappings. This binding is currently
described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
since v7.2.0.

Import the binding for use while validating dtb dumps from QEMU and
upcoming hardware (eg JH7110 SoC) that will make use of the event
mapping.

Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
Co-developed-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
I asked Rob on IRC about these bindings a few weeks ago & he said he
would be willing to take them. I have modified wording slightly in the
descriptions, but have mostly left things as close to the OpenSBI
documentation as possible.

I'm not super sure about what I've done with the properties being
correct type wise, I went digging in bindings and am sorta using the
first thing that "fit".

Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
is BSD-2-Clause licensed so I am also unsure as to what license I can
use for this binding since that's where I took it from.
---
 .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
 1 file changed, 158 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml

diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
new file mode 100644
index 000000000000..d65f937680af
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V SBI PMU events
+
+maintainers:
+  - Atish Patra <atishp@rivosinc.com>
+
+description: |
+  SBI PMU extension supports allow supervisor software to configure, start &
+  stop any performance counter at anytime. Thus, a user can leverage full
+  capability of performance analysis tools such as perf if the SBI PMU
+  extension is enabled. The OpenSBI implementation makes the following
+  assumptions about the hardware platform:
+    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
+    PMU extension will not be enabled.
+
+    The platform must provide information about PMU event to counter mapping
+    via device tree or platform specific hooks. Otherwise, the SBI PMU
+    extension will not be enabled.
+
+    The platforms should provide information about the PMU event selector
+    values that should be encoded in the expected value of MHPMEVENTx while
+    configuring MHPMCOUNTERx for that specific event. This can be done via a
+    device tree or platform specific hooks. The exact value to be written to
+    the MHPMEVENTx is completely dependent on the platform. The generic
+    platform writes the zero-extended event_idx as the expected value for
+    hardware cache/generic events as suggested by the SBI specification.
+
+properties:
+  compatible:
+    const: riscv,pmu
+
+  riscv,event-to-mhpmevent:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents an ONE-to-ONE mapping between a PMU event and the event
+      selector value that platform expects to be written to the MHPMEVENTx CSR
+      for that event.
+      The mapping is encoded in an array format where each row represents an
+      event. The first element represents the event idx while the second &
+      third elements represent the event selector value that should be encoded
+      in the expected value to be written in MHPMEVENTx.
+      This property shouldn't encode any raw hardware event.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 3
+
+  riscv,event-to-mhpmcounters:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents a MANY-to-MANY mapping between a range of events and all the
+      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
+      of events. The information is encoded in an array format where each
+      row represents a certain range of events and corresponding counters.
+      The first element represents starting of the pmu event id and 2nd column
+      represents the end of the pmu event id. The third element represent a
+      bitmap of all the MHPMCOUNTERx.
+      This property is mandatory if event-to-mhpmevent is present. Otherwise,
+      it can be omitted.
+      This property shouldn't encode any raw event.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 3
+
+  riscv,raw-event-to-mhpmcounters:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
+      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
+      that raw event.
+      The encoding of the raw events are platform specific. The information is
+      encoded in an array format where each row represents the specific raw
+      event(s). The first element is a 64-bit match value where the invariant
+      bits of range of events are set. The second element is a 64-bit mask that
+      will have all the variant bits of the range of events cleared. All other
+      bits should be set in the mask. The third element is a 32-bit value to
+      represent bitmap of all MHPMCOUNTERx that can monitor these set of
+      event(s). If a platform directly encodes each raw PMU event as a unique
+      ID, the value of select_mask must be 0xffffffff_ffffffff.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 5
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    pmu {
+        compatible = "riscv,pmu";
+        riscv,event-to-mhpmevent = <0x0000B 0x0000 0x0001>;
+        riscv,event-to-mhpmcounters = <0x00001 0x00001 0x00000001>,
+                                      <0x00002 0x00002 0x00000004>,
+                                      <0x00003 0x0000A 0x00000ff8>,
+                                      <0x10000 0x10033 0x000ff000>;
+        riscv,raw-event-to-mhpmcounters =
+            /* For event ID 0x0002 */
+            <0x0000 0x0002 0xffffffff 0xffffffff 0x00000f8>,
+            /* For event ID 0-4 */
+            <0x0 0x0 0xffffffff 0xfffffff0 0x00000ff0>,
+            /* For event ID 0xffffffff0000000f - 0xffffffff000000ff */
+            <0xffffffff 0x0 0xffffffff 0xffffff0f 0x00000ff0>;
+    };
+
+  - |
+    /*
+     * For HiFive Unmatched board the encodings can be found here
+     * https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf
+     * This example also binds standard SBI PMU hardware id's to U74 PMU event
+     * codes, U74 uses a bitfield for events encoding, so several U74 events
+     * can be bound to single perf id.
+     * See SBI PMU hardware id's in OpenSBI's include/sbi/sbi_ecall_interface.h
+     */
+    pmu {
+          compatible = "riscv,pmu";
+          riscv,event-to-mhpmevent =
+              /* SBI_PMU_HW_CACHE_REFERENCES -> Instruction or Data cache/ITIM busy */
+              <0x00003 0x00000000 0x1801>,
+              /* SBI_PMU_HW_CACHE_MISSES -> Instruction or Data cache miss or MMIO access */
+              <0x00004 0x00000000 0x0302>,
+              /* SBI_PMU_HW_BRANCH_INSTRUCTIONS -> Conditional branch retired */
+              <0x00005 0x00000000 0x4000>,
+              /* SBI_PMU_HW_BRANCH_MISSES -> Branch or jump misprediction */
+              <0x00006 0x00000000 0x6001>,
+              /* L1D_READ_MISS -> Data cache miss or MMIO access */
+              <0x10001 0x00000000 0x0202>,
+              /* L1D_WRITE_ACCESS -> Data cache write-back */
+              <0x10002 0x00000000 0x0402>,
+              /* L1I_READ_ACCESS -> Instruction cache miss */
+              <0x10009 0x00000000 0x0102>,
+              /* LL_READ_MISS -> UTLB miss */
+              <0x10011 0x00000000 0x2002>,
+              /* DTLB_READ_MISS -> Data TLB miss */
+              <0x10019 0x00000000 0x1002>,
+              /* ITLB_READ_MISS-> Instruction TLB miss */
+              <0x10021 0x00000000 0x0802>;
+          riscv,event-to-mhpmcounters = <0x00003 0x00006 0x18>,
+                                        <0x10001 0x10002 0x18>,
+                                        <0x10009 0x10009 0x18>,
+                                        <0x10011 0x10011 0x18>,
+                                        <0x10019 0x10019 0x18>,
+                                        <0x10021 0x10021 0x18>;
+          riscv,raw-event-to-mhpmcounters = <0x0 0x0 0xffffffff 0xfc0000ff 0x18>,
+                                            <0x0 0x1 0xffffffff 0xfff800ff 0x18>,
+                                            <0x0 0x2 0xffffffff 0xffffe0ff 0x18>;
+    };
-- 
2.38.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-21 14:15 ` Conor Dooley
  0 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-21 14:15 UTC (permalink / raw)
  To: conor, robh+dt, krzysztof.kozlowski+dt, palmer, atishp
  Cc: Conor Dooley, devicetree, linux-riscv, linux-kernel, apatel

From: Conor Dooley <conor.dooley@microchip.com>

The SBI PMU extension requires a firmware to be aware of the event to
counter/mhpmevent mappings supported by the hardware. OpenSBI may use
DeviceTree to describe the PMU mappings. This binding is currently
described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
since v7.2.0.

Import the binding for use while validating dtb dumps from QEMU and
upcoming hardware (eg JH7110 SoC) that will make use of the event
mapping.

Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
Co-developed-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Atish Patra <atishp@rivosinc.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
I asked Rob on IRC about these bindings a few weeks ago & he said he
would be willing to take them. I have modified wording slightly in the
descriptions, but have mostly left things as close to the OpenSBI
documentation as possible.

I'm not super sure about what I've done with the properties being
correct type wise, I went digging in bindings and am sorta using the
first thing that "fit".

Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
is BSD-2-Clause licensed so I am also unsure as to what license I can
use for this binding since that's where I took it from.
---
 .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
 1 file changed, 158 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml

diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
new file mode 100644
index 000000000000..d65f937680af
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
@@ -0,0 +1,158 @@
+# SPDX-License-Identifier: BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RISC-V SBI PMU events
+
+maintainers:
+  - Atish Patra <atishp@rivosinc.com>
+
+description: |
+  SBI PMU extension supports allow supervisor software to configure, start &
+  stop any performance counter at anytime. Thus, a user can leverage full
+  capability of performance analysis tools such as perf if the SBI PMU
+  extension is enabled. The OpenSBI implementation makes the following
+  assumptions about the hardware platform:
+    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
+    PMU extension will not be enabled.
+
+    The platform must provide information about PMU event to counter mapping
+    via device tree or platform specific hooks. Otherwise, the SBI PMU
+    extension will not be enabled.
+
+    The platforms should provide information about the PMU event selector
+    values that should be encoded in the expected value of MHPMEVENTx while
+    configuring MHPMCOUNTERx for that specific event. This can be done via a
+    device tree or platform specific hooks. The exact value to be written to
+    the MHPMEVENTx is completely dependent on the platform. The generic
+    platform writes the zero-extended event_idx as the expected value for
+    hardware cache/generic events as suggested by the SBI specification.
+
+properties:
+  compatible:
+    const: riscv,pmu
+
+  riscv,event-to-mhpmevent:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents an ONE-to-ONE mapping between a PMU event and the event
+      selector value that platform expects to be written to the MHPMEVENTx CSR
+      for that event.
+      The mapping is encoded in an array format where each row represents an
+      event. The first element represents the event idx while the second &
+      third elements represent the event selector value that should be encoded
+      in the expected value to be written in MHPMEVENTx.
+      This property shouldn't encode any raw hardware event.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 3
+
+  riscv,event-to-mhpmcounters:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents a MANY-to-MANY mapping between a range of events and all the
+      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
+      of events. The information is encoded in an array format where each
+      row represents a certain range of events and corresponding counters.
+      The first element represents starting of the pmu event id and 2nd column
+      represents the end of the pmu event id. The third element represent a
+      bitmap of all the MHPMCOUNTERx.
+      This property is mandatory if event-to-mhpmevent is present. Otherwise,
+      it can be omitted.
+      This property shouldn't encode any raw event.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 3
+
+  riscv,raw-event-to-mhpmcounters:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description:
+      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
+      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
+      that raw event.
+      The encoding of the raw events are platform specific. The information is
+      encoded in an array format where each row represents the specific raw
+      event(s). The first element is a 64-bit match value where the invariant
+      bits of range of events are set. The second element is a 64-bit mask that
+      will have all the variant bits of the range of events cleared. All other
+      bits should be set in the mask. The third element is a 32-bit value to
+      represent bitmap of all MHPMCOUNTERx that can monitor these set of
+      event(s). If a platform directly encodes each raw PMU event as a unique
+      ID, the value of select_mask must be 0xffffffff_ffffffff.
+    minItems: 1
+    maxItems: 255
+    items:
+      $ref: /schemas/types.yaml#/definitions/uint32-array
+      maxItems: 5
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    pmu {
+        compatible = "riscv,pmu";
+        riscv,event-to-mhpmevent = <0x0000B 0x0000 0x0001>;
+        riscv,event-to-mhpmcounters = <0x00001 0x00001 0x00000001>,
+                                      <0x00002 0x00002 0x00000004>,
+                                      <0x00003 0x0000A 0x00000ff8>,
+                                      <0x10000 0x10033 0x000ff000>;
+        riscv,raw-event-to-mhpmcounters =
+            /* For event ID 0x0002 */
+            <0x0000 0x0002 0xffffffff 0xffffffff 0x00000f8>,
+            /* For event ID 0-4 */
+            <0x0 0x0 0xffffffff 0xfffffff0 0x00000ff0>,
+            /* For event ID 0xffffffff0000000f - 0xffffffff000000ff */
+            <0xffffffff 0x0 0xffffffff 0xffffff0f 0x00000ff0>;
+    };
+
+  - |
+    /*
+     * For HiFive Unmatched board the encodings can be found here
+     * https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf
+     * This example also binds standard SBI PMU hardware id's to U74 PMU event
+     * codes, U74 uses a bitfield for events encoding, so several U74 events
+     * can be bound to single perf id.
+     * See SBI PMU hardware id's in OpenSBI's include/sbi/sbi_ecall_interface.h
+     */
+    pmu {
+          compatible = "riscv,pmu";
+          riscv,event-to-mhpmevent =
+              /* SBI_PMU_HW_CACHE_REFERENCES -> Instruction or Data cache/ITIM busy */
+              <0x00003 0x00000000 0x1801>,
+              /* SBI_PMU_HW_CACHE_MISSES -> Instruction or Data cache miss or MMIO access */
+              <0x00004 0x00000000 0x0302>,
+              /* SBI_PMU_HW_BRANCH_INSTRUCTIONS -> Conditional branch retired */
+              <0x00005 0x00000000 0x4000>,
+              /* SBI_PMU_HW_BRANCH_MISSES -> Branch or jump misprediction */
+              <0x00006 0x00000000 0x6001>,
+              /* L1D_READ_MISS -> Data cache miss or MMIO access */
+              <0x10001 0x00000000 0x0202>,
+              /* L1D_WRITE_ACCESS -> Data cache write-back */
+              <0x10002 0x00000000 0x0402>,
+              /* L1I_READ_ACCESS -> Instruction cache miss */
+              <0x10009 0x00000000 0x0102>,
+              /* LL_READ_MISS -> UTLB miss */
+              <0x10011 0x00000000 0x2002>,
+              /* DTLB_READ_MISS -> Data TLB miss */
+              <0x10019 0x00000000 0x1002>,
+              /* ITLB_READ_MISS-> Instruction TLB miss */
+              <0x10021 0x00000000 0x0802>;
+          riscv,event-to-mhpmcounters = <0x00003 0x00006 0x18>,
+                                        <0x10001 0x10002 0x18>,
+                                        <0x10009 0x10009 0x18>,
+                                        <0x10011 0x10011 0x18>,
+                                        <0x10019 0x10019 0x18>,
+                                        <0x10021 0x10021 0x18>;
+          riscv,raw-event-to-mhpmcounters = <0x0 0x0 0xffffffff 0xfc0000ff 0x18>,
+                                            <0x0 0x1 0xffffffff 0xfff800ff 0x18>,
+                                            <0x0 0x2 0xffffffff 0xffffe0ff 0x18>;
+    };
-- 
2.38.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
  2022-12-21 14:15 ` Conor Dooley
@ 2022-12-22 18:06   ` Rob Herring
  -1 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2022-12-22 18:06 UTC (permalink / raw)
  To: Conor Dooley
  Cc: krzysztof.kozlowski+dt, palmer, atishp, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, apatel

On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The SBI PMU extension requires a firmware to be aware of the event to
> counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> DeviceTree to describe the PMU mappings. This binding is currently
> described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> since v7.2.0.
> 
> Import the binding for use while validating dtb dumps from QEMU and
> upcoming hardware (eg JH7110 SoC) that will make use of the event
> mapping.
> 
> Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> Co-developed-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> I asked Rob on IRC about these bindings a few weeks ago & he said he
> would be willing to take them. I have modified wording slightly in the
> descriptions, but have mostly left things as close to the OpenSBI
> documentation as possible.

Please CC the perf maintainers. Might be crickets, but so they at least 
have a chance to see it.

> 
> I'm not super sure about what I've done with the properties being
> correct type wise, I went digging in bindings and am sorta using the
> first thing that "fit".
> 
> Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> is BSD-2-Clause licensed so I am also unsure as to what license I can
> use for this binding since that's where I took it from.
> ---
>  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
>  1 file changed, 158 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> new file mode 100644
> index 000000000000..d65f937680af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> @@ -0,0 +1,158 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V SBI PMU events
> +
> +maintainers:
> +  - Atish Patra <atishp@rivosinc.com>
> +
> +description: |
> +  SBI PMU extension supports allow supervisor software to configure, start &
> +  stop any performance counter at anytime. Thus, a user can leverage full
> +  capability of performance analysis tools such as perf if the SBI PMU
> +  extension is enabled. The OpenSBI implementation makes the following
> +  assumptions about the hardware platform:
> +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> +    PMU extension will not be enabled.
> +
> +    The platform must provide information about PMU event to counter mapping
> +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> +    extension will not be enabled.
> +
> +    The platforms should provide information about the PMU event selector
> +    values that should be encoded in the expected value of MHPMEVENTx while
> +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> +    device tree or platform specific hooks. The exact value to be written to
> +    the MHPMEVENTx is completely dependent on the platform. The generic
> +    platform writes the zero-extended event_idx as the expected value for
> +    hardware cache/generic events as suggested by the SBI specification.
> +
> +properties:
> +  compatible:
> +    const: riscv,pmu
> +
> +  riscv,event-to-mhpmevent:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents an ONE-to-ONE mapping between a PMU event and the event
> +      selector value that platform expects to be written to the MHPMEVENTx CSR
> +      for that event.
> +      The mapping is encoded in an array format where each row represents an

s/array/matrix/

> +      event. The first element represents the event idx while the second &
> +      third elements represent the event selector value that should be encoded
> +      in the expected value to be written in MHPMEVENTx.
> +      This property shouldn't encode any raw hardware event.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array

Huh? A property can only have 1 type. I wonder what the tools do with 
this...

> +      maxItems: 3

Better to do:

         items:
           - description: what's in the 1st word
           - description: what's in the 2nd word
           - description: what's in the 3rd word

And rework the overall description to not say the same thing.

> +
> +  riscv,event-to-mhpmcounters:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents a MANY-to-MANY mapping between a range of events and all the
> +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> +      of events. The information is encoded in an array format where each
> +      row represents a certain range of events and corresponding counters.
> +      The first element represents starting of the pmu event id and 2nd column
> +      represents the end of the pmu event id. The third element represent a
> +      bitmap of all the MHPMCOUNTERx.
> +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> +      it can be omitted.

No need to state this in freeform text, use 'dependencies'.

> +      This property shouldn't encode any raw event.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array
> +      maxItems: 3
> +
> +  riscv,raw-event-to-mhpmcounters:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> +      that raw event.
> +      The encoding of the raw events are platform specific. The information is
> +      encoded in an array format where each row represents the specific raw
> +      event(s). The first element is a 64-bit match value where the invariant
> +      bits of range of events are set. The second element is a 64-bit mask that
> +      will have all the variant bits of the range of events cleared. All other
> +      bits should be set in the mask. The third element is a 32-bit value to
> +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> +      event(s). If a platform directly encodes each raw PMU event as a unique
> +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array
> +      maxItems: 5
> +
> +required:
> +  - compatible

I assume at least one of the other properties must be present?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pmu {
> +        compatible = "riscv,pmu";
> +        riscv,event-to-mhpmevent = <0x0000B 0x0000 0x0001>;
> +        riscv,event-to-mhpmcounters = <0x00001 0x00001 0x00000001>,
> +                                      <0x00002 0x00002 0x00000004>,
> +                                      <0x00003 0x0000A 0x00000ff8>,
> +                                      <0x10000 0x10033 0x000ff000>;
> +        riscv,raw-event-to-mhpmcounters =
> +            /* For event ID 0x0002 */
> +            <0x0000 0x0002 0xffffffff 0xffffffff 0x00000f8>,
> +            /* For event ID 0-4 */
> +            <0x0 0x0 0xffffffff 0xfffffff0 0x00000ff0>,
> +            /* For event ID 0xffffffff0000000f - 0xffffffff000000ff */
> +            <0xffffffff 0x0 0xffffffff 0xffffff0f 0x00000ff0>;
> +    };
> +
> +  - |
> +    /*
> +     * For HiFive Unmatched board the encodings can be found here
> +     * https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf
> +     * This example also binds standard SBI PMU hardware id's to U74 PMU event
> +     * codes, U74 uses a bitfield for events encoding, so several U74 events
> +     * can be bound to single perf id.
> +     * See SBI PMU hardware id's in OpenSBI's include/sbi/sbi_ecall_interface.h
> +     */
> +    pmu {
> +          compatible = "riscv,pmu";
> +          riscv,event-to-mhpmevent =
> +              /* SBI_PMU_HW_CACHE_REFERENCES -> Instruction or Data cache/ITIM busy */
> +              <0x00003 0x00000000 0x1801>,
> +              /* SBI_PMU_HW_CACHE_MISSES -> Instruction or Data cache miss or MMIO access */
> +              <0x00004 0x00000000 0x0302>,
> +              /* SBI_PMU_HW_BRANCH_INSTRUCTIONS -> Conditional branch retired */
> +              <0x00005 0x00000000 0x4000>,
> +              /* SBI_PMU_HW_BRANCH_MISSES -> Branch or jump misprediction */
> +              <0x00006 0x00000000 0x6001>,
> +              /* L1D_READ_MISS -> Data cache miss or MMIO access */
> +              <0x10001 0x00000000 0x0202>,
> +              /* L1D_WRITE_ACCESS -> Data cache write-back */
> +              <0x10002 0x00000000 0x0402>,
> +              /* L1I_READ_ACCESS -> Instruction cache miss */
> +              <0x10009 0x00000000 0x0102>,
> +              /* LL_READ_MISS -> UTLB miss */
> +              <0x10011 0x00000000 0x2002>,
> +              /* DTLB_READ_MISS -> Data TLB miss */
> +              <0x10019 0x00000000 0x1002>,
> +              /* ITLB_READ_MISS-> Instruction TLB miss */
> +              <0x10021 0x00000000 0x0802>;
> +          riscv,event-to-mhpmcounters = <0x00003 0x00006 0x18>,
> +                                        <0x10001 0x10002 0x18>,
> +                                        <0x10009 0x10009 0x18>,
> +                                        <0x10011 0x10011 0x18>,
> +                                        <0x10019 0x10019 0x18>,
> +                                        <0x10021 0x10021 0x18>;
> +          riscv,raw-event-to-mhpmcounters = <0x0 0x0 0xffffffff 0xfc0000ff 0x18>,
> +                                            <0x0 0x1 0xffffffff 0xfff800ff 0x18>,
> +                                            <0x0 0x2 0xffffffff 0xffffe0ff 0x18>;
> +    };
> -- 
> 2.38.1
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-22 18:06   ` Rob Herring
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring @ 2022-12-22 18:06 UTC (permalink / raw)
  To: Conor Dooley
  Cc: krzysztof.kozlowski+dt, palmer, atishp, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, apatel

On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> The SBI PMU extension requires a firmware to be aware of the event to
> counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> DeviceTree to describe the PMU mappings. This binding is currently
> described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> since v7.2.0.
> 
> Import the binding for use while validating dtb dumps from QEMU and
> upcoming hardware (eg JH7110 SoC) that will make use of the event
> mapping.
> 
> Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> Co-developed-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> I asked Rob on IRC about these bindings a few weeks ago & he said he
> would be willing to take them. I have modified wording slightly in the
> descriptions, but have mostly left things as close to the OpenSBI
> documentation as possible.

Please CC the perf maintainers. Might be crickets, but so they at least 
have a chance to see it.

> 
> I'm not super sure about what I've done with the properties being
> correct type wise, I went digging in bindings and am sorta using the
> first thing that "fit".
> 
> Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> is BSD-2-Clause licensed so I am also unsure as to what license I can
> use for this binding since that's where I took it from.
> ---
>  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
>  1 file changed, 158 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> 
> diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> new file mode 100644
> index 000000000000..d65f937680af
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> @@ -0,0 +1,158 @@
> +# SPDX-License-Identifier: BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RISC-V SBI PMU events
> +
> +maintainers:
> +  - Atish Patra <atishp@rivosinc.com>
> +
> +description: |
> +  SBI PMU extension supports allow supervisor software to configure, start &
> +  stop any performance counter at anytime. Thus, a user can leverage full
> +  capability of performance analysis tools such as perf if the SBI PMU
> +  extension is enabled. The OpenSBI implementation makes the following
> +  assumptions about the hardware platform:
> +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> +    PMU extension will not be enabled.
> +
> +    The platform must provide information about PMU event to counter mapping
> +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> +    extension will not be enabled.
> +
> +    The platforms should provide information about the PMU event selector
> +    values that should be encoded in the expected value of MHPMEVENTx while
> +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> +    device tree or platform specific hooks. The exact value to be written to
> +    the MHPMEVENTx is completely dependent on the platform. The generic
> +    platform writes the zero-extended event_idx as the expected value for
> +    hardware cache/generic events as suggested by the SBI specification.
> +
> +properties:
> +  compatible:
> +    const: riscv,pmu
> +
> +  riscv,event-to-mhpmevent:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents an ONE-to-ONE mapping between a PMU event and the event
> +      selector value that platform expects to be written to the MHPMEVENTx CSR
> +      for that event.
> +      The mapping is encoded in an array format where each row represents an

s/array/matrix/

> +      event. The first element represents the event idx while the second &
> +      third elements represent the event selector value that should be encoded
> +      in the expected value to be written in MHPMEVENTx.
> +      This property shouldn't encode any raw hardware event.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array

Huh? A property can only have 1 type. I wonder what the tools do with 
this...

> +      maxItems: 3

Better to do:

         items:
           - description: what's in the 1st word
           - description: what's in the 2nd word
           - description: what's in the 3rd word

And rework the overall description to not say the same thing.

> +
> +  riscv,event-to-mhpmcounters:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents a MANY-to-MANY mapping between a range of events and all the
> +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> +      of events. The information is encoded in an array format where each
> +      row represents a certain range of events and corresponding counters.
> +      The first element represents starting of the pmu event id and 2nd column
> +      represents the end of the pmu event id. The third element represent a
> +      bitmap of all the MHPMCOUNTERx.
> +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> +      it can be omitted.

No need to state this in freeform text, use 'dependencies'.

> +      This property shouldn't encode any raw event.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array
> +      maxItems: 3
> +
> +  riscv,raw-event-to-mhpmcounters:
> +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> +    description:
> +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> +      that raw event.
> +      The encoding of the raw events are platform specific. The information is
> +      encoded in an array format where each row represents the specific raw
> +      event(s). The first element is a 64-bit match value where the invariant
> +      bits of range of events are set. The second element is a 64-bit mask that
> +      will have all the variant bits of the range of events cleared. All other
> +      bits should be set in the mask. The third element is a 32-bit value to
> +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> +      event(s). If a platform directly encodes each raw PMU event as a unique
> +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> +    minItems: 1
> +    maxItems: 255
> +    items:
> +      $ref: /schemas/types.yaml#/definitions/uint32-array
> +      maxItems: 5
> +
> +required:
> +  - compatible

I assume at least one of the other properties must be present?

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pmu {
> +        compatible = "riscv,pmu";
> +        riscv,event-to-mhpmevent = <0x0000B 0x0000 0x0001>;
> +        riscv,event-to-mhpmcounters = <0x00001 0x00001 0x00000001>,
> +                                      <0x00002 0x00002 0x00000004>,
> +                                      <0x00003 0x0000A 0x00000ff8>,
> +                                      <0x10000 0x10033 0x000ff000>;
> +        riscv,raw-event-to-mhpmcounters =
> +            /* For event ID 0x0002 */
> +            <0x0000 0x0002 0xffffffff 0xffffffff 0x00000f8>,
> +            /* For event ID 0-4 */
> +            <0x0 0x0 0xffffffff 0xfffffff0 0x00000ff0>,
> +            /* For event ID 0xffffffff0000000f - 0xffffffff000000ff */
> +            <0xffffffff 0x0 0xffffffff 0xffffff0f 0x00000ff0>;
> +    };
> +
> +  - |
> +    /*
> +     * For HiFive Unmatched board the encodings can be found here
> +     * https://sifive.cdn.prismic.io/sifive/1a82e600-1f93-4f41-b2d8-86ed8b16acba_fu740-c000-manual-v1p6.pdf
> +     * This example also binds standard SBI PMU hardware id's to U74 PMU event
> +     * codes, U74 uses a bitfield for events encoding, so several U74 events
> +     * can be bound to single perf id.
> +     * See SBI PMU hardware id's in OpenSBI's include/sbi/sbi_ecall_interface.h
> +     */
> +    pmu {
> +          compatible = "riscv,pmu";
> +          riscv,event-to-mhpmevent =
> +              /* SBI_PMU_HW_CACHE_REFERENCES -> Instruction or Data cache/ITIM busy */
> +              <0x00003 0x00000000 0x1801>,
> +              /* SBI_PMU_HW_CACHE_MISSES -> Instruction or Data cache miss or MMIO access */
> +              <0x00004 0x00000000 0x0302>,
> +              /* SBI_PMU_HW_BRANCH_INSTRUCTIONS -> Conditional branch retired */
> +              <0x00005 0x00000000 0x4000>,
> +              /* SBI_PMU_HW_BRANCH_MISSES -> Branch or jump misprediction */
> +              <0x00006 0x00000000 0x6001>,
> +              /* L1D_READ_MISS -> Data cache miss or MMIO access */
> +              <0x10001 0x00000000 0x0202>,
> +              /* L1D_WRITE_ACCESS -> Data cache write-back */
> +              <0x10002 0x00000000 0x0402>,
> +              /* L1I_READ_ACCESS -> Instruction cache miss */
> +              <0x10009 0x00000000 0x0102>,
> +              /* LL_READ_MISS -> UTLB miss */
> +              <0x10011 0x00000000 0x2002>,
> +              /* DTLB_READ_MISS -> Data TLB miss */
> +              <0x10019 0x00000000 0x1002>,
> +              /* ITLB_READ_MISS-> Instruction TLB miss */
> +              <0x10021 0x00000000 0x0802>;
> +          riscv,event-to-mhpmcounters = <0x00003 0x00006 0x18>,
> +                                        <0x10001 0x10002 0x18>,
> +                                        <0x10009 0x10009 0x18>,
> +                                        <0x10011 0x10011 0x18>,
> +                                        <0x10019 0x10019 0x18>,
> +                                        <0x10021 0x10021 0x18>;
> +          riscv,raw-event-to-mhpmcounters = <0x0 0x0 0xffffffff 0xfc0000ff 0x18>,
> +                                            <0x0 0x1 0xffffffff 0xfff800ff 0x18>,
> +                                            <0x0 0x2 0xffffffff 0xffffe0ff 0x18>;
> +    };
> -- 
> 2.38.1
> 
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
  2022-12-22 18:06   ` Rob Herring
@ 2022-12-22 19:23     ` Conor Dooley
  -1 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-22 19:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: krzysztof.kozlowski+dt, palmer, atishp, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, apatel, Will Deacon, Mark Rutland,
	opensbi

[-- Attachment #1: Type: text/plain, Size: 8801 bytes --]

Hey Rob,

On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The SBI PMU extension requires a firmware to be aware of the event to
> > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > DeviceTree to describe the PMU mappings. This binding is currently
> > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > since v7.2.0.
> > 
> > Import the binding for use while validating dtb dumps from QEMU and
> > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > mapping.
> > 
> > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > I asked Rob on IRC about these bindings a few weeks ago & he said he
> > would be willing to take them. I have modified wording slightly in the
> > descriptions, but have mostly left things as close to the OpenSBI
> > documentation as possible.
> 
> Please CC the perf maintainers. Might be crickets, but so they at least 
> have a chance to see it.

Yah, I was kinda unsure who to CC. It does list them as being
specifically ARM so I probably made the wrong choice about inclusion.
I've added them now.

> > I'm not super sure about what I've done with the properties being
> > correct type wise, I went digging in bindings and am sorta using the
> > first thing that "fit".
> > 
> > Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> > is BSD-2-Clause licensed so I am also unsure as to what license I can
> > use for this binding since that's where I took it from.
> > ---
> >  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
> >  1 file changed, 158 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > new file mode 100644
> > index 000000000000..d65f937680af
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > @@ -0,0 +1,158 @@
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V SBI PMU events
> > +
> > +maintainers:
> > +  - Atish Patra <atishp@rivosinc.com>
> > +
> > +description: |
> > +  SBI PMU extension supports allow supervisor software to configure, start &
> > +  stop any performance counter at anytime. Thus, a user can leverage full
> > +  capability of performance analysis tools such as perf if the SBI PMU
> > +  extension is enabled. The OpenSBI implementation makes the following
> > +  assumptions about the hardware platform:
> > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > +    PMU extension will not be enabled.
> > +
> > +    The platform must provide information about PMU event to counter mapping
> > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > +    extension will not be enabled.
> > +
> > +    The platforms should provide information about the PMU event selector
> > +    values that should be encoded in the expected value of MHPMEVENTx while
> > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > +    device tree or platform specific hooks. The exact value to be written to
> > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > +    platform writes the zero-extended event_idx as the expected value for
> > +    hardware cache/generic events as suggested by the SBI specification.
> > +
> > +properties:
> > +  compatible:
> > +    const: riscv,pmu
> > +
> > +  riscv,event-to-mhpmevent:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > +      for that event.
> > +      The mapping is encoded in an array format where each row represents an
> 
> s/array/matrix/
> 
> > +      event. The first element represents the event idx while the second &
> > +      third elements represent the event selector value that should be encoded
> > +      in the expected value to be written in MHPMEVENTx.
> > +      This property shouldn't encode any raw hardware event.
> > +    minItems: 1
> > +    maxItems: 255

I copied these 255s from dt-schema somewhere as a sane max.
Atish, is there a cap here or should we allow as many as someone likes?
The md binding doesn't mention any limits - is it in the SBI spec?
The strongest wording I saw there was "limited" & event idx is 20 bits
wide as per the spec [0]. Does that make 2^20 the hard limit here?

0 - https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc#11-performance-monitoring-unit-extension-eid-0x504d55-pmu

> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Huh? A property can only have 1 type. I wonder what the tools do with 
> this...

I suppose I left this over (by accident) from when I had it arranged
slightly differently. I guess I never actually noticed as the tools
don't appear to complain. Now dropped :)

> > +      maxItems: 3
> 
> Better to do:
> 
>          items:
>            - description: what's in the 1st word
>            - description: what's in the 2nd word
>            - description: what's in the 3rd word
> 
> And rework the overall description to not say the same thing.

Yah, good idea. I'll do that for the next version.

> > +  riscv,event-to-mhpmcounters:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents a MANY-to-MANY mapping between a range of events and all the
> > +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> > +      of events. The information is encoded in an array format where each
> > +      row represents a certain range of events and corresponding counters.
> > +      The first element represents starting of the pmu event id and 2nd column
> > +      represents the end of the pmu event id. The third element represent a
> > +      bitmap of all the MHPMCOUNTERx.
> > +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> > +      it can be omitted.
> 
> No need to state this in freeform text, use 'dependencies'.

Oh! I didn't know that that existed. I was going to go and do some sort
of required properties dance but perhaps that's not needed now (at least
to the same extent).

> > +      This property shouldn't encode any raw event.
> > +    minItems: 1
> > +    maxItems: 255
> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      maxItems: 3
> > +
> > +  riscv,raw-event-to-mhpmcounters:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > +      that raw event.
> > +      The encoding of the raw events are platform specific. The information is
> > +      encoded in an array format where each row represents the specific raw
> > +      event(s). The first element is a 64-bit match value where the invariant
> > +      bits of range of events are set. The second element is a 64-bit mask that
> > +      will have all the variant bits of the range of events cleared. All other
> > +      bits should be set in the mask. The third element is a 32-bit value to
> > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > +    minItems: 1
> > +    maxItems: 255
> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      maxItems: 5
> > +
> > +required:
> > +  - compatible
> 
> I assume at least one of the other properties must be present?

Atish: (and +CC the OpenSBI list)
Which properties are actually needed here? Or are any? The md binding
in the OpenSBI sources doesn't seem to require anything other than the
compatible?

Thanks Rob,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-22 19:23     ` Conor Dooley
  0 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-22 19:23 UTC (permalink / raw)
  To: Rob Herring
  Cc: krzysztof.kozlowski+dt, palmer, atishp, Conor Dooley, devicetree,
	linux-riscv, linux-kernel, apatel, Will Deacon, Mark Rutland,
	opensbi


[-- Attachment #1.1: Type: text/plain, Size: 8801 bytes --]

Hey Rob,

On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > The SBI PMU extension requires a firmware to be aware of the event to
> > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > DeviceTree to describe the PMU mappings. This binding is currently
> > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > since v7.2.0.
> > 
> > Import the binding for use while validating dtb dumps from QEMU and
> > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > mapping.
> > 
> > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > I asked Rob on IRC about these bindings a few weeks ago & he said he
> > would be willing to take them. I have modified wording slightly in the
> > descriptions, but have mostly left things as close to the OpenSBI
> > documentation as possible.
> 
> Please CC the perf maintainers. Might be crickets, but so they at least 
> have a chance to see it.

Yah, I was kinda unsure who to CC. It does list them as being
specifically ARM so I probably made the wrong choice about inclusion.
I've added them now.

> > I'm not super sure about what I've done with the properties being
> > correct type wise, I went digging in bindings and am sorta using the
> > first thing that "fit".
> > 
> > Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> > is BSD-2-Clause licensed so I am also unsure as to what license I can
> > use for this binding since that's where I took it from.
> > ---
> >  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
> >  1 file changed, 158 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > new file mode 100644
> > index 000000000000..d65f937680af
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > @@ -0,0 +1,158 @@
> > +# SPDX-License-Identifier: BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: RISC-V SBI PMU events
> > +
> > +maintainers:
> > +  - Atish Patra <atishp@rivosinc.com>
> > +
> > +description: |
> > +  SBI PMU extension supports allow supervisor software to configure, start &
> > +  stop any performance counter at anytime. Thus, a user can leverage full
> > +  capability of performance analysis tools such as perf if the SBI PMU
> > +  extension is enabled. The OpenSBI implementation makes the following
> > +  assumptions about the hardware platform:
> > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > +    PMU extension will not be enabled.
> > +
> > +    The platform must provide information about PMU event to counter mapping
> > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > +    extension will not be enabled.
> > +
> > +    The platforms should provide information about the PMU event selector
> > +    values that should be encoded in the expected value of MHPMEVENTx while
> > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > +    device tree or platform specific hooks. The exact value to be written to
> > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > +    platform writes the zero-extended event_idx as the expected value for
> > +    hardware cache/generic events as suggested by the SBI specification.
> > +
> > +properties:
> > +  compatible:
> > +    const: riscv,pmu
> > +
> > +  riscv,event-to-mhpmevent:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > +      for that event.
> > +      The mapping is encoded in an array format where each row represents an
> 
> s/array/matrix/
> 
> > +      event. The first element represents the event idx while the second &
> > +      third elements represent the event selector value that should be encoded
> > +      in the expected value to be written in MHPMEVENTx.
> > +      This property shouldn't encode any raw hardware event.
> > +    minItems: 1
> > +    maxItems: 255

I copied these 255s from dt-schema somewhere as a sane max.
Atish, is there a cap here or should we allow as many as someone likes?
The md binding doesn't mention any limits - is it in the SBI spec?
The strongest wording I saw there was "limited" & event idx is 20 bits
wide as per the spec [0]. Does that make 2^20 the hard limit here?

0 - https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc#11-performance-monitoring-unit-extension-eid-0x504d55-pmu

> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> 
> Huh? A property can only have 1 type. I wonder what the tools do with 
> this...

I suppose I left this over (by accident) from when I had it arranged
slightly differently. I guess I never actually noticed as the tools
don't appear to complain. Now dropped :)

> > +      maxItems: 3
> 
> Better to do:
> 
>          items:
>            - description: what's in the 1st word
>            - description: what's in the 2nd word
>            - description: what's in the 3rd word
> 
> And rework the overall description to not say the same thing.

Yah, good idea. I'll do that for the next version.

> > +  riscv,event-to-mhpmcounters:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents a MANY-to-MANY mapping between a range of events and all the
> > +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> > +      of events. The information is encoded in an array format where each
> > +      row represents a certain range of events and corresponding counters.
> > +      The first element represents starting of the pmu event id and 2nd column
> > +      represents the end of the pmu event id. The third element represent a
> > +      bitmap of all the MHPMCOUNTERx.
> > +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> > +      it can be omitted.
> 
> No need to state this in freeform text, use 'dependencies'.

Oh! I didn't know that that existed. I was going to go and do some sort
of required properties dance but perhaps that's not needed now (at least
to the same extent).

> > +      This property shouldn't encode any raw event.
> > +    minItems: 1
> > +    maxItems: 255
> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      maxItems: 3
> > +
> > +  riscv,raw-event-to-mhpmcounters:
> > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > +    description:
> > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > +      that raw event.
> > +      The encoding of the raw events are platform specific. The information is
> > +      encoded in an array format where each row represents the specific raw
> > +      event(s). The first element is a 64-bit match value where the invariant
> > +      bits of range of events are set. The second element is a 64-bit mask that
> > +      will have all the variant bits of the range of events cleared. All other
> > +      bits should be set in the mask. The third element is a 32-bit value to
> > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > +    minItems: 1
> > +    maxItems: 255
> > +    items:
> > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > +      maxItems: 5
> > +
> > +required:
> > +  - compatible
> 
> I assume at least one of the other properties must be present?

Atish: (and +CC the OpenSBI list)
Which properties are actually needed here? Or are any? The md binding
in the OpenSBI sources doesn't seem to require anything other than the
compatible?

Thanks Rob,
Conor.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
  2022-12-22 19:23     ` Conor Dooley
@ 2022-12-22 19:55       ` Atish Kumar Patra
  -1 siblings, 0 replies; 12+ messages in thread
From: Atish Kumar Patra @ 2022-12-22 19:55 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi

On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
>
> Hey Rob,
>
> On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > The SBI PMU extension requires a firmware to be aware of the event to
> > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > DeviceTree to describe the PMU mappings. This binding is currently
> > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > since v7.2.0.
> > >
> > > Import the binding for use while validating dtb dumps from QEMU and
> > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > mapping.
> > >
> > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > > I asked Rob on IRC about these bindings a few weeks ago & he said he
> > > would be willing to take them. I have modified wording slightly in the
> > > descriptions, but have mostly left things as close to the OpenSBI
> > > documentation as possible.
> >
> > Please CC the perf maintainers. Might be crickets, but so they at least
> > have a chance to see it.
>
> Yah, I was kinda unsure who to CC. It does list them as being
> specifically ARM so I probably made the wrong choice about inclusion.
> I've added them now.
>
> > > I'm not super sure about what I've done with the properties being
> > > correct type wise, I went digging in bindings and am sorta using the
> > > first thing that "fit".
> > >
> > > Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> > > is BSD-2-Clause licensed so I am also unsure as to what license I can
> > > use for this binding since that's where I took it from.
> > > ---
> > >  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
> > >  1 file changed, 158 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > > new file mode 100644
> > > index 000000000000..d65f937680af
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > > @@ -0,0 +1,158 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: RISC-V SBI PMU events
> > > +
> > > +maintainers:
> > > +  - Atish Patra <atishp@rivosinc.com>
> > > +
> > > +description: |
> > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > +  extension is enabled. The OpenSBI implementation makes the following
> > > +  assumptions about the hardware platform:
> > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > +    PMU extension will not be enabled.
> > > +

This is no longer true since OpenSBI commit b28f070. We should remove
this from OpenSBI docs as well.

> > > +    The platform must provide information about PMU event to counter mapping
> > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > +    extension will not be enabled.
> > > +
> > > +    The platforms should provide information about the PMU event selector
> > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > +    device tree or platform specific hooks. The exact value to be written to
> > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > +    platform writes the zero-extended event_idx as the expected value for
> > > +    hardware cache/generic events as suggested by the SBI specification.
> > > +

But the larger question is these are OpenSBI implementation specific
assumptions. Should it be included in
the DT binding ?

> > > +properties:
> > > +  compatible:
> > > +    const: riscv,pmu
> > > +
> > > +  riscv,event-to-mhpmevent:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > +      for that event.
> > > +      The mapping is encoded in an array format where each row represents an
> >
> > s/array/matrix/
> >
> > > +      event. The first element represents the event idx while the second &
> > > +      third elements represent the event selector value that should be encoded
> > > +      in the expected value to be written in MHPMEVENTx.
> > > +      This property shouldn't encode any raw hardware event.
> > > +    minItems: 1
> > > +    maxItems: 255
>
> I copied these 255s from dt-schema somewhere as a sane max.
> Atish, is there a cap here or should we allow as many as someone likes?
> The md binding doesn't mention any limits - is it in the SBI spec?
> The strongest wording I saw there was "limited" & event idx is 20 bits
> wide as per the spec [0]. Does that make 2^20 the hard limit here?
>

This is for hardware & cache events. The total number of events
defined there are 52 (10 HW + 42 Cache) events.
So 255 is a sane max that provides enough room for expansion in future
if required.

> 0 - https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc#11-performance-monitoring-unit-extension-eid-0x504d55-pmu
>
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > Huh? A property can only have 1 type. I wonder what the tools do with
> > this...
>
> I suppose I left this over (by accident) from when I had it arranged
> slightly differently. I guess I never actually noticed as the tools
> don't appear to complain. Now dropped :)
>
> > > +      maxItems: 3
> >
> > Better to do:
> >
> >          items:
> >            - description: what's in the 1st word
> >            - description: what's in the 2nd word
> >            - description: what's in the 3rd word
> >
> > And rework the overall description to not say the same thing.
>
> Yah, good idea. I'll do that for the next version.
>
> > > +  riscv,event-to-mhpmcounters:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents a MANY-to-MANY mapping between a range of events and all the
> > > +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> > > +      of events. The information is encoded in an array format where each
> > > +      row represents a certain range of events and corresponding counters.
> > > +      The first element represents starting of the pmu event id and 2nd column
> > > +      represents the end of the pmu event id. The third element represent a
> > > +      bitmap of all the MHPMCOUNTERx.
> > > +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> > > +      it can be omitted.
> >
> > No need to state this in freeform text, use 'dependencies'.
>
> Oh! I didn't know that that existed. I was going to go and do some sort
> of required properties dance but perhaps that's not needed now (at least
> to the same extent).
>
> > > +      This property shouldn't encode any raw event.
> > > +    minItems: 1
> > > +    maxItems: 255
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +      maxItems: 3
> > > +
> > > +  riscv,raw-event-to-mhpmcounters:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > +      that raw event.
> > > +      The encoding of the raw events are platform specific. The information is
> > > +      encoded in an array format where each row represents the specific raw
> > > +      event(s). The first element is a 64-bit match value where the invariant
> > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > +      will have all the variant bits of the range of events cleared. All other
> > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > +    minItems: 1
> > > +    maxItems: 255
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +      maxItems: 5
> > > +
> > > +required:
> > > +  - compatible
> >
> > I assume at least one of the other properties must be present?
>
> Atish: (and +CC the OpenSBI list)
> Which properties are actually needed here? Or are any? The md binding
> in the OpenSBI sources doesn't seem to require anything other than the
> compatible?
>

That's true. Opensbi allows the platform to define functions
to provide the mapping. Now that's an OpenSBI implementation thing.
Other implementations may choose to use it differently.


> Thanks Rob,
> Conor.
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-22 19:55       ` Atish Kumar Patra
  0 siblings, 0 replies; 12+ messages in thread
From: Atish Kumar Patra @ 2022-12-22 19:55 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi

On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
>
> Hey Rob,
>
> On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > From: Conor Dooley <conor.dooley@microchip.com>
> > >
> > > The SBI PMU extension requires a firmware to be aware of the event to
> > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > DeviceTree to describe the PMU mappings. This binding is currently
> > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > since v7.2.0.
> > >
> > > Import the binding for use while validating dtb dumps from QEMU and
> > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > mapping.
> > >
> > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > ---
> > > I asked Rob on IRC about these bindings a few weeks ago & he said he
> > > would be willing to take them. I have modified wording slightly in the
> > > descriptions, but have mostly left things as close to the OpenSBI
> > > documentation as possible.
> >
> > Please CC the perf maintainers. Might be crickets, but so they at least
> > have a chance to see it.
>
> Yah, I was kinda unsure who to CC. It does list them as being
> specifically ARM so I probably made the wrong choice about inclusion.
> I've added them now.
>
> > > I'm not super sure about what I've done with the properties being
> > > correct type wise, I went digging in bindings and am sorta using the
> > > first thing that "fit".
> > >
> > > Since you wrote the md doc Atish, I put your co-developed-by. OpenSBI
> > > is BSD-2-Clause licensed so I am also unsure as to what license I can
> > > use for this binding since that's where I took it from.
> > > ---
> > >  .../devicetree/bindings/perf/riscv,pmu.yaml   | 158 ++++++++++++++++++
> > >  1 file changed, 158 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/perf/riscv,pmu.yaml b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > > new file mode 100644
> > > index 000000000000..d65f937680af
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/perf/riscv,pmu.yaml
> > > @@ -0,0 +1,158 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/perf/riscv,pmu.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: RISC-V SBI PMU events
> > > +
> > > +maintainers:
> > > +  - Atish Patra <atishp@rivosinc.com>
> > > +
> > > +description: |
> > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > +  extension is enabled. The OpenSBI implementation makes the following
> > > +  assumptions about the hardware platform:
> > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > +    PMU extension will not be enabled.
> > > +

This is no longer true since OpenSBI commit b28f070. We should remove
this from OpenSBI docs as well.

> > > +    The platform must provide information about PMU event to counter mapping
> > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > +    extension will not be enabled.
> > > +
> > > +    The platforms should provide information about the PMU event selector
> > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > +    device tree or platform specific hooks. The exact value to be written to
> > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > +    platform writes the zero-extended event_idx as the expected value for
> > > +    hardware cache/generic events as suggested by the SBI specification.
> > > +

But the larger question is these are OpenSBI implementation specific
assumptions. Should it be included in
the DT binding ?

> > > +properties:
> > > +  compatible:
> > > +    const: riscv,pmu
> > > +
> > > +  riscv,event-to-mhpmevent:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > +      for that event.
> > > +      The mapping is encoded in an array format where each row represents an
> >
> > s/array/matrix/
> >
> > > +      event. The first element represents the event idx while the second &
> > > +      third elements represent the event selector value that should be encoded
> > > +      in the expected value to be written in MHPMEVENTx.
> > > +      This property shouldn't encode any raw hardware event.
> > > +    minItems: 1
> > > +    maxItems: 255
>
> I copied these 255s from dt-schema somewhere as a sane max.
> Atish, is there a cap here or should we allow as many as someone likes?
> The md binding doesn't mention any limits - is it in the SBI spec?
> The strongest wording I saw there was "limited" & event idx is 20 bits
> wide as per the spec [0]. Does that make 2^20 the hard limit here?
>

This is for hardware & cache events. The total number of events
defined there are 52 (10 HW + 42 Cache) events.
So 255 is a sane max that provides enough room for expansion in future
if required.

> 0 - https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/riscv-sbi.adoc#11-performance-monitoring-unit-extension-eid-0x504d55-pmu
>
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > Huh? A property can only have 1 type. I wonder what the tools do with
> > this...
>
> I suppose I left this over (by accident) from when I had it arranged
> slightly differently. I guess I never actually noticed as the tools
> don't appear to complain. Now dropped :)
>
> > > +      maxItems: 3
> >
> > Better to do:
> >
> >          items:
> >            - description: what's in the 1st word
> >            - description: what's in the 2nd word
> >            - description: what's in the 3rd word
> >
> > And rework the overall description to not say the same thing.
>
> Yah, good idea. I'll do that for the next version.
>
> > > +  riscv,event-to-mhpmcounters:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents a MANY-to-MANY mapping between a range of events and all the
> > > +      MHPMCOUNTERx in a bitmap format that can be used to monitor these range
> > > +      of events. The information is encoded in an array format where each
> > > +      row represents a certain range of events and corresponding counters.
> > > +      The first element represents starting of the pmu event id and 2nd column
> > > +      represents the end of the pmu event id. The third element represent a
> > > +      bitmap of all the MHPMCOUNTERx.
> > > +      This property is mandatory if event-to-mhpmevent is present. Otherwise,
> > > +      it can be omitted.
> >
> > No need to state this in freeform text, use 'dependencies'.
>
> Oh! I didn't know that that existed. I was going to go and do some sort
> of required properties dance but perhaps that's not needed now (at least
> to the same extent).
>
> > > +      This property shouldn't encode any raw event.
> > > +    minItems: 1
> > > +    maxItems: 255
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +      maxItems: 3
> > > +
> > > +  riscv,raw-event-to-mhpmcounters:
> > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +    description:
> > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > +      that raw event.
> > > +      The encoding of the raw events are platform specific. The information is
> > > +      encoded in an array format where each row represents the specific raw
> > > +      event(s). The first element is a 64-bit match value where the invariant
> > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > +      will have all the variant bits of the range of events cleared. All other
> > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > +    minItems: 1
> > > +    maxItems: 255
> > > +    items:
> > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > +      maxItems: 5
> > > +
> > > +required:
> > > +  - compatible
> >
> > I assume at least one of the other properties must be present?
>
> Atish: (and +CC the OpenSBI list)
> Which properties are actually needed here? Or are any? The md binding
> in the OpenSBI sources doesn't seem to require anything other than the
> compatible?
>

That's true. Opensbi allows the platform to define functions
to provide the mapping. Now that's an OpenSBI implementation thing.
Other implementations may choose to use it differently.


> Thanks Rob,
> Conor.
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
  2022-12-22 19:55       ` Atish Kumar Patra
@ 2022-12-22 20:16         ` Conor Dooley
  -1 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-22 20:16 UTC (permalink / raw)
  To: Atish Kumar Patra
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi

[-- Attachment #1: Type: text/plain, Size: 6408 bytes --]

On Thu, Dec 22, 2022 at 11:55:29AM -0800, Atish Kumar Patra wrote:
> On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
> > On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > > From: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > The SBI PMU extension requires a firmware to be aware of the event to
> > > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > > DeviceTree to describe the PMU mappings. This binding is currently
> > > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > > since v7.2.0.
> > > >
> > > > Import the binding for use while validating dtb dumps from QEMU and
> > > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > > mapping.
> > > >
> > > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > +description: |
> > > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > > +  extension is enabled. The OpenSBI implementation makes the following
> > > > +  assumptions about the hardware platform:
> > > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > > +    PMU extension will not be enabled.
> > > > +
> 
> This is no longer true since OpenSBI commit b28f070. We should remove
> this from OpenSBI docs as well.

Since you know the detail of it, I volunteer you for that one ;)

> > > > +    The platform must provide information about PMU event to counter mapping
> > > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > > +    extension will not be enabled.
> > > > +
> > > > +    The platforms should provide information about the PMU event selector
> > > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > > +    device tree or platform specific hooks. The exact value to be written to
> > > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > > +    platform writes the zero-extended event_idx as the expected value for
> > > > +    hardware cache/generic events as suggested by the SBI specification.
> > > > +
> 
> But the larger question is these are OpenSBI implementation specific
> assumptions. Should it be included in
> the DT binding?

Probably not. I'll drop it for v2.

> > > > +properties:
> > > > +  compatible:
> > > > +    const: riscv,pmu
> > > > +
> > > > +  riscv,event-to-mhpmevent:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > +    description:
> > > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > > +      for that event.
> > > > +      The mapping is encoded in an array format where each row represents an
> > >
> > > s/array/matrix/
> > >
> > > > +      event. The first element represents the event idx while the second &
> > > > +      third elements represent the event selector value that should be encoded
> > > > +      in the expected value to be written in MHPMEVENTx.
> > > > +      This property shouldn't encode any raw hardware event.
> > > > +    minItems: 1
> > > > +    maxItems: 255
> >
> > I copied these 255s from dt-schema somewhere as a sane max.
> > Atish, is there a cap here or should we allow as many as someone likes?
> > The md binding doesn't mention any limits - is it in the SBI spec?
> > The strongest wording I saw there was "limited" & event idx is 20 bits
> > wide as per the spec [0]. Does that make 2^20 the hard limit here?
> >
> 
> This is for hardware & cache events. The total number of events
> defined there are 52 (10 HW + 42 Cache) events.
> So 255 is a sane max that provides enough room for expansion in future
> if required.

WFM.

> > > > +  riscv,raw-event-to-mhpmcounters:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > +    description:
> > > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > > +      that raw event.
> > > > +      The encoding of the raw events are platform specific. The information is
> > > > +      encoded in an array format where each row represents the specific raw
> > > > +      event(s). The first element is a 64-bit match value where the invariant
> > > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > > +      will have all the variant bits of the range of events cleared. All other
> > > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > > +    minItems: 1
> > > > +    maxItems: 255
> > > > +    items:
> > > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +      maxItems: 5
> > > > +
> > > > +required:
> > > > +  - compatible
> > >
> > > I assume at least one of the other properties must be present?
> >
> > Atish: (and +CC the OpenSBI list)
> > Which properties are actually needed here? Or are any? The md binding
> > in the OpenSBI sources doesn't seem to require anything other than the
> > compatible?
> >
> 
> That's true. Opensbi allows the platform to define functions
> to provide the mapping. Now that's an OpenSBI implementation thing.
> Other implementations may choose to use it differently.

In the case where the platform can define functions, the "bare" node
with just the compatible is still required though in that case, right?

Thanks,
Conor.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-22 20:16         ` Conor Dooley
  0 siblings, 0 replies; 12+ messages in thread
From: Conor Dooley @ 2022-12-22 20:16 UTC (permalink / raw)
  To: Atish Kumar Patra
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi


[-- Attachment #1.1: Type: text/plain, Size: 6408 bytes --]

On Thu, Dec 22, 2022 at 11:55:29AM -0800, Atish Kumar Patra wrote:
> On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
> > On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > > From: Conor Dooley <conor.dooley@microchip.com>
> > > >
> > > > The SBI PMU extension requires a firmware to be aware of the event to
> > > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > > DeviceTree to describe the PMU mappings. This binding is currently
> > > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > > since v7.2.0.
> > > >
> > > > Import the binding for use while validating dtb dumps from QEMU and
> > > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > > mapping.
> > > >
> > > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > +description: |
> > > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > > +  extension is enabled. The OpenSBI implementation makes the following
> > > > +  assumptions about the hardware platform:
> > > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > > +    PMU extension will not be enabled.
> > > > +
> 
> This is no longer true since OpenSBI commit b28f070. We should remove
> this from OpenSBI docs as well.

Since you know the detail of it, I volunteer you for that one ;)

> > > > +    The platform must provide information about PMU event to counter mapping
> > > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > > +    extension will not be enabled.
> > > > +
> > > > +    The platforms should provide information about the PMU event selector
> > > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > > +    device tree or platform specific hooks. The exact value to be written to
> > > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > > +    platform writes the zero-extended event_idx as the expected value for
> > > > +    hardware cache/generic events as suggested by the SBI specification.
> > > > +
> 
> But the larger question is these are OpenSBI implementation specific
> assumptions. Should it be included in
> the DT binding?

Probably not. I'll drop it for v2.

> > > > +properties:
> > > > +  compatible:
> > > > +    const: riscv,pmu
> > > > +
> > > > +  riscv,event-to-mhpmevent:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > +    description:
> > > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > > +      for that event.
> > > > +      The mapping is encoded in an array format where each row represents an
> > >
> > > s/array/matrix/
> > >
> > > > +      event. The first element represents the event idx while the second &
> > > > +      third elements represent the event selector value that should be encoded
> > > > +      in the expected value to be written in MHPMEVENTx.
> > > > +      This property shouldn't encode any raw hardware event.
> > > > +    minItems: 1
> > > > +    maxItems: 255
> >
> > I copied these 255s from dt-schema somewhere as a sane max.
> > Atish, is there a cap here or should we allow as many as someone likes?
> > The md binding doesn't mention any limits - is it in the SBI spec?
> > The strongest wording I saw there was "limited" & event idx is 20 bits
> > wide as per the spec [0]. Does that make 2^20 the hard limit here?
> >
> 
> This is for hardware & cache events. The total number of events
> defined there are 52 (10 HW + 42 Cache) events.
> So 255 is a sane max that provides enough room for expansion in future
> if required.

WFM.

> > > > +  riscv,raw-event-to-mhpmcounters:
> > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > +    description:
> > > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > > +      that raw event.
> > > > +      The encoding of the raw events are platform specific. The information is
> > > > +      encoded in an array format where each row represents the specific raw
> > > > +      event(s). The first element is a 64-bit match value where the invariant
> > > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > > +      will have all the variant bits of the range of events cleared. All other
> > > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > > +    minItems: 1
> > > > +    maxItems: 255
> > > > +    items:
> > > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > +      maxItems: 5
> > > > +
> > > > +required:
> > > > +  - compatible
> > >
> > > I assume at least one of the other properties must be present?
> >
> > Atish: (and +CC the OpenSBI list)
> > Which properties are actually needed here? Or are any? The md binding
> > in the OpenSBI sources doesn't seem to require anything other than the
> > compatible?
> >
> 
> That's true. Opensbi allows the platform to define functions
> to provide the mapping. Now that's an OpenSBI implementation thing.
> Other implementations may choose to use it differently.

In the case where the platform can define functions, the "bare" node
with just the compatible is still required though in that case, right?

Thanks,
Conor.


[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
  2022-12-22 20:16         ` Conor Dooley
@ 2022-12-22 20:39           ` Atish Kumar Patra
  -1 siblings, 0 replies; 12+ messages in thread
From: Atish Kumar Patra @ 2022-12-22 20:39 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi

On Thu, Dec 22, 2022 at 12:16 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Dec 22, 2022 at 11:55:29AM -0800, Atish Kumar Patra wrote:
> > On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
> > > On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > > > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > > > From: Conor Dooley <conor.dooley@microchip.com>
> > > > >
> > > > > The SBI PMU extension requires a firmware to be aware of the event to
> > > > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > > > DeviceTree to describe the PMU mappings. This binding is currently
> > > > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > > > since v7.2.0.
> > > > >
> > > > > Import the binding for use while validating dtb dumps from QEMU and
> > > > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > > > mapping.
> > > > >
> > > > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > +description: |
> > > > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > > > +  extension is enabled. The OpenSBI implementation makes the following
> > > > > +  assumptions about the hardware platform:
> > > > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > > > +    PMU extension will not be enabled.
> > > > > +
> >
> > This is no longer true since OpenSBI commit b28f070. We should remove
> > this from OpenSBI docs as well.
>
> Since you know the detail of it, I volunteer you for that one ;)
>

Done.

> > > > > +    The platform must provide information about PMU event to counter mapping
> > > > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > > > +    extension will not be enabled.
> > > > > +
> > > > > +    The platforms should provide information about the PMU event selector
> > > > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > > > +    device tree or platform specific hooks. The exact value to be written to
> > > > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > > > +    platform writes the zero-extended event_idx as the expected value for
> > > > > +    hardware cache/generic events as suggested by the SBI specification.
> > > > > +
> >
> > But the larger question is these are OpenSBI implementation specific
> > assumptions. Should it be included in
> > the DT binding?
>
> Probably not. I'll drop it for v2.
>
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: riscv,pmu
> > > > > +
> > > > > +  riscv,event-to-mhpmevent:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > > +    description:
> > > > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > > > +      for that event.
> > > > > +      The mapping is encoded in an array format where each row represents an
> > > >
> > > > s/array/matrix/
> > > >
> > > > > +      event. The first element represents the event idx while the second &
> > > > > +      third elements represent the event selector value that should be encoded
> > > > > +      in the expected value to be written in MHPMEVENTx.
> > > > > +      This property shouldn't encode any raw hardware event.
> > > > > +    minItems: 1
> > > > > +    maxItems: 255
> > >
> > > I copied these 255s from dt-schema somewhere as a sane max.
> > > Atish, is there a cap here or should we allow as many as someone likes?
> > > The md binding doesn't mention any limits - is it in the SBI spec?
> > > The strongest wording I saw there was "limited" & event idx is 20 bits
> > > wide as per the spec [0]. Does that make 2^20 the hard limit here?
> > >
> >
> > This is for hardware & cache events. The total number of events
> > defined there are 52 (10 HW + 42 Cache) events.
> > So 255 is a sane max that provides enough room for expansion in future
> > if required.
>
> WFM.
>
> > > > > +  riscv,raw-event-to-mhpmcounters:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > > +    description:
> > > > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > > > +      that raw event.
> > > > > +      The encoding of the raw events are platform specific. The information is
> > > > > +      encoded in an array format where each row represents the specific raw
> > > > > +      event(s). The first element is a 64-bit match value where the invariant
> > > > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > > > +      will have all the variant bits of the range of events cleared. All other
> > > > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > > > +    minItems: 1
> > > > > +    maxItems: 255
> > > > > +    items:
> > > > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +      maxItems: 5
> > > > > +
> > > > > +required:
> > > > > +  - compatible
> > > >
> > > > I assume at least one of the other properties must be present?
> > >
> > > Atish: (and +CC the OpenSBI list)
> > > Which properties are actually needed here? Or are any? The md binding
> > > in the OpenSBI sources doesn't seem to require anything other than the
> > > compatible?
> > >
> >
> > That's true. Opensbi allows the platform to define functions
> > to provide the mapping. Now that's an OpenSBI implementation thing.
> > Other implementations may choose to use it differently.
>
> In the case where the platform can define functions, the "bare" node
> with just the compatible is still required though in that case, right?
>

Yup.

> Thanks,
> Conor.
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings
@ 2022-12-22 20:39           ` Atish Kumar Patra
  0 siblings, 0 replies; 12+ messages in thread
From: Atish Kumar Patra @ 2022-12-22 20:39 UTC (permalink / raw)
  To: Conor Dooley
  Cc: Rob Herring, krzysztof.kozlowski+dt, palmer, Conor Dooley,
	devicetree, linux-riscv, linux-kernel, apatel, Will Deacon,
	Mark Rutland, opensbi

On Thu, Dec 22, 2022 at 12:16 PM Conor Dooley <conor@kernel.org> wrote:
>
> On Thu, Dec 22, 2022 at 11:55:29AM -0800, Atish Kumar Patra wrote:
> > On Thu, Dec 22, 2022 at 11:24 AM Conor Dooley <conor@kernel.org> wrote:
> > > On Thu, Dec 22, 2022 at 12:06:27PM -0600, Rob Herring wrote:
> > > > On Wed, Dec 21, 2022 at 02:15:49PM +0000, Conor Dooley wrote:
> > > > > From: Conor Dooley <conor.dooley@microchip.com>
> > > > >
> > > > > The SBI PMU extension requires a firmware to be aware of the event to
> > > > > counter/mhpmevent mappings supported by the hardware. OpenSBI may use
> > > > > DeviceTree to describe the PMU mappings. This binding is currently
> > > > > described in markdown in OpenSBI (since v1.0 in Dec 2021) & used by QEMU
> > > > > since v7.2.0.
> > > > >
> > > > > Import the binding for use while validating dtb dumps from QEMU and
> > > > > upcoming hardware (eg JH7110 SoC) that will make use of the event
> > > > > mapping.
> > > > >
> > > > > Link: https://github.com/riscv-software-src/opensbi/blob/master/docs/pmu_support.md
> > > > > Co-developed-by: Atish Patra <atishp@rivosinc.com>
> > > > > Signed-off-by: Atish Patra <atishp@rivosinc.com>
> > > > > Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> > > > > +description: |
> > > > > +  SBI PMU extension supports allow supervisor software to configure, start &
> > > > > +  stop any performance counter at anytime. Thus, a user can leverage full
> > > > > +  capability of performance analysis tools such as perf if the SBI PMU
> > > > > +  extension is enabled. The OpenSBI implementation makes the following
> > > > > +  assumptions about the hardware platform:
> > > > > +    MCOUNTINHIBIT CSR must be implemented in the hardware. Otherwise, the SBI
> > > > > +    PMU extension will not be enabled.
> > > > > +
> >
> > This is no longer true since OpenSBI commit b28f070. We should remove
> > this from OpenSBI docs as well.
>
> Since you know the detail of it, I volunteer you for that one ;)
>

Done.

> > > > > +    The platform must provide information about PMU event to counter mapping
> > > > > +    via device tree or platform specific hooks. Otherwise, the SBI PMU
> > > > > +    extension will not be enabled.
> > > > > +
> > > > > +    The platforms should provide information about the PMU event selector
> > > > > +    values that should be encoded in the expected value of MHPMEVENTx while
> > > > > +    configuring MHPMCOUNTERx for that specific event. This can be done via a
> > > > > +    device tree or platform specific hooks. The exact value to be written to
> > > > > +    the MHPMEVENTx is completely dependent on the platform. The generic
> > > > > +    platform writes the zero-extended event_idx as the expected value for
> > > > > +    hardware cache/generic events as suggested by the SBI specification.
> > > > > +
> >
> > But the larger question is these are OpenSBI implementation specific
> > assumptions. Should it be included in
> > the DT binding?
>
> Probably not. I'll drop it for v2.
>
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    const: riscv,pmu
> > > > > +
> > > > > +  riscv,event-to-mhpmevent:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > > +    description:
> > > > > +      Represents an ONE-to-ONE mapping between a PMU event and the event
> > > > > +      selector value that platform expects to be written to the MHPMEVENTx CSR
> > > > > +      for that event.
> > > > > +      The mapping is encoded in an array format where each row represents an
> > > >
> > > > s/array/matrix/
> > > >
> > > > > +      event. The first element represents the event idx while the second &
> > > > > +      third elements represent the event selector value that should be encoded
> > > > > +      in the expected value to be written in MHPMEVENTx.
> > > > > +      This property shouldn't encode any raw hardware event.
> > > > > +    minItems: 1
> > > > > +    maxItems: 255
> > >
> > > I copied these 255s from dt-schema somewhere as a sane max.
> > > Atish, is there a cap here or should we allow as many as someone likes?
> > > The md binding doesn't mention any limits - is it in the SBI spec?
> > > The strongest wording I saw there was "limited" & event idx is 20 bits
> > > wide as per the spec [0]. Does that make 2^20 the hard limit here?
> > >
> >
> > This is for hardware & cache events. The total number of events
> > defined there are 52 (10 HW + 42 Cache) events.
> > So 255 is a sane max that provides enough room for expansion in future
> > if required.
>
> WFM.
>
> > > > > +  riscv,raw-event-to-mhpmcounters:
> > > > > +    $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > > > +    description:
> > > > > +      Represents an ONE-to-MANY or MANY-to-MANY mapping between the rawevent(s)
> > > > > +      and all the MHPMCOUNTERx in a bitmap format that can be used to monitor
> > > > > +      that raw event.
> > > > > +      The encoding of the raw events are platform specific. The information is
> > > > > +      encoded in an array format where each row represents the specific raw
> > > > > +      event(s). The first element is a 64-bit match value where the invariant
> > > > > +      bits of range of events are set. The second element is a 64-bit mask that
> > > > > +      will have all the variant bits of the range of events cleared. All other
> > > > > +      bits should be set in the mask. The third element is a 32-bit value to
> > > > > +      represent bitmap of all MHPMCOUNTERx that can monitor these set of
> > > > > +      event(s). If a platform directly encodes each raw PMU event as a unique
> > > > > +      ID, the value of select_mask must be 0xffffffff_ffffffff.
> > > > > +    minItems: 1
> > > > > +    maxItems: 255
> > > > > +    items:
> > > > > +      $ref: /schemas/types.yaml#/definitions/uint32-array
> > > > > +      maxItems: 5
> > > > > +
> > > > > +required:
> > > > > +  - compatible
> > > >
> > > > I assume at least one of the other properties must be present?
> > >
> > > Atish: (and +CC the OpenSBI list)
> > > Which properties are actually needed here? Or are any? The md binding
> > > in the OpenSBI sources doesn't seem to require anything other than the
> > > compatible?
> > >
> >
> > That's true. Opensbi allows the platform to define functions
> > to provide the mapping. Now that's an OpenSBI implementation thing.
> > Other implementations may choose to use it differently.
>
> In the case where the platform can define functions, the "bare" node
> with just the compatible is still required though in that case, right?
>

Yup.

> Thanks,
> Conor.
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-12-22 20:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-21 14:15 [PATCH v1] dt-bindings: riscv: add SBI PMU event mappings Conor Dooley
2022-12-21 14:15 ` Conor Dooley
2022-12-22 18:06 ` Rob Herring
2022-12-22 18:06   ` Rob Herring
2022-12-22 19:23   ` Conor Dooley
2022-12-22 19:23     ` Conor Dooley
2022-12-22 19:55     ` Atish Kumar Patra
2022-12-22 19:55       ` Atish Kumar Patra
2022-12-22 20:16       ` Conor Dooley
2022-12-22 20:16         ` Conor Dooley
2022-12-22 20:39         ` Atish Kumar Patra
2022-12-22 20:39           ` Atish Kumar Patra

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.