Re: [kvm-unit-tests PATCH v3 12/13] x86/pmu: Add assignment framework for Intel-specific HW resources

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/10/2022 6:44 am, Sean Christopherson wrote:
On Fri, Aug 19, 2022, Like Xu wrote:
@@ -65,7 +65,13 @@ struct pmu_event {
  };
#define PMU_CAP_FW_WRITES (1ULL << 13)
-static u64 gp_counter_base = MSR_IA32_PERFCTR0;
+static u32 gp_counter_base;
+static u32 gp_select_base;
+static unsigned int gp_events_size;
+static unsigned int nr_gp_counters;
+
+typedef struct pmu_event PMU_EVENTS_ARRAY_t[];
+static PMU_EVENTS_ARRAY_t *gp_events = NULL;

There's no need for a layer of indirection, C d.  The NULL is also not strictly
required.  E.g. this should Just Work.

   static struct pmu_event *gp_events;

Applied.


And then I beleve a large number of changes in this series go away.

  char *buf;
@@ -114,9 +120,9 @@ static struct pmu_event* get_counter_event(pmu_counter_t *cnt)
  	if (is_gp(cnt)) {
  		int i;
- for (i = 0; i < sizeof(gp_events)/sizeof(gp_events[0]); i++)
-			if (gp_events[i].unit_sel == (cnt->config & 0xffff))
-				return &gp_events[i];
+		for (i = 0; i < gp_events_size; i++)
+			if ((*gp_events)[i].unit_sel == (cnt->config & 0xffff))
+				return &(*gp_events)[i];
  	} else
  		return &fixed_events[cnt->ctr - MSR_CORE_PERF_FIXED_CTR0];
@@ -142,12 +148,22 @@ static void global_disable(pmu_counter_t *cnt)
  			~(1ull << cnt->idx));
  }
+static inline uint32_t get_gp_counter_msr(unsigned int i)

Rather than helpers, what about macros?  The problem with "get" is that it sounds
like the helper is actually reading the counter/MSR.  E.g. see MSR_IA32_MCx_CTL()

Something like this?

   MSR_PERF_GP_CTRx()

The base address msr is different for intel and amd (including K7), and using different macros
in the same location sounds like it would require a helper.

This proposal will be dropped or need to be re-emphasised in the next version.


+{
+	return gp_counter_base + i;
+}
+
+static inline uint32_t get_gp_select_msr(unsigned int i)
+{
+	return gp_select_base + i;

Same here, maybe?

   MSR_PERF_GP_SELECTx()





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux