sbi: sbi_domain_context: Add 'domain context switched' firmware event

RFC PATCH New Yu Chien Peter Lin 3 participants 5 messages Jun 7, 2024 – Jun 25, 2024

Share & tools

Yu Chien Peter Lin

author

2024-06-07 11:49 UTC #0 lore raw

RFC PATCH 1/1 sbi: sbi_domain_context: Add 'domain context switched' firmware event

Event codes 256-65534 are reserved for counting SBI-specific firmware events. Allocate event code 256 as the 'domain context switched' firmware event. This enables the perf tool to monitor trusted application invocations.

  $ perf stat -e r8000000000000100 optee_example_random
  Invoking TA to generate random UUID...
  TA generated UUID value = 0xd1dafb5b988373c9a14d1a85361b436

Performance counter stats for 'optee_example_random':

                  96      r8000000000000100

         0.186135100 seconds time elapsed

         0.051499000 seconds user
         0.125070000 seconds sys

Also, factor out valid event code checking to the fw_event_code_valid() function.

Signed-off-by Yu Chien Peter Lin <[email protected]>

Reviewed-by Alvin Chang <[email protected]>

---
 include/sbi/sbi_ecall_interface.h |  9 +++++--
 lib/sbi/sbi_domain_context.c      |  3 +++
 lib/sbi/sbi_pmu.c                 | 43 +++++++++++++++++++------------
 3 files changed, 36 insertions(+), 19 deletions(-)

include/sbi/sbi_ecall_interface.h +7 -2

index ec845c88..aff9ec4c 100644
@@ -199,13 +199,18 @@ enum sbi_pmu_fw_event_code_id {
 	SBI_PMU_FW_HFENCE_VVMA_RCVD	= 19,
 	SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
 	SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
-	SBI_PMU_FW_MAX,
+	SBI_PMU_FW_LAST,
 	/*
 	 * Event codes 22 to 255 are reserved for future use.
+	 */
+	SBI_PMU_FW_MAX			= 255,
+	SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED = 256,
+	SBI_PMU_FW_IMPL_LAST,
+	/*
 	 * Event codes 256 to 65534 are reserved for SBI implementation
 	 * specific custom firmware events.
 	 */
-	SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+	SBI_PMU_FW_IMPL_MAX = 0xFFFE,
 	/*
 	 * Event code 0xFFFF is used for platform specific firmware
 	 * events where the event data contains any event specific information.

lib/sbi/sbi_domain_context.c +3 -0

index d9395490..d0b174c9 100755
@@ -11,6 +11,7 @@
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_heap.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain_context.h>

@@ -33,6 +34,8 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
 	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
 	unsigned int pmp_count = sbi_hart_pmp_count(scratch);
 
+	sbi_pmu_ctr_incr_fw(SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED);
+
 	/* Assign current hart to target domain */
 	spin_lock(&current_dom->assigned_harts_lock);
 	sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);

lib/sbi/sbi_pmu.c +26 ~~-17~~

92 lines not on this page · raw message

Signature · 1 line

2.34.1

Anup Patel

2024-06-13 13:33 UTC #1 lore raw

RFC PATCH 1/1 sbi: sbi_domain_context: Add 'domain context switched' firmware event

Quoted 41 lines · On Fri, Jun 7, 2024 at 5:19?PM Yu Chien Peter Lin <[email protected]>…

  $ perf stat -e r8000000000000100 optee_example_random
  Invoking TA to generate random UUID...
  TA generated UUID value = 0xd1dafb5b988373c9a14d1a85361b436

   Performance counter stats for 'optee_example_random':

                  96      r8000000000000100

         0.186135100 seconds time elapsed

         0.051499000 seconds user
         0.125070000 seconds sys

Also, factor out valid event code checking to the fw_event_code_valid() function.

Signed-off-by Yu Chien Peter Lin <[email protected]>

Reviewed-by Alvin Chang <[email protected]>

---
 include/sbi/sbi_ecall_interface.h |  9 +++++--
 lib/sbi/sbi_domain_context.c      |  3 +++
 lib/sbi/sbi_pmu.c                 | 43 +++++++++++++++++++------------
 3 files changed, 36 insertions(+), 19 deletions(-)

include/sbi/sbi_ecall_interface.h +1 -1

index ec845c88..aff9ec4c 100644
@@ -199,13 +199,18 @@ enum sbi_pmu_fw_event_code_id {
        SBI_PMU_FW_HFENCE_VVMA_RCVD     = 19,
        SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
        SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
-       SBI_PMU_FW_MAX,
+       SBI_PMU_FW_LAST,
        /*
         * Event codes 22 to 255 are reserved for future use.

include/sbi/sbi_ecall_interface.h enum sbi_pmu_fw_event_code_id { · line 213

Quoted 10 lines

include/sbi/sbi_ecall_interface.h +6 -1

+        */
+       SBI_PMU_FW_MAX                  = 255,
+       SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED = 256,
+       SBI_PMU_FW_IMPL_LAST,
+       /*
         * Event codes 256 to 65534 are reserved for SBI implementation
         * specific custom firmware events.
         */
-       SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+       SBI_PMU_FW_IMPL_MAX = 0xFFFE,

Need first discuss these SBI PMU spec changes in RVI PRS TG before we can review it here.

Regards,
Anup

Quoted 131 lines

include/sbi/sbi_ecall_interface.h +0 -0

        /*
         * Event code 0xFFFF is used for platform specific firmware
         * events where the event data contains any event specific information.

lib/sbi/sbi_domain_context.c +3 -0

index d9395490..d0b174c9 100755
@@ -11,6 +11,7 @@
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_heap.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain_context.h>

@@ -33,6 +34,8 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
        struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
        unsigned int pmp_count = sbi_hart_pmp_count(scratch);
 
+       sbi_pmu_ctr_incr_fw(SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED);
+
        /* Assign current hart to target domain */
        spin_lock(&current_dom->assigned_harts_lock);
        sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);

lib/sbi/sbi_pmu.c +26 ~~-18~~

index b9e84543..fd3e70f2 100644
@@ -130,6 +130,23 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
        return false;
 }
 
+static bool fw_event_code_valid(uint32_t event_idx_code)
+{
+       /* Standard firmware events */
+       if ((0 <= event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_LAST))
+               return true;
+       /* Implementation specific events */
+       if ((SBI_PMU_FW_MAX < event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_IMPL_LAST))
+               return true;
+       /* Platform specific firmware events */
+       if (event_idx_code == SBI_PMU_FW_PLATFORM)
+               return true;
+
+       return false;
+}
+
 static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                              unsigned long event_idx, uint64_t edata)
 {

@@ -143,9 +160,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_FW:
-               if ((event_idx_code >= SBI_PMU_FW_MAX &&
-                   event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
-                   event_idx_code > SBI_PMU_FW_PLATFORM)
+               if (!fw_event_code_valid(event_idx_code))
                        return SBI_EINVAL;
 
                if (SBI_PMU_FW_PLATFORM == event_idx_code &&

@@ -153,7 +168,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                        return pmu_dev->fw_event_validate_encoding(phs->hartid,
                                                                   edata);
                else
-                       event_idx_code_max = SBI_PMU_FW_MAX;
+                       event_idx_code_max = SBI_PMU_FW_IMPL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_HW_CACHE:
                cache_ops_result = event_idx_code &

@@ -217,9 +232,7 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
        if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
                return SBI_EINVAL;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -414,9 +427,7 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
                            uint64_t event_data, uint64_t ival,
                            bool ival_update)
 {
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -518,9 +529,7 @@ static int pmu_ctr_stop_fw(struct sbi_pmu_hart_state *phs,
 {
        int ret;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code &&

@@ -792,9 +801,7 @@ static int pmu_ctr_find_fw(struct sbi_pmu_hart_state *phs,
 {
        int i, cidx;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        for_each_set_bit(i, &cmask, BITS_PER_LONG) {

@@ -907,8 +914,10 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
        if (likely(!phs->fw_counters_started))
                return 0;
 
-       if (unlikely(fw_id >= SBI_PMU_FW_MAX))
+       if (unlikely((fw_id == SBI_PMU_FW_PLATFORM) ||
+                    !fw_event_code_valid(fw_id))) {
                return SBI_EINVAL;
+       }
 
        for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
                if (get_cidx_code(phs->active_events[cidx]) == fw_id &&
--

2.34.1

Anup Patel

2024-06-13 13:50 UTC #2 lore raw

RFC PATCH 1/1 sbi: sbi_domain_context: Add 'domain context switched' firmware event

+Atish

Quoted 49 lines · On Thu, Jun 13, 2024 at 7:03?PM Anup Patel <[email protected]> wrote:

Quoted 46 lines · On Fri, Jun 7, 2024 at 5:19?PM Yu Chien Peter Lin <[email protected]>…

  $ perf stat -e r8000000000000100 optee_example_random
  Invoking TA to generate random UUID...
  TA generated UUID value = 0xd1dafb5b988373c9a14d1a85361b436

   Performance counter stats for 'optee_example_random':

                  96      r8000000000000100

         0.186135100 seconds time elapsed

         0.051499000 seconds user
         0.125070000 seconds sys

Also, factor out valid event code checking to the fw_event_code_valid() function.

Signed-off-by Yu Chien Peter Lin <[email protected]>

Reviewed-by Alvin Chang <[email protected]>

---
 include/sbi/sbi_ecall_interface.h |  9 +++++--
 lib/sbi/sbi_domain_context.c      |  3 +++
 lib/sbi/sbi_pmu.c                 | 43 +++++++++++++++++++------------
 3 files changed, 36 insertions(+), 19 deletions(-)

include/sbi/sbi_ecall_interface.h +6 -1

index ec845c88..aff9ec4c 100644
@@ -199,13 +199,18 @@ enum sbi_pmu_fw_event_code_id {
        SBI_PMU_FW_HFENCE_VVMA_RCVD     = 19,
        SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
        SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
-       SBI_PMU_FW_MAX,
+       SBI_PMU_FW_LAST,
        /*
         * Event codes 22 to 255 are reserved for future use.
+        */
+       SBI_PMU_FW_MAX                  = 255,
+       SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED = 256,
+       SBI_PMU_FW_IMPL_LAST,
+       /*

Quoted 8 lines

include/sbi/sbi_ecall_interface.h enum sbi_pmu_fw_event_code_id { · line 213
Quoted 5 lines
include/sbi/sbi_ecall_interface.h +1 -1
         * Event codes 256 to 65534 are reserved for SBI implementation
         * specific custom firmware events.
         */
-       SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+       SBI_PMU_FW_IMPL_MAX = 0xFFFE,
Need first discuss these SBI PMU spec changes in RVI PRS TG before we can review it here.

Thinking about this more, do we really need this to be an OpenSBI specific event ?

I am leaning towards "yes" but need others to chime in.

Regards,
Anup

Quoted 135 lines

Regards,
Anup

Quoted 131 lines

include/sbi/sbi_ecall_interface.h +0 -0

        /*
         * Event code 0xFFFF is used for platform specific firmware
         * events where the event data contains any event specific information.

lib/sbi/sbi_domain_context.c +3 -0

index d9395490..d0b174c9 100755
@@ -11,6 +11,7 @@
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_heap.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain_context.h>

@@ -33,6 +34,8 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
        struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
        unsigned int pmp_count = sbi_hart_pmp_count(scratch);
 
+       sbi_pmu_ctr_incr_fw(SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED);
+
        /* Assign current hart to target domain */
        spin_lock(&current_dom->assigned_harts_lock);
        sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);

lib/sbi/sbi_pmu.c +26 ~~-18~~

index b9e84543..fd3e70f2 100644
@@ -130,6 +130,23 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
        return false;
 }
 
+static bool fw_event_code_valid(uint32_t event_idx_code)
+{
+       /* Standard firmware events */
+       if ((0 <= event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_LAST))
+               return true;
+       /* Implementation specific events */
+       if ((SBI_PMU_FW_MAX < event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_IMPL_LAST))
+               return true;
+       /* Platform specific firmware events */
+       if (event_idx_code == SBI_PMU_FW_PLATFORM)
+               return true;
+
+       return false;
+}
+
 static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                              unsigned long event_idx, uint64_t edata)
 {

@@ -143,9 +160,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_FW:
-               if ((event_idx_code >= SBI_PMU_FW_MAX &&
-                   event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
-                   event_idx_code > SBI_PMU_FW_PLATFORM)
+               if (!fw_event_code_valid(event_idx_code))
                        return SBI_EINVAL;
 
                if (SBI_PMU_FW_PLATFORM == event_idx_code &&

@@ -153,7 +168,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                        return pmu_dev->fw_event_validate_encoding(phs->hartid,
                                                                   edata);
                else
-                       event_idx_code_max = SBI_PMU_FW_MAX;
+                       event_idx_code_max = SBI_PMU_FW_IMPL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_HW_CACHE:
                cache_ops_result = event_idx_code &

@@ -217,9 +232,7 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
        if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
                return SBI_EINVAL;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -414,9 +427,7 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
                            uint64_t event_data, uint64_t ival,
                            bool ival_update)
 {
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -518,9 +529,7 @@ static int pmu_ctr_stop_fw(struct sbi_pmu_hart_state *phs,
 {
        int ret;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code &&

@@ -792,9 +801,7 @@ static int pmu_ctr_find_fw(struct sbi_pmu_hart_state *phs,
 {
        int i, cidx;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        for_each_set_bit(i, &cmask, BITS_PER_LONG) {

@@ -907,8 +914,10 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
        if (likely(!phs->fw_counters_started))
                return 0;
 
-       if (unlikely(fw_id >= SBI_PMU_FW_MAX))
+       if (unlikely((fw_id == SBI_PMU_FW_PLATFORM) ||
+                    !fw_event_code_valid(fw_id))) {
                return SBI_EINVAL;
+       }
 
        for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
                if (get_cidx_code(phs->active_events[cidx]) == fw_id &&
--

2.34.1

Atish Patra

2024-06-20 00:38 UTC #3 lore raw

RFC PATCH 1/1 sbi: sbi_domain_context: Add 'domain context switched' firmware event

Quoted 62 lines · On Thu, Jun 13, 2024 at 6:50?AM Anup Patel <[email protected]> wrote:

+Atish
Quoted 49 lines · On Thu, Jun 13, 2024 at 7:03?PM Anup Patel <[email protected]> wrote:
Quoted 46 lines · On Fri, Jun 7, 2024 at 5:19?PM Yu Chien Peter Lin <[email protected]>…
Event codes 256-65534 are reserved for counting SBI-specific firmware events. Allocate event code 256 as the 'domain context switched' firmware event. This enables the perf tool to monitor trusted application invocations.
  $ perf stat -e r8000000000000100 optee_example_random
  Invoking TA to generate random UUID...
  TA generated UUID value = 0xd1dafb5b988373c9a14d1a85361b436
   Performance counter stats for 'optee_example_random':

                  96      r8000000000000100

         0.186135100 seconds time elapsed

         0.051499000 seconds user
         0.125070000 seconds sys
Also, factor out valid event code checking to the fw_event_code_valid() function.
Signed-off-by Yu Chien Peter Lin <[email protected]>
Reviewed-by Alvin Chang <[email protected]>
---
 include/sbi/sbi_ecall_interface.h |  9 +++++--
 lib/sbi/sbi_domain_context.c      |  3 +++
 lib/sbi/sbi_pmu.c                 | 43 +++++++++++++++++++------------
 3 files changed, 36 insertions(+), 19 deletions(-)
include/sbi/sbi_ecall_interface.h +6 -1
index ec845c88..aff9ec4c 100644
@@ -199,13 +199,18 @@ enum sbi_pmu_fw_event_code_id {
        SBI_PMU_FW_HFENCE_VVMA_RCVD     = 19,
        SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
        SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
-       SBI_PMU_FW_MAX,
+       SBI_PMU_FW_LAST,
        /*
         * Event codes 22 to 255 are reserved for future use.
+        */
+       SBI_PMU_FW_MAX                  = 255,
+       SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED = 256,
+       SBI_PMU_FW_IMPL_LAST,
+       /*
Quoted 8 lines
include/sbi/sbi_ecall_interface.h enum sbi_pmu_fw_event_code_id { · line 213
Quoted 5 lines
include/sbi/sbi_ecall_interface.h +1 -1
         * Event codes 256 to 65534 are reserved for SBI implementation
         * specific custom firmware events.
         */
-       SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+       SBI_PMU_FW_IMPL_MAX = 0xFFFE,
Need first discuss these SBI PMU spec changes in RVI PRS TG before we can review it here.

Quoted 5 lines

Thinking about this more, do we really need this to be an OpenSBI specific event ?
I am leaning towards "yes" but need others to chime in.

I agree. I am not sure which other SBI implementations support domains anyways. If we decide to add it as an implementation specific event, should we add a scheme to encode implementation ID
in the event code ? The lower 8 bits can indicate the SBI implementation ID.

Quoted 138 lines

Regards,
Anup

Quoted 135 lines

Regards,
Anup

Quoted 131 lines

include/sbi/sbi_ecall_interface.h +0 -0

        /*
         * Event code 0xFFFF is used for platform specific firmware
         * events where the event data contains any event specific information.

lib/sbi/sbi_domain_context.c +3 -0

index d9395490..d0b174c9 100755
@@ -11,6 +11,7 @@
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_heap.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain_context.h>

@@ -33,6 +34,8 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
        struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
        unsigned int pmp_count = sbi_hart_pmp_count(scratch);
 
+       sbi_pmu_ctr_incr_fw(SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED);
+
        /* Assign current hart to target domain */
        spin_lock(&current_dom->assigned_harts_lock);
        sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);

lib/sbi/sbi_pmu.c +26 ~~-18~~

index b9e84543..fd3e70f2 100644
@@ -130,6 +130,23 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
        return false;
 }
 
+static bool fw_event_code_valid(uint32_t event_idx_code)
+{
+       /* Standard firmware events */
+       if ((0 <= event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_LAST))
+               return true;
+       /* Implementation specific events */
+       if ((SBI_PMU_FW_MAX < event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_IMPL_LAST))
+               return true;
+       /* Platform specific firmware events */
+       if (event_idx_code == SBI_PMU_FW_PLATFORM)
+               return true;
+
+       return false;
+}
+
 static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                              unsigned long event_idx, uint64_t edata)
 {

@@ -143,9 +160,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_FW:
-               if ((event_idx_code >= SBI_PMU_FW_MAX &&
-                   event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
-                   event_idx_code > SBI_PMU_FW_PLATFORM)
+               if (!fw_event_code_valid(event_idx_code))
                        return SBI_EINVAL;
 
                if (SBI_PMU_FW_PLATFORM == event_idx_code &&

@@ -153,7 +168,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                        return pmu_dev->fw_event_validate_encoding(phs->hartid,
                                                                   edata);
                else
-                       event_idx_code_max = SBI_PMU_FW_MAX;
+                       event_idx_code_max = SBI_PMU_FW_IMPL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_HW_CACHE:
                cache_ops_result = event_idx_code &

@@ -217,9 +232,7 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
        if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
                return SBI_EINVAL;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -414,9 +427,7 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
                            uint64_t event_data, uint64_t ival,
                            bool ival_update)
 {
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -518,9 +529,7 @@ static int pmu_ctr_stop_fw(struct sbi_pmu_hart_state *phs,
 {
        int ret;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code &&

@@ -792,9 +801,7 @@ static int pmu_ctr_find_fw(struct sbi_pmu_hart_state *phs,
 {
        int i, cidx;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        for_each_set_bit(i, &cmask, BITS_PER_LONG) {

@@ -907,8 +914,10 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
        if (likely(!phs->fw_counters_started))
                return 0;
 
-       if (unlikely(fw_id >= SBI_PMU_FW_MAX))
+       if (unlikely((fw_id == SBI_PMU_FW_PLATFORM) ||
+                    !fw_event_code_valid(fw_id))) {
                return SBI_EINVAL;
+       }
 
        for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
                if (get_cidx_code(phs->active_events[cidx]) == fw_id &&
--

2.34.1

Signature · 2 lines

Regards,
Atish

Yu-Chien Peter Lin

author

2024-06-25 03:16 UTC #4 lore raw

RFC PATCH 1/1 sbi: sbi_domain_context: Add 'domain context switched' firmware event

Hi Atish,

Quoted 68 lines · Atish Patra

[EXTERNAL MAIL]
Quoted 65 lines · On Thu, Jun 13, 2024 at 6:50?AM Anup Patel <[email protected]> wrote:
+Atish
Quoted 49 lines · On Thu, Jun 13, 2024 at 7:03?PM Anup Patel <[email protected]> wrote:
Quoted 46 lines · On Fri, Jun 7, 2024 at 5:19?PM Yu Chien Peter Lin <[email protected]>…
Event codes 256-65534 are reserved for counting SBI-specific firmware events. Allocate event code 256 as the 'domain context switched' firmware event. This enables the perf tool to monitor trusted application invocations.
  $ perf stat -e r8000000000000100 optee_example_random
  Invoking TA to generate random UUID...
  TA generated UUID value = 0xd1dafb5b988373c9a14d1a85361b436
   Performance counter stats for 'optee_example_random':

                  96      r8000000000000100

         0.186135100 seconds time elapsed

         0.051499000 seconds user
         0.125070000 seconds sys
Also, factor out valid event code checking to the fw_event_code_valid() function.
Signed-off-by Yu Chien Peter Lin <[email protected]>
Reviewed-by Alvin Chang <[email protected]>
---
 include/sbi/sbi_ecall_interface.h |  9 +++++--
 lib/sbi/sbi_domain_context.c      |  3 +++
 lib/sbi/sbi_pmu.c                 | 43 +++++++++++++++++++------------
 3 files changed, 36 insertions(+), 19 deletions(-)
include/sbi/sbi_ecall_interface.h +6 -1
index ec845c88..aff9ec4c 100644
@@ -199,13 +199,18 @@ enum sbi_pmu_fw_event_code_id {
        SBI_PMU_FW_HFENCE_VVMA_RCVD     = 19,
        SBI_PMU_FW_HFENCE_VVMA_ASID_SENT = 20,
        SBI_PMU_FW_HFENCE_VVMA_ASID_RCVD = 21,
-       SBI_PMU_FW_MAX,
+       SBI_PMU_FW_LAST,
        /*
         * Event codes 22 to 255 are reserved for future use.
+        */
+       SBI_PMU_FW_MAX                  = 255,
+       SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED = 256,
+       SBI_PMU_FW_IMPL_LAST,
+       /*
Quoted 8 lines
include/sbi/sbi_ecall_interface.h enum sbi_pmu_fw_event_code_id { · line 213
Quoted 5 lines
include/sbi/sbi_ecall_interface.h +1 -1
         * Event codes 256 to 65534 are reserved for SBI implementation
         * specific custom firmware events.
         */
-       SBI_PMU_FW_RESERVED_MAX = 0xFFFE,
+       SBI_PMU_FW_IMPL_MAX = 0xFFFE,
Need first discuss these SBI PMU spec changes in RVI PRS TG before we can review it here.
Thinking about this more, do we really need this to be an OpenSBI specific event ?

Quoted 7 lines

Quoted 2 lines
I am leaning towards "yes" but need others to chime in.
I agree. I am not sure which other SBI implementations support domains anyways. If we decide to add it as an implementation specific event, should we add a scheme to encode implementation ID
in the event code ? The lower 8 bits can indicate the SBI implementation ID.

How about using upper 8 bits for implementation ID. Except that the last SBI implementation (impl. ID#254) has only 255 events, each one has lower 8 bits available to encode 256 events.

event_idx.code[15:8]: impl. ID + 1
event_idx.code[ 7:0]: event ID

              0000 0000 | 1111 1111 |   255 | Standard FW event#255
===============================================================================
impl. ID#0    0000 0001 | 0000 0000 |   256 | BBL-specific FW event#0
(256 events)  0000 0001 | 0000 0001 |   257 | BBL-specific FW event#1
              ...
              0000 0001 | 1111 1111 |   511 | BBL-specific FW event#255
-------------------------------------------------------------------------------
impl. ID#1    0000 0010 | 0000 0000 |   512 | OpenSBI-specific FW event#0
(256 events)  0000 0010 | 0000 0001 |   513 | OpenSBI-specific FW event#1
              ...
              0000 0010 | 1111 1111 |   767 | OpenSBI-specific FW event#255
-------------------------------------------------------------------------------
...
-------------------------------------------------------------------------------
impl. ID#253  1111 1110 | 0000 0000 | 65024 | XXX-specific FW event#0
(256 events)  1111 1110 | 0000 0001 | 65025 | XXX-specific FW event#1
              ...
              1111 1110 | 1111 1111 | 65279 | XXX-specific FW event#255
-------------------------------------------------------------------------------
impl. ID#254  1111 1111 | 0000 0000 | 65280 | XXX-specific FW event#0
(255 events)  1111 1111 | 0000 0001 | 65281 | XXX-specific FW event#1
              ...
              1111 1111 | 1111 1110 | 65534 | XXX-specific FW event#255
===============================================================================
              1111 1111 | 1111 1111 | 65535 | RISC-V platform specific FW event

Given that there are only 9 SBI implementations at this point, I'm not sure if 8-bit field for the SBI impl. ID is a waste of bits.

Thoughts on this approach?

Thanks,
Peter Lin

Quoted 144 lines

Quoted 133 lines

Regards,
Anup

Quoted 130 lines

Regards,
Anup

Quoted 126 lines

include/sbi/sbi_ecall_interface.h +0 -0

        /*
         * Event code 0xFFFF is used for platform specific firmware
         * events where the event data contains any event specific information.

lib/sbi/sbi_domain_context.c +3 -0

index d9395490..d0b174c9 100755
@@ -11,6 +11,7 @@
 #include <sbi/sbi_hsm.h>
 #include <sbi/sbi_hart.h>
 #include <sbi/sbi_heap.h>
+#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_scratch.h>
 #include <sbi/sbi_string.h>
 #include <sbi/sbi_domain_context.h>

@@ -33,6 +34,8 @@ static void switch_to_next_domain_context(struct sbi_context *ctx,
        struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
        unsigned int pmp_count = sbi_hart_pmp_count(scratch);
 
+       sbi_pmu_ctr_incr_fw(SBI_PMU_FW_DOMAIN_CONTEXT_SWITCHED);
+
        /* Assign current hart to target domain */
        spin_lock(&current_dom->assigned_harts_lock);
        sbi_hartmask_clear_hartindex(hartindex, &current_dom->assigned_harts);

lib/sbi/sbi_pmu.c +26 ~~-17~~

index b9e84543..fd3e70f2 100644
@@ -130,6 +130,23 @@ static bool pmu_event_select_overlap(struct sbi_pmu_hw_event *evt,
        return false;
 }
 
+static bool fw_event_code_valid(uint32_t event_idx_code)
+{
+       /* Standard firmware events */
+       if ((0 <= event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_LAST))
+               return true;
+       /* Implementation specific events */
+       if ((SBI_PMU_FW_MAX < event_idx_code) &&
+           (event_idx_code < SBI_PMU_FW_IMPL_LAST))
+               return true;
+       /* Platform specific firmware events */
+       if (event_idx_code == SBI_PMU_FW_PLATFORM)
+               return true;
+
+       return false;
+}
+
 static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                              unsigned long event_idx, uint64_t edata)
 {

@@ -143,9 +160,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                event_idx_code_max = SBI_PMU_HW_GENERAL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_FW:
-               if ((event_idx_code >= SBI_PMU_FW_MAX &&
-                   event_idx_code <= SBI_PMU_FW_RESERVED_MAX) ||
-                   event_idx_code > SBI_PMU_FW_PLATFORM)
+               if (!fw_event_code_valid(event_idx_code))
                        return SBI_EINVAL;
 
                if (SBI_PMU_FW_PLATFORM == event_idx_code &&

@@ -153,7 +168,7 @@ static int pmu_event_validate(struct sbi_pmu_hart_state *phs,
                        return pmu_dev->fw_event_validate_encoding(phs->hartid,
                                                                   edata);
                else
-                       event_idx_code_max = SBI_PMU_FW_MAX;
+                       event_idx_code_max = SBI_PMU_FW_IMPL_MAX;
                break;
        case SBI_PMU_EVENT_TYPE_HW_CACHE:
                cache_ops_result = event_idx_code &

@@ -217,9 +232,7 @@ int sbi_pmu_ctr_fw_read(uint32_t cidx, uint64_t *cval)
        if (event_idx_type != SBI_PMU_EVENT_TYPE_FW)
                return SBI_EINVAL;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -414,9 +427,7 @@ static int pmu_ctr_start_fw(struct sbi_pmu_hart_state *phs,
                            uint64_t event_data, uint64_t ival,
                            bool ival_update)
 {
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code) {

@@ -518,9 +529,7 @@ static int pmu_ctr_stop_fw(struct sbi_pmu_hart_state *phs,
 {
        int ret;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        if (SBI_PMU_FW_PLATFORM == event_code &&

@@ -792,9 +801,7 @@ static int pmu_ctr_find_fw(struct sbi_pmu_hart_state *phs,
 {
        int i, cidx;
 
-       if ((event_code >= SBI_PMU_FW_MAX &&
-           event_code <= SBI_PMU_FW_RESERVED_MAX) ||
-           event_code > SBI_PMU_FW_PLATFORM)
+       if (!fw_event_code_valid(event_code))
                return SBI_EINVAL;
 
        for_each_set_bit(i, &cmask, BITS_PER_LONG) {

@@ -907,8 +914,10 @@ int sbi_pmu_ctr_incr_fw(enum sbi_pmu_fw_event_code_id fw_id)
        if (likely(!phs->fw_counters_started))
                return 0;
 
-       if (unlikely(fw_id >= SBI_PMU_FW_MAX))
+       if (unlikely((fw_id == SBI_PMU_FW_PLATFORM) ||
+                    !fw_event_code_valid(fw_id))) {
                return SBI_EINVAL;
+       }

Quoted 5 lines

lib/sbi/sbi_pmu.c +0 -1

        for (cidx = num_hw_ctrs; cidx < total_ctrs; cidx++) {
                if (get_cidx_code(phs->active_events[cidx]) == fw_id &&
--

2.34.1

--
Regards,
Atish