linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] perf/x86/intel: Constify static structs
@ 2021-06-05 15:56 Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops Rikard Falkeborn
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-05 15:56 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Kan Liang, Alexander Antonov, Rikard Falkeborn

Constify a number of static structs which are not modified to allow the
compiler to place them in read-only memory. All in all, it moves ~25kB
to read-only memory (if looking at o-files).

Rikard Falkeborn (4):
  perf/x86/intel/uncore: Constify intel_uncore_ops
  perf/x86/intel: Constify static attribute_group structs
  perf/x86/intel/uncore: Constify freerunning_counters
  perf/x86/intel/uncore: Constify unmodified static extra_reg structs

 arch/x86/events/intel/core.c             |   8 +-
 arch/x86/events/intel/cstate.c           |  10 +--
 arch/x86/events/intel/pt.c               |   4 +-
 arch/x86/events/intel/uncore.h           |   6 +-
 arch/x86/events/intel/uncore_discovery.c |   6 +-
 arch/x86/events/intel/uncore_nhmex.c     |  16 ++--
 arch/x86/events/intel/uncore_snb.c       |  20 ++---
 arch/x86/events/intel/uncore_snbep.c     | 102 +++++++++++------------
 8 files changed, 86 insertions(+), 86 deletions(-)

-- 
2.31.1


^ permalink raw reply	[flat|nested] 8+ messages in thread
* [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops
  2021-06-05 15:56 [PATCH 0/4] perf/x86/intel: Constify static structs Rikard Falkeborn
@ 2021-06-05 15:56 ` Rikard Falkeborn
  2021-06-07 19:09   ` Liang, Kan
  2021-06-05 15:56 ` [PATCH 2/4] perf/x86/intel: Constify static attribute_group structs Rikard Falkeborn
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-05 15:56 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Kan Liang, Alexander Antonov, Rikard Falkeborn

These are not modified, so make them const to allow the compiler to put
them in read-only memory.

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 arch/x86/events/intel/uncore.h           |  2 +-
 arch/x86/events/intel/uncore_discovery.c |  6 +--
 arch/x86/events/intel/uncore_nhmex.c     | 10 ++--
 arch/x86/events/intel/uncore_snb.c       | 12 ++---
 arch/x86/events/intel/uncore_snbep.c     | 66 ++++++++++++------------
 5 files changed, 48 insertions(+), 48 deletions(-)

diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
index 187d7287039c..83b25a7b8c27 100644
--- a/arch/x86/events/intel/uncore.h
+++ b/arch/x86/events/intel/uncore.h
@@ -77,7 +77,7 @@ struct intel_uncore_type {
 	struct event_constraint unconstrainted;
 	struct event_constraint *constraints;
 	struct intel_uncore_pmu *pmus;
-	struct intel_uncore_ops *ops;
+	const struct intel_uncore_ops *ops;
 	struct uncore_event_desc *event_descs;
 	struct freerunning_counters *freerunning;
 	const struct attribute_group *attr_groups[4];
diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
index aba9bff95413..2b8e9a1d793e 100644
--- a/arch/x86/events/intel/uncore_discovery.c
+++ b/arch/x86/events/intel/uncore_discovery.c
@@ -368,7 +368,7 @@ static void intel_generic_uncore_msr_disable_event(struct intel_uncore_box *box,
 	wrmsrl(hwc->config_base, 0);
 }
 
-static struct intel_uncore_ops generic_uncore_msr_ops = {
+static const struct intel_uncore_ops generic_uncore_msr_ops = {
 	.init_box		= intel_generic_uncore_msr_init_box,
 	.disable_box		= intel_generic_uncore_msr_disable_box,
 	.enable_box		= intel_generic_uncore_msr_enable_box,
@@ -433,7 +433,7 @@ static u64 intel_generic_uncore_pci_read_counter(struct intel_uncore_box *box,
 	return count;
 }
 
-static struct intel_uncore_ops generic_uncore_pci_ops = {
+static const struct intel_uncore_ops generic_uncore_pci_ops = {
 	.init_box	= intel_generic_uncore_pci_init_box,
 	.disable_box	= intel_generic_uncore_pci_disable_box,
 	.enable_box	= intel_generic_uncore_pci_enable_box,
@@ -516,7 +516,7 @@ static void intel_generic_uncore_mmio_disable_event(struct intel_uncore_box *box
 	writel(0, box->io_addr + hwc->config_base);
 }
 
-static struct intel_uncore_ops generic_uncore_mmio_ops = {
+static const struct intel_uncore_ops generic_uncore_mmio_ops = {
 	.init_box	= intel_generic_uncore_mmio_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.disable_box	= intel_generic_uncore_mmio_disable_box,
diff --git a/arch/x86/events/intel/uncore_nhmex.c b/arch/x86/events/intel/uncore_nhmex.c
index 173e2674be6e..5f7c27d7f428 100644
--- a/arch/x86/events/intel/uncore_nhmex.c
+++ b/arch/x86/events/intel/uncore_nhmex.c
@@ -262,7 +262,7 @@ static void nhmex_uncore_msr_enable_event(struct intel_uncore_box *box, struct p
 	.disable_event	= nhmex_uncore_msr_disable_event,	\
 	.read_counter	= uncore_msr_read_counter
 
-static struct intel_uncore_ops nhmex_uncore_ops = {
+static const struct intel_uncore_ops nhmex_uncore_ops = {
 	NHMEX_UNCORE_OPS_COMMON_INIT(),
 	.enable_event	= nhmex_uncore_msr_enable_event,
 };
@@ -413,7 +413,7 @@ static const struct attribute_group nhmex_uncore_bbox_format_group = {
 	.attrs = nhmex_uncore_bbox_formats_attr,
 };
 
-static struct intel_uncore_ops nhmex_uncore_bbox_ops = {
+static const struct intel_uncore_ops nhmex_uncore_bbox_ops = {
 	NHMEX_UNCORE_OPS_COMMON_INIT(),
 	.enable_event		= nhmex_bbox_msr_enable_event,
 	.hw_config		= nhmex_bbox_hw_config,
@@ -490,7 +490,7 @@ static const struct attribute_group nhmex_uncore_sbox_format_group = {
 	.attrs			= nhmex_uncore_sbox_formats_attr,
 };
 
-static struct intel_uncore_ops nhmex_uncore_sbox_ops = {
+static const struct intel_uncore_ops nhmex_uncore_sbox_ops = {
 	NHMEX_UNCORE_OPS_COMMON_INIT(),
 	.enable_event		= nhmex_sbox_msr_enable_event,
 	.hw_config		= nhmex_sbox_hw_config,
@@ -916,7 +916,7 @@ static struct uncore_event_desc wsmex_uncore_mbox_events[] = {
 	{ /* end: all zeroes */ },
 };
 
-static struct intel_uncore_ops nhmex_uncore_mbox_ops = {
+static const struct intel_uncore_ops nhmex_uncore_mbox_ops = {
 	NHMEX_UNCORE_OPS_COMMON_INIT(),
 	.enable_event	= nhmex_mbox_msr_enable_event,
 	.hw_config	= nhmex_mbox_hw_config,
@@ -1179,7 +1179,7 @@ static struct uncore_event_desc nhmex_uncore_rbox_events[] = {
 	{ /* end: all zeroes */ },
 };
 
-static struct intel_uncore_ops nhmex_uncore_rbox_ops = {
+static const struct intel_uncore_ops nhmex_uncore_rbox_ops = {
 	NHMEX_UNCORE_OPS_COMMON_INIT(),
 	.enable_event		= nhmex_rbox_msr_enable_event,
 	.hw_config		= nhmex_rbox_hw_config,
diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
index 0f63706cdadf..5826cff568f4 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -216,7 +216,7 @@ static const struct attribute_group snb_uncore_format_group = {
 	.attrs		= snb_uncore_formats_attr,
 };
 
-static struct intel_uncore_ops snb_uncore_msr_ops = {
+static const struct intel_uncore_ops snb_uncore_msr_ops = {
 	.init_box	= snb_uncore_msr_init_box,
 	.enable_box	= snb_uncore_msr_enable_box,
 	.exit_box	= snb_uncore_msr_exit_box,
@@ -341,7 +341,7 @@ void skl_uncore_cpu_init(void)
 	snb_uncore_arb.ops = &skl_uncore_msr_ops;
 }
 
-static struct intel_uncore_ops icl_uncore_msr_ops = {
+static const struct intel_uncore_ops icl_uncore_msr_ops = {
 	.disable_event	= snb_uncore_msr_disable_event,
 	.enable_event	= snb_uncore_msr_enable_event,
 	.read_counter	= uncore_msr_read_counter,
@@ -468,7 +468,7 @@ static void adl_uncore_msr_exit_box(struct intel_uncore_box *box)
 		wrmsrl(ADL_UNC_PERF_GLOBAL_CTL, 0);
 }
 
-static struct intel_uncore_ops adl_uncore_msr_ops = {
+static const struct intel_uncore_ops adl_uncore_msr_ops = {
 	.init_box	= adl_uncore_msr_init_box,
 	.enable_box	= adl_uncore_msr_enable_box,
 	.disable_box	= adl_uncore_msr_disable_box,
@@ -799,7 +799,7 @@ static struct pmu snb_uncore_imc_pmu = {
 	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
 };
 
-static struct intel_uncore_ops snb_uncore_imc_ops = {
+static const struct intel_uncore_ops snb_uncore_imc_ops = {
 	.init_box	= snb_uncore_imc_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.enable_box	= snb_uncore_imc_enable_box,
@@ -1267,7 +1267,7 @@ static struct uncore_event_desc nhm_uncore_events[] = {
 	{ /* end: all zeroes */ },
 };
 
-static struct intel_uncore_ops nhm_uncore_msr_ops = {
+static const struct intel_uncore_ops nhm_uncore_msr_ops = {
 	.disable_box	= nhm_uncore_msr_disable_box,
 	.enable_box	= nhm_uncore_msr_enable_box,
 	.disable_event	= snb_uncore_msr_disable_event,
@@ -1422,7 +1422,7 @@ static void tgl_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
 		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
 }
 
-static struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
+static const struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
 	.init_box	= tgl_uncore_imc_freerunning_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.read_counter	= uncore_mmio_read_counter,
diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index bb6eb1e5569c..43eabe8d37dc 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -775,7 +775,7 @@ static const struct attribute_group snbep_uncore_qpi_format_group = {
 	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),			\
 	.init_box	= snbep_uncore_msr_init_box		\
 
-static struct intel_uncore_ops snbep_uncore_msr_ops = {
+static const struct intel_uncore_ops snbep_uncore_msr_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };
 
@@ -786,7 +786,7 @@ static struct intel_uncore_ops snbep_uncore_msr_ops = {
 	.disable_event	= snbep_uncore_pci_disable_event,	\
 	.read_counter	= snbep_uncore_pci_read_counter
 
-static struct intel_uncore_ops snbep_uncore_pci_ops = {
+static const struct intel_uncore_ops snbep_uncore_pci_ops = {
 	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
 	.enable_event	= snbep_uncore_pci_enable_event,	\
 };
@@ -1017,7 +1017,7 @@ static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event
 	return 0;
 }
 
-static struct intel_uncore_ops snbep_uncore_cbox_ops = {
+static const struct intel_uncore_ops snbep_uncore_cbox_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= snbep_cbox_hw_config,
 	.get_constraint		= snbep_cbox_get_constraint,
@@ -1127,7 +1127,7 @@ static int snbep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
 	return 0;
 }
 
-static struct intel_uncore_ops snbep_uncore_pcu_ops = {
+static const struct intel_uncore_ops snbep_uncore_pcu_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= snbep_pcu_hw_config,
 	.get_constraint		= snbep_pcu_get_constraint,
@@ -1211,7 +1211,7 @@ static void snbep_qpi_enable_event(struct intel_uncore_box *box, struct perf_eve
 	pci_write_config_dword(pdev, hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops snbep_uncore_qpi_ops = {
+static const struct intel_uncore_ops snbep_uncore_qpi_ops = {
 	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
 	.enable_event		= snbep_qpi_enable_event,
 	.hw_config		= snbep_qpi_hw_config,
@@ -1517,11 +1517,11 @@ static void ivbep_uncore_pci_init_box(struct intel_uncore_box *box)
 	.enable_event	= snbep_uncore_msr_enable_event,	\
 	.read_counter	= uncore_msr_read_counter
 
-static struct intel_uncore_ops ivbep_uncore_msr_ops = {
+static const struct intel_uncore_ops ivbep_uncore_msr_ops = {
 	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 };
 
-static struct intel_uncore_ops ivbep_uncore_pci_ops = {
+static const struct intel_uncore_ops ivbep_uncore_pci_ops = {
 	.init_box	= ivbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -1757,7 +1757,7 @@ static void ivbep_cbox_enable_event(struct intel_uncore_box *box, struct perf_ev
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops ivbep_uncore_cbox_ops = {
+static const struct intel_uncore_ops ivbep_uncore_cbox_ops = {
 	.init_box		= ivbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -1785,7 +1785,7 @@ static struct intel_uncore_type ivbep_uncore_cbox = {
 	.format_group		= &ivbep_uncore_cbox_format_group,
 };
 
-static struct intel_uncore_ops ivbep_uncore_pcu_ops = {
+static const struct intel_uncore_ops ivbep_uncore_pcu_ops = {
 	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= snbep_pcu_hw_config,
 	.get_constraint		= snbep_pcu_get_constraint,
@@ -1873,7 +1873,7 @@ static u64 ivbep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
 	return count;
 }
 
-static struct intel_uncore_ops ivbep_uncore_irp_ops = {
+static const struct intel_uncore_ops ivbep_uncore_irp_ops = {
 	.init_box	= ivbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -1893,7 +1893,7 @@ static struct intel_uncore_type ivbep_uncore_irp = {
 	.format_group		= &ivbep_uncore_format_group,
 };
 
-static struct intel_uncore_ops ivbep_uncore_qpi_ops = {
+static const struct intel_uncore_ops ivbep_uncore_qpi_ops = {
 	.init_box	= ivbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -2178,7 +2178,7 @@ static int knl_cha_hw_config(struct intel_uncore_box *box,
 static void hswep_cbox_enable_event(struct intel_uncore_box *box,
 				    struct perf_event *event);
 
-static struct intel_uncore_ops knl_uncore_cha_ops = {
+static const struct intel_uncore_ops knl_uncore_cha_ops = {
 	.init_box		= snbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -2272,7 +2272,7 @@ static void knl_uncore_imc_enable_event(struct intel_uncore_box *box,
 				       hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops knl_uncore_imc_ops = {
+static const struct intel_uncore_ops knl_uncore_imc_ops = {
 	.init_box	= snbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= knl_uncore_imc_enable_box,
@@ -2581,7 +2581,7 @@ static int hswep_ubox_hw_config(struct intel_uncore_box *box, struct perf_event
 	return 0;
 }
 
-static struct intel_uncore_ops hswep_uncore_ubox_ops = {
+static const struct intel_uncore_ops hswep_uncore_ubox_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= hswep_ubox_hw_config,
 	.get_constraint		= uncore_get_constraint,
@@ -2741,7 +2741,7 @@ static void hswep_cbox_enable_event(struct intel_uncore_box *box,
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops hswep_uncore_cbox_ops = {
+static const struct intel_uncore_ops hswep_uncore_cbox_ops = {
 	.init_box		= snbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -2788,7 +2788,7 @@ static void hswep_uncore_sbox_msr_init_box(struct intel_uncore_box *box)
 	}
 }
 
-static struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
+static const struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
 	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.init_box		= hswep_uncore_sbox_msr_init_box
 };
@@ -2836,7 +2836,7 @@ static int hswep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
 	return 0;
 }
 
-static struct intel_uncore_ops hswep_uncore_pcu_ops = {
+static const struct intel_uncore_ops hswep_uncore_pcu_ops = {
 	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= hswep_pcu_hw_config,
 	.get_constraint		= snbep_pcu_get_constraint,
@@ -2941,7 +2941,7 @@ static u64 hswep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
 	return count;
 }
 
-static struct intel_uncore_ops hswep_uncore_irp_ops = {
+static const struct intel_uncore_ops hswep_uncore_irp_ops = {
 	.init_box	= snbep_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -3611,7 +3611,7 @@ static int skx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
 	return 0;
 }
 
-static struct intel_uncore_ops skx_uncore_chabox_ops = {
+static const struct intel_uncore_ops skx_uncore_chabox_ops = {
 	/* There is no frz_en for chabox ctl */
 	.init_box		= ivbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
@@ -3673,7 +3673,7 @@ static void skx_iio_enable_event(struct intel_uncore_box *box,
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops skx_uncore_iio_ops = {
+static const struct intel_uncore_ops skx_uncore_iio_ops = {
 	.init_box		= ivbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -3931,7 +3931,7 @@ static struct uncore_event_desc skx_uncore_iio_freerunning_events[] = {
 	{ /* end: all zeroes */ },
 };
 
-static struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
+static const struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
 	.read_counter		= uncore_msr_read_counter,
 	.hw_config		= uncore_freerunning_hw_config,
 };
@@ -4006,7 +4006,7 @@ static struct attribute_group skx_uncore_pcu_format_group = {
 	.attrs = skx_uncore_pcu_formats_attr,
 };
 
-static struct intel_uncore_ops skx_uncore_pcu_ops = {
+static const struct intel_uncore_ops skx_uncore_pcu_ops = {
 	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= hswep_pcu_hw_config,
 	.get_constraint		= snbep_pcu_get_constraint,
@@ -4105,7 +4105,7 @@ static void skx_upi_uncore_pci_init_box(struct intel_uncore_box *box)
 	pci_write_config_dword(pdev, SKX_UPI_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
 }
 
-static struct intel_uncore_ops skx_upi_uncore_pci_ops = {
+static const struct intel_uncore_ops skx_upi_uncore_pci_ops = {
 	.init_box	= skx_upi_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -4136,7 +4136,7 @@ static void skx_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
 	pci_write_config_dword(pdev, SKX_M2M_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
 }
 
-static struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
+static const struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
 	.init_box	= skx_m2m_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -4373,7 +4373,7 @@ static void snr_cha_enable_event(struct intel_uncore_box *box,
 	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
 }
 
-static struct intel_uncore_ops snr_uncore_chabox_ops = {
+static const struct intel_uncore_ops snr_uncore_chabox_ops = {
 	.init_box		= ivbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -4560,7 +4560,7 @@ static int snr_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *ev
 	return 0;
 }
 
-static struct intel_uncore_ops snr_uncore_pcu_ops = {
+static const struct intel_uncore_ops snr_uncore_pcu_ops = {
 	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
 	.hw_config		= snr_pcu_hw_config,
 	.get_constraint		= snbep_pcu_get_constraint,
@@ -4660,7 +4660,7 @@ static void snr_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
 	pci_write_config_dword(pdev, box_ctl, IVBEP_PMON_BOX_CTL_INT);
 }
 
-static struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
+static const struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
 	.init_box	= snr_m2m_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -4706,7 +4706,7 @@ static void snr_uncore_pci_enable_event(struct intel_uncore_box *box, struct per
 	pci_write_config_dword(pdev, hwc->config_base + 4, (u32)(hwc->config >> 32));
 }
 
-static struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
+static const struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
 	.init_box	= snr_m2m_uncore_pci_init_box,
 	.disable_box	= snbep_uncore_pci_disable_box,
 	.enable_box	= snbep_uncore_pci_enable_box,
@@ -4884,7 +4884,7 @@ static void snr_uncore_mmio_disable_event(struct intel_uncore_box *box,
 	writel(hwc->config, box->io_addr + hwc->config_base);
 }
 
-static struct intel_uncore_ops snr_uncore_mmio_ops = {
+static const struct intel_uncore_ops snr_uncore_mmio_ops = {
 	.init_box	= snr_uncore_mmio_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.disable_box	= snr_uncore_mmio_disable_box,
@@ -4948,7 +4948,7 @@ static struct uncore_event_desc snr_uncore_imc_freerunning_events[] = {
 	{ /* end: all zeroes */ },
 };
 
-static struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
+static const struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
 	.init_box	= snr_uncore_mmio_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.read_counter	= uncore_mmio_read_counter,
@@ -5005,7 +5005,7 @@ static int icx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
 	return 0;
 }
 
-static struct intel_uncore_ops icx_uncore_chabox_ops = {
+static const struct intel_uncore_ops icx_uncore_chabox_ops = {
 	.init_box		= ivbep_uncore_msr_init_box,
 	.disable_box		= snbep_uncore_msr_disable_box,
 	.enable_box		= snbep_uncore_msr_enable_box,
@@ -5406,7 +5406,7 @@ static void icx_uncore_imc_init_box(struct intel_uncore_box *box)
 	__snr_uncore_mmio_init_box(box, box_ctl, mem_offset);
 }
 
-static struct intel_uncore_ops icx_uncore_mmio_ops = {
+static const struct intel_uncore_ops icx_uncore_mmio_ops = {
 	.init_box	= icx_uncore_imc_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.disable_box	= snr_uncore_mmio_disable_box,
@@ -5476,7 +5476,7 @@ static void icx_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
 	__snr_uncore_mmio_init_box(box, uncore_mmio_box_ctl(box), mem_offset);
 }
 
-static struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
+static const struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
 	.init_box	= icx_uncore_imc_freerunning_init_box,
 	.exit_box	= uncore_mmio_exit_box,
 	.read_counter	= uncore_mmio_read_counter,
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 2/4] perf/x86/intel: Constify static attribute_group structs
  2021-06-05 15:56 [PATCH 0/4] perf/x86/intel: Constify static structs Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops Rikard Falkeborn
@ 2021-06-05 15:56 ` Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 3/4] perf/x86/intel/uncore: Constify freerunning_counters Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 4/4] perf/x86/intel/uncore: Constify unmodified static extra_reg structs Rikard Falkeborn
  3 siblings, 0 replies; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-05 15:56 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Kan Liang, Alexander Antonov, Rikard Falkeborn

These either have their address put in an array of pointers to const
attribute structs, or (in uncore_snb.c and uncore_snbep.c) have their
address assigned to at pointer to const field in the intel_uncore_type
struct.

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 arch/x86/events/intel/core.c         |  8 ++++----
 arch/x86/events/intel/cstate.c       | 10 +++++-----
 arch/x86/events/intel/pt.c           |  4 ++--
 arch/x86/events/intel/uncore_snb.c   |  2 +-
 arch/x86/events/intel/uncore_snbep.c |  2 +-
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index e28892270c58..b07d832f304e 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -5164,12 +5164,12 @@ static struct attribute_group group_events_tsx = {
 	.is_visible = tsx_is_visible,
 };
 
-static struct attribute_group group_caps_gen = {
+static const struct attribute_group group_caps_gen = {
 	.name  = "caps",
 	.attrs = intel_pmu_caps_attrs,
 };
 
-static struct attribute_group group_caps_lbr = {
+static const struct attribute_group group_caps_lbr = {
 	.name       = "caps",
 	.attrs	    = lbr_attrs,
 	.is_visible = lbr_is_visible,
@@ -5185,7 +5185,7 @@ static struct attribute_group group_format_extra_skl = {
 	.is_visible = exra_is_visible,
 };
 
-static struct attribute_group group_default = {
+static const struct attribute_group group_default = {
 	.attrs      = intel_pmu_attrs,
 	.is_visible = default_is_visible,
 };
@@ -5364,7 +5364,7 @@ static struct attribute *intel_hybrid_cpus_attrs[] = {
 	NULL,
 };
 
-static struct attribute_group hybrid_group_cpus = {
+static const struct attribute_group hybrid_group_cpus = {
 	.attrs		= intel_hybrid_cpus_attrs,
 };
 
diff --git a/arch/x86/events/intel/cstate.c b/arch/x86/events/intel/cstate.c
index 433399069e27..089f71ac83f5 100644
--- a/arch/x86/events/intel/cstate.c
+++ b/arch/x86/events/intel/cstate.c
@@ -187,7 +187,7 @@ static struct attribute *attrs_empty[] = {
  * "events" group (with empty attrs) before updating
  * it with detected events.
  */
-static struct attribute_group core_events_attr_group = {
+static const struct attribute_group core_events_attr_group = {
 	.name = "events",
 	.attrs = attrs_empty,
 };
@@ -198,7 +198,7 @@ static struct attribute *core_format_attrs[] = {
 	NULL,
 };
 
-static struct attribute_group core_format_attr_group = {
+static const struct attribute_group core_format_attr_group = {
 	.name = "format",
 	.attrs = core_format_attrs,
 };
@@ -211,7 +211,7 @@ static struct attribute *cstate_cpumask_attrs[] = {
 	NULL,
 };
 
-static struct attribute_group cpumask_attr_group = {
+static const struct attribute_group cpumask_attr_group = {
 	.attrs = cstate_cpumask_attrs,
 };
 
@@ -266,7 +266,7 @@ static struct perf_msr pkg_msr[] = {
 	[PERF_CSTATE_PKG_C10_RES] = { MSR_PKG_C10_RESIDENCY,	&group_cstate_pkg_c10,	test_msr },
 };
 
-static struct attribute_group pkg_events_attr_group = {
+static const struct attribute_group pkg_events_attr_group = {
 	.name = "events",
 	.attrs = attrs_empty,
 };
@@ -276,7 +276,7 @@ static struct attribute *pkg_format_attrs[] = {
 	&format_attr_pkg_event.attr,
 	NULL,
 };
-static struct attribute_group pkg_format_attr_group = {
+static const struct attribute_group pkg_format_attr_group = {
 	.name = "format",
 	.attrs = pkg_format_attrs,
 };
diff --git a/arch/x86/events/intel/pt.c b/arch/x86/events/intel/pt.c
index 915847655c06..83ccf106ed13 100644
--- a/arch/x86/events/intel/pt.c
+++ b/arch/x86/events/intel/pt.c
@@ -128,7 +128,7 @@ static struct attribute *pt_formats_attr[] = {
 	NULL,
 };
 
-static struct attribute_group pt_format_group = {
+static const struct attribute_group pt_format_group = {
 	.name	= "format",
 	.attrs	= pt_formats_attr,
 };
@@ -165,7 +165,7 @@ static struct attribute *pt_timing_attr[] = {
 	NULL,
 };
 
-static struct attribute_group pt_timing_group = {
+static const struct attribute_group pt_timing_group = {
 	.attrs	= pt_timing_attr,
 };
 
diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
index 5826cff568f4..3eff6f1a5b99 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -369,7 +369,7 @@ static struct attribute *icl_uncore_clock_formats_attr[] = {
 	NULL,
 };
 
-static struct attribute_group icl_uncore_clock_format_group = {
+static const struct attribute_group icl_uncore_clock_format_group = {
 	.name = "format",
 	.attrs = icl_uncore_clock_formats_attr,
 };
diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index 43eabe8d37dc..b5b22fe473d8 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -4001,7 +4001,7 @@ static struct attribute *skx_uncore_pcu_formats_attr[] = {
 	NULL,
 };
 
-static struct attribute_group skx_uncore_pcu_format_group = {
+static const struct attribute_group skx_uncore_pcu_format_group = {
 	.name = "format",
 	.attrs = skx_uncore_pcu_formats_attr,
 };
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 3/4] perf/x86/intel/uncore: Constify freerunning_counters
  2021-06-05 15:56 [PATCH 0/4] perf/x86/intel: Constify static structs Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 2/4] perf/x86/intel: Constify static attribute_group structs Rikard Falkeborn
@ 2021-06-05 15:56 ` Rikard Falkeborn
  2021-06-05 15:56 ` [PATCH 4/4] perf/x86/intel/uncore: Constify unmodified static extra_reg structs Rikard Falkeborn
  3 siblings, 0 replies; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-05 15:56 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Kan Liang, Alexander Antonov, Rikard Falkeborn

These are never modified, so make them const to allow the compiler to
put them in read-only memory.

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 arch/x86/events/intel/uncore.h       |  4 ++--
 arch/x86/events/intel/uncore_snb.c   |  6 +++---
 arch/x86/events/intel/uncore_snbep.c | 14 +++++++-------
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
index 83b25a7b8c27..6a7f0104bb38 100644
--- a/arch/x86/events/intel/uncore.h
+++ b/arch/x86/events/intel/uncore.h
@@ -79,7 +79,7 @@ struct intel_uncore_type {
 	struct intel_uncore_pmu *pmus;
 	const struct intel_uncore_ops *ops;
 	struct uncore_event_desc *event_descs;
-	struct freerunning_counters *freerunning;
+	const struct freerunning_counters *freerunning;
 	const struct attribute_group *attr_groups[4];
 	const struct attribute_group **attr_update;
 	struct pmu *pmu; /* for custom pmu ops */
@@ -175,7 +175,7 @@ struct freerunning_counters {
 	unsigned int box_offset;
 	unsigned int num_counters;
 	unsigned int bits;
-	unsigned *box_offsets;
+	const unsigned *box_offsets;
 };
 
 struct intel_uncore_topology {
diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
index 3eff6f1a5b99..475e48b7a686 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -603,7 +603,7 @@ enum perf_snb_uncore_imc_freerunning_types {
 	SNB_PCI_UNCORE_IMC_FREERUNNING_TYPE_MAX,
 };
 
-static struct freerunning_counters snb_uncore_imc_freerunning[] = {
+static const struct freerunning_counters snb_uncore_imc_freerunning[] = {
 	[SNB_PCI_UNCORE_IMC_DATA_READS]		= { SNB_UNCORE_PCI_IMC_DATA_READS_BASE,
 							0x0, 0x0, 1, 32 },
 	[SNB_PCI_UNCORE_IMC_DATA_WRITES]	= { SNB_UNCORE_PCI_IMC_DATA_WRITES_BASE,
@@ -1344,13 +1344,13 @@ enum perf_tgl_uncore_imc_freerunning_types {
 	TGL_MMIO_UNCORE_IMC_FREERUNNING_TYPE_MAX
 };
 
-static struct freerunning_counters tgl_l_uncore_imc_freerunning[] = {
+static const struct freerunning_counters tgl_l_uncore_imc_freerunning[] = {
 	[TGL_MMIO_UNCORE_IMC_DATA_TOTAL]	= { 0x5040, 0x0, 0x0, 1, 64 },
 	[TGL_MMIO_UNCORE_IMC_DATA_READ]		= { 0x5058, 0x0, 0x0, 1, 64 },
 	[TGL_MMIO_UNCORE_IMC_DATA_WRITE]	= { 0x50A0, 0x0, 0x0, 1, 64 },
 };
 
-static struct freerunning_counters tgl_uncore_imc_freerunning[] = {
+static const struct freerunning_counters tgl_uncore_imc_freerunning[] = {
 	[TGL_MMIO_UNCORE_IMC_DATA_TOTAL]	= { 0xd840, 0x0, 0x0, 1, 64 },
 	[TGL_MMIO_UNCORE_IMC_DATA_READ]		= { 0xd858, 0x0, 0x0, 1, 64 },
 	[TGL_MMIO_UNCORE_IMC_DATA_WRITE]	= { 0xd8A0, 0x0, 0x0, 1, 64 },
diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index b5b22fe473d8..507bb83e1463 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -3885,7 +3885,7 @@ enum perf_uncore_iio_freerunning_type_id {
 };
 
 
-static struct freerunning_counters skx_iio_freerunning[] = {
+static const struct freerunning_counters skx_iio_freerunning[] = {
 	[SKX_IIO_MSR_IOCLK]	= { 0xa45, 0x1, 0x20, 1, 36 },
 	[SKX_IIO_MSR_BW]	= { 0xb00, 0x1, 0x10, 8, 36 },
 	[SKX_IIO_MSR_UTIL]	= { 0xb08, 0x1, 0x10, 8, 36 },
@@ -4588,7 +4588,7 @@ enum perf_uncore_snr_iio_freerunning_type_id {
 	SNR_IIO_FREERUNNING_TYPE_MAX,
 };
 
-static struct freerunning_counters snr_iio_freerunning[] = {
+static const struct freerunning_counters snr_iio_freerunning[] = {
 	[SNR_IIO_MSR_IOCLK]	= { 0x1eac, 0x1, 0x10, 1, 48 },
 	[SNR_IIO_MSR_BW_IN]	= { 0x1f00, 0x1, 0x10, 8, 48 },
 };
@@ -4931,7 +4931,7 @@ enum perf_uncore_snr_imc_freerunning_type_id {
 	SNR_IMC_FREERUNNING_TYPE_MAX,
 };
 
-static struct freerunning_counters snr_imc_freerunning[] = {
+static const struct freerunning_counters snr_imc_freerunning[] = {
 	[SNR_IMC_DCLK]	= { 0x22b0, 0x0, 0, 1, 48 },
 	[SNR_IMC_DDR]	= { 0x2290, 0x8, 0, 2, 48 },
 };
@@ -5153,15 +5153,15 @@ enum perf_uncore_icx_iio_freerunning_type_id {
 	ICX_IIO_FREERUNNING_TYPE_MAX,
 };
 
-static unsigned icx_iio_clk_freerunning_box_offsets[] = {
+static const unsigned icx_iio_clk_freerunning_box_offsets[] = {
 	0x0, 0x20, 0x40, 0x90, 0xb0, 0xd0,
 };
 
-static unsigned icx_iio_bw_freerunning_box_offsets[] = {
+static const unsigned icx_iio_bw_freerunning_box_offsets[] = {
 	0x0, 0x10, 0x20, 0x90, 0xa0, 0xb0,
 };
 
-static struct freerunning_counters icx_iio_freerunning[] = {
+static const struct freerunning_counters icx_iio_freerunning[] = {
 	[ICX_IIO_MSR_IOCLK]	= { 0xa55, 0x1, 0x20, 1, 48, icx_iio_clk_freerunning_box_offsets },
 	[ICX_IIO_MSR_BW_IN]	= { 0xaa0, 0x1, 0x10, 8, 48, icx_iio_bw_freerunning_box_offsets },
 };
@@ -5443,7 +5443,7 @@ enum perf_uncore_icx_imc_freerunning_type_id {
 	ICX_IMC_FREERUNNING_TYPE_MAX,
 };
 
-static struct freerunning_counters icx_imc_freerunning[] = {
+static const struct freerunning_counters icx_imc_freerunning[] = {
 	[ICX_IMC_DCLK]	= { 0x22b0, 0x0, 0, 1, 48 },
 	[ICX_IMC_DDR]	= { 0x2290, 0x8, 0, 2, 48 },
 	[ICX_IMC_DDRT]	= { 0x22a0, 0x8, 0, 2, 48 },
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* [PATCH 4/4] perf/x86/intel/uncore: Constify unmodified static extra_reg structs
  2021-06-05 15:56 [PATCH 0/4] perf/x86/intel: Constify static structs Rikard Falkeborn
                   ` (2 preceding siblings ...)
  2021-06-05 15:56 ` [PATCH 3/4] perf/x86/intel/uncore: Constify freerunning_counters Rikard Falkeborn
@ 2021-06-05 15:56 ` Rikard Falkeborn
  3 siblings, 0 replies; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-05 15:56 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Kan Liang, Alexander Antonov, Rikard Falkeborn

These are never modified, so make them const to allow the compiler to
put them in read-only memory.

Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
---
 arch/x86/events/intel/uncore_nhmex.c |  6 +++---
 arch/x86/events/intel/uncore_snbep.c | 20 ++++++++++----------
 2 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/arch/x86/events/intel/uncore_nhmex.c b/arch/x86/events/intel/uncore_nhmex.c
index 5f7c27d7f428..b4567489580d 100644
--- a/arch/x86/events/intel/uncore_nhmex.c
+++ b/arch/x86/events/intel/uncore_nhmex.c
@@ -525,7 +525,7 @@ enum {
 	EXTRA_REG_NHMEX_M_ZDP_CTL_FVC,
 };
 
-static struct extra_reg nhmex_uncore_mbox_extra_regs[] = {
+static const struct extra_reg nhmex_uncore_mbox_extra_regs[] = {
 	MBOX_INC_SEL_EXTAR_REG(0x0, DSP),
 	MBOX_INC_SEL_EXTAR_REG(0x4, MSC_THR),
 	MBOX_INC_SEL_EXTAR_REG(0x5, MSC_THR),
@@ -755,7 +755,7 @@ static void nhmex_mbox_put_constraint(struct intel_uncore_box *box, struct perf_
 	}
 }
 
-static int nhmex_mbox_extra_reg_idx(struct extra_reg *er)
+static int nhmex_mbox_extra_reg_idx(const struct extra_reg *er)
 {
 	if (er->idx < EXTRA_REG_NHMEX_M_ZDP_CTL_FVC)
 		return er->idx;
@@ -767,7 +767,7 @@ static int nhmex_mbox_hw_config(struct intel_uncore_box *box, struct perf_event
 	struct intel_uncore_type *type = box->pmu->type;
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
 	struct hw_perf_event_extra *reg2 = &event->hw.branch_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	unsigned msr;
 	int reg_idx = 0;
 	/*
diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
index 507bb83e1463..77b342637688 100644
--- a/arch/x86/events/intel/uncore_snbep.c
+++ b/arch/x86/events/intel/uncore_snbep.c
@@ -882,7 +882,7 @@ static struct intel_uncore_type snbep_uncore_ubox = {
 	.format_group	= &snbep_uncore_ubox_format_group,
 };
 
-static struct extra_reg snbep_uncore_cbox_extra_regs[] = {
+static const struct extra_reg snbep_uncore_cbox_extra_regs[] = {
 	SNBEP_CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
 				  SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x0334, 0xffff, 0x4),
@@ -999,7 +999,7 @@ snbep_cbox_get_constraint(struct intel_uncore_box *box, struct perf_event *event
 static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	int idx = 0;
 
 	for (er = snbep_uncore_cbox_extra_regs; er->msr; er++) {
@@ -1653,7 +1653,7 @@ static struct intel_uncore_type ivbep_uncore_ubox = {
 	.format_group	= &ivbep_uncore_ubox_format_group,
 };
 
-static struct extra_reg ivbep_uncore_cbox_extra_regs[] = {
+static const struct extra_reg ivbep_uncore_cbox_extra_regs[] = {
 	SNBEP_CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
 				  SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x1031, 0x10ff, 0x2),
@@ -1725,7 +1725,7 @@ ivbep_cbox_get_constraint(struct intel_uncore_box *box, struct perf_event *event
 static int ivbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	int idx = 0;
 
 	for (er = ivbep_uncore_cbox_extra_regs; er->msr; er++) {
@@ -2121,7 +2121,7 @@ static struct event_constraint knl_uncore_cha_constraints[] = {
 	EVENT_CONSTRAINT_END
 };
 
-static struct extra_reg knl_uncore_cha_extra_regs[] = {
+static const struct extra_reg knl_uncore_cha_extra_regs[] = {
 	SNBEP_CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
 				  SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x3d, 0xff, 0x2),
@@ -2153,7 +2153,7 @@ static int knl_cha_hw_config(struct intel_uncore_box *box,
 			     struct perf_event *event)
 {
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	int idx = 0;
 
 	for (er = knl_uncore_cha_extra_regs; er->msr; er++) {
@@ -2637,7 +2637,7 @@ static struct event_constraint hswep_uncore_cbox_constraints[] = {
 	EVENT_CONSTRAINT_END
 };
 
-static struct extra_reg hswep_uncore_cbox_extra_regs[] = {
+static const struct extra_reg hswep_uncore_cbox_extra_regs[] = {
 	SNBEP_CBO_EVENT_EXTRA_REG(SNBEP_CBO_PMON_CTL_TID_EN,
 				  SNBEP_CBO_PMON_CTL_TID_EN, 0x1),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x0334, 0xffff, 0x4),
@@ -2708,7 +2708,7 @@ hswep_cbox_get_constraint(struct intel_uncore_box *box, struct perf_event *event
 static int hswep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	int idx = 0;
 
 	for (er = hswep_uncore_cbox_extra_regs; er->msr; er++) {
@@ -3547,7 +3547,7 @@ static struct event_constraint skx_uncore_chabox_constraints[] = {
 	EVENT_CONSTRAINT_END
 };
 
-static struct extra_reg skx_uncore_cha_extra_regs[] = {
+static const struct extra_reg skx_uncore_cha_extra_regs[] = {
 	SNBEP_CBO_EVENT_EXTRA_REG(0x0334, 0xffff, 0x4),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x0534, 0xffff, 0x4),
 	SNBEP_CBO_EVENT_EXTRA_REG(0x0934, 0xffff, 0x4),
@@ -3593,7 +3593,7 @@ skx_cha_get_constraint(struct intel_uncore_box *box, struct perf_event *event)
 static int skx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *event)
 {
 	struct hw_perf_event_extra *reg1 = &event->hw.extra_reg;
-	struct extra_reg *er;
+	const struct extra_reg *er;
 	int idx = 0;
 
 	for (er = skx_uncore_cha_extra_regs; er->msr; er++) {
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops
  2021-06-05 15:56 ` [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops Rikard Falkeborn
@ 2021-06-07 19:09   ` Liang, Kan
  2021-06-08 19:17     ` Rikard Falkeborn
  0 siblings, 1 reply; 8+ messages in thread
From: Liang, Kan @ 2021-06-07 19:09 UTC (permalink / raw)
  To: Rikard Falkeborn, Peter Zijlstra
  Cc: Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, Thomas Gleixner,
	Borislav Petkov, x86, H. Peter Anvin, linux-perf-users,
	linux-kernel, Alexander Antonov



On 6/5/2021 11:56 AM, Rikard Falkeborn wrote:
> These are not modified, so make them const to allow the compiler to put
> them in read-only memory.

For most of the cases, yes, but the ops are modified for the TGL/RKL. We 
may want to create a read-only ops for the TGL/RKL as below, and make 
both skl_uncore_msr_ops and rkl_uncore_msr_ops const as well.


diff --git a/arch/x86/events/intel/uncore_snb.c 
b/arch/x86/events/intel/uncore_snb.c
index 0f63706..e750514 100644
--- a/arch/x86/events/intel/uncore_snb.c
+++ b/arch/x86/events/intel/uncore_snb.c
@@ -435,14 +435,22 @@ static void rkl_uncore_msr_init_box(struct 
intel_uncore_box *box)
  		wrmsrl(SKL_UNC_PERF_GLOBAL_CTL, SNB_UNC_GLOBAL_CTL_EN);
  }

+static struct intel_uncore_ops rkl_uncore_msr_ops = {
+	.init_box	= rkl_uncore_msr_init_box,
+	.enable_box	= skl_uncore_msr_enable_box,
+	.exit_box	= skl_uncore_msr_exit_box,
+	.disable_event	= snb_uncore_msr_disable_event,
+	.enable_event	= snb_uncore_msr_enable_event,
+	.read_counter	= uncore_msr_read_counter,
+};
+
  void tgl_uncore_cpu_init(void)
  {
  	uncore_msr_uncores = tgl_msr_uncores;
  	icl_uncore_cbox.num_boxes = icl_get_cbox_num();
-	icl_uncore_cbox.ops = &skl_uncore_msr_ops;
-	icl_uncore_clockbox.ops = &skl_uncore_msr_ops;
-	snb_uncore_arb.ops = &skl_uncore_msr_ops;
-	skl_uncore_msr_ops.init_box = rkl_uncore_msr_init_box;
+	icl_uncore_cbox.ops = &rkl_uncore_msr_ops;
+	icl_uncore_clockbox.ops = &rkl_uncore_msr_ops;
+	snb_uncore_arb.ops = &rkl_uncore_msr_ops;
  }

  static void adl_uncore_msr_init_box(struct intel_uncore_box *box)

Thanks,
Kan


> 
> Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> ---
>   arch/x86/events/intel/uncore.h           |  2 +-
>   arch/x86/events/intel/uncore_discovery.c |  6 +--
>   arch/x86/events/intel/uncore_nhmex.c     | 10 ++--
>   arch/x86/events/intel/uncore_snb.c       | 12 ++---
>   arch/x86/events/intel/uncore_snbep.c     | 66 ++++++++++++------------
>   5 files changed, 48 insertions(+), 48 deletions(-)
> 
> diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
> index 187d7287039c..83b25a7b8c27 100644
> --- a/arch/x86/events/intel/uncore.h
> +++ b/arch/x86/events/intel/uncore.h
> @@ -77,7 +77,7 @@ struct intel_uncore_type {
>   	struct event_constraint unconstrainted;
>   	struct event_constraint *constraints;
>   	struct intel_uncore_pmu *pmus;
> -	struct intel_uncore_ops *ops;
> +	const struct intel_uncore_ops *ops;
>   	struct uncore_event_desc *event_descs;
>   	struct freerunning_counters *freerunning;
>   	const struct attribute_group *attr_groups[4];
> diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
> index aba9bff95413..2b8e9a1d793e 100644
> --- a/arch/x86/events/intel/uncore_discovery.c
> +++ b/arch/x86/events/intel/uncore_discovery.c
> @@ -368,7 +368,7 @@ static void intel_generic_uncore_msr_disable_event(struct intel_uncore_box *box,
>   	wrmsrl(hwc->config_base, 0);
>   }
>   
> -static struct intel_uncore_ops generic_uncore_msr_ops = {
> +static const struct intel_uncore_ops generic_uncore_msr_ops = {
>   	.init_box		= intel_generic_uncore_msr_init_box,
>   	.disable_box		= intel_generic_uncore_msr_disable_box,
>   	.enable_box		= intel_generic_uncore_msr_enable_box,
> @@ -433,7 +433,7 @@ static u64 intel_generic_uncore_pci_read_counter(struct intel_uncore_box *box,
>   	return count;
>   }
>   
> -static struct intel_uncore_ops generic_uncore_pci_ops = {
> +static const struct intel_uncore_ops generic_uncore_pci_ops = {
>   	.init_box	= intel_generic_uncore_pci_init_box,
>   	.disable_box	= intel_generic_uncore_pci_disable_box,
>   	.enable_box	= intel_generic_uncore_pci_enable_box,
> @@ -516,7 +516,7 @@ static void intel_generic_uncore_mmio_disable_event(struct intel_uncore_box *box
>   	writel(0, box->io_addr + hwc->config_base);
>   }
>   
> -static struct intel_uncore_ops generic_uncore_mmio_ops = {
> +static const struct intel_uncore_ops generic_uncore_mmio_ops = {
>   	.init_box	= intel_generic_uncore_mmio_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.disable_box	= intel_generic_uncore_mmio_disable_box,
> diff --git a/arch/x86/events/intel/uncore_nhmex.c b/arch/x86/events/intel/uncore_nhmex.c
> index 173e2674be6e..5f7c27d7f428 100644
> --- a/arch/x86/events/intel/uncore_nhmex.c
> +++ b/arch/x86/events/intel/uncore_nhmex.c
> @@ -262,7 +262,7 @@ static void nhmex_uncore_msr_enable_event(struct intel_uncore_box *box, struct p
>   	.disable_event	= nhmex_uncore_msr_disable_event,	\
>   	.read_counter	= uncore_msr_read_counter
>   
> -static struct intel_uncore_ops nhmex_uncore_ops = {
> +static const struct intel_uncore_ops nhmex_uncore_ops = {
>   	NHMEX_UNCORE_OPS_COMMON_INIT(),
>   	.enable_event	= nhmex_uncore_msr_enable_event,
>   };
> @@ -413,7 +413,7 @@ static const struct attribute_group nhmex_uncore_bbox_format_group = {
>   	.attrs = nhmex_uncore_bbox_formats_attr,
>   };
>   
> -static struct intel_uncore_ops nhmex_uncore_bbox_ops = {
> +static const struct intel_uncore_ops nhmex_uncore_bbox_ops = {
>   	NHMEX_UNCORE_OPS_COMMON_INIT(),
>   	.enable_event		= nhmex_bbox_msr_enable_event,
>   	.hw_config		= nhmex_bbox_hw_config,
> @@ -490,7 +490,7 @@ static const struct attribute_group nhmex_uncore_sbox_format_group = {
>   	.attrs			= nhmex_uncore_sbox_formats_attr,
>   };
>   
> -static struct intel_uncore_ops nhmex_uncore_sbox_ops = {
> +static const struct intel_uncore_ops nhmex_uncore_sbox_ops = {
>   	NHMEX_UNCORE_OPS_COMMON_INIT(),
>   	.enable_event		= nhmex_sbox_msr_enable_event,
>   	.hw_config		= nhmex_sbox_hw_config,
> @@ -916,7 +916,7 @@ static struct uncore_event_desc wsmex_uncore_mbox_events[] = {
>   	{ /* end: all zeroes */ },
>   };
>   
> -static struct intel_uncore_ops nhmex_uncore_mbox_ops = {
> +static const struct intel_uncore_ops nhmex_uncore_mbox_ops = {
>   	NHMEX_UNCORE_OPS_COMMON_INIT(),
>   	.enable_event	= nhmex_mbox_msr_enable_event,
>   	.hw_config	= nhmex_mbox_hw_config,
> @@ -1179,7 +1179,7 @@ static struct uncore_event_desc nhmex_uncore_rbox_events[] = {
>   	{ /* end: all zeroes */ },
>   };
>   
> -static struct intel_uncore_ops nhmex_uncore_rbox_ops = {
> +static const struct intel_uncore_ops nhmex_uncore_rbox_ops = {
>   	NHMEX_UNCORE_OPS_COMMON_INIT(),
>   	.enable_event		= nhmex_rbox_msr_enable_event,
>   	.hw_config		= nhmex_rbox_hw_config,
> diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
> index 0f63706cdadf..5826cff568f4 100644
> --- a/arch/x86/events/intel/uncore_snb.c
> +++ b/arch/x86/events/intel/uncore_snb.c
> @@ -216,7 +216,7 @@ static const struct attribute_group snb_uncore_format_group = {
>   	.attrs		= snb_uncore_formats_attr,
>   };
>   
> -static struct intel_uncore_ops snb_uncore_msr_ops = {
> +static const struct intel_uncore_ops snb_uncore_msr_ops = {
>   	.init_box	= snb_uncore_msr_init_box,
>   	.enable_box	= snb_uncore_msr_enable_box,
>   	.exit_box	= snb_uncore_msr_exit_box,
> @@ -341,7 +341,7 @@ void skl_uncore_cpu_init(void)
>   	snb_uncore_arb.ops = &skl_uncore_msr_ops;
>   }
>   
> -static struct intel_uncore_ops icl_uncore_msr_ops = {
> +static const struct intel_uncore_ops icl_uncore_msr_ops = {
>   	.disable_event	= snb_uncore_msr_disable_event,
>   	.enable_event	= snb_uncore_msr_enable_event,
>   	.read_counter	= uncore_msr_read_counter,
> @@ -468,7 +468,7 @@ static void adl_uncore_msr_exit_box(struct intel_uncore_box *box)
>   		wrmsrl(ADL_UNC_PERF_GLOBAL_CTL, 0);
>   }
>   
> -static struct intel_uncore_ops adl_uncore_msr_ops = {
> +static const struct intel_uncore_ops adl_uncore_msr_ops = {
>   	.init_box	= adl_uncore_msr_init_box,
>   	.enable_box	= adl_uncore_msr_enable_box,
>   	.disable_box	= adl_uncore_msr_disable_box,
> @@ -799,7 +799,7 @@ static struct pmu snb_uncore_imc_pmu = {
>   	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
>   };
>   
> -static struct intel_uncore_ops snb_uncore_imc_ops = {
> +static const struct intel_uncore_ops snb_uncore_imc_ops = {
>   	.init_box	= snb_uncore_imc_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.enable_box	= snb_uncore_imc_enable_box,
> @@ -1267,7 +1267,7 @@ static struct uncore_event_desc nhm_uncore_events[] = {
>   	{ /* end: all zeroes */ },
>   };
>   
> -static struct intel_uncore_ops nhm_uncore_msr_ops = {
> +static const struct intel_uncore_ops nhm_uncore_msr_ops = {
>   	.disable_box	= nhm_uncore_msr_disable_box,
>   	.enable_box	= nhm_uncore_msr_enable_box,
>   	.disable_event	= snb_uncore_msr_disable_event,
> @@ -1422,7 +1422,7 @@ static void tgl_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
>   		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
>   }
>   
> -static struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
> +static const struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
>   	.init_box	= tgl_uncore_imc_freerunning_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.read_counter	= uncore_mmio_read_counter,
> diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> index bb6eb1e5569c..43eabe8d37dc 100644
> --- a/arch/x86/events/intel/uncore_snbep.c
> +++ b/arch/x86/events/intel/uncore_snbep.c
> @@ -775,7 +775,7 @@ static const struct attribute_group snbep_uncore_qpi_format_group = {
>   	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),			\
>   	.init_box	= snbep_uncore_msr_init_box		\
>   
> -static struct intel_uncore_ops snbep_uncore_msr_ops = {
> +static const struct intel_uncore_ops snbep_uncore_msr_ops = {
>   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   };
>   
> @@ -786,7 +786,7 @@ static struct intel_uncore_ops snbep_uncore_msr_ops = {
>   	.disable_event	= snbep_uncore_pci_disable_event,	\
>   	.read_counter	= snbep_uncore_pci_read_counter
>   
> -static struct intel_uncore_ops snbep_uncore_pci_ops = {
> +static const struct intel_uncore_ops snbep_uncore_pci_ops = {
>   	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
>   	.enable_event	= snbep_uncore_pci_enable_event,	\
>   };
> @@ -1017,7 +1017,7 @@ static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops snbep_uncore_cbox_ops = {
> +static const struct intel_uncore_ops snbep_uncore_cbox_ops = {
>   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= snbep_cbox_hw_config,
>   	.get_constraint		= snbep_cbox_get_constraint,
> @@ -1127,7 +1127,7 @@ static int snbep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops snbep_uncore_pcu_ops = {
> +static const struct intel_uncore_ops snbep_uncore_pcu_ops = {
>   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= snbep_pcu_hw_config,
>   	.get_constraint		= snbep_pcu_get_constraint,
> @@ -1211,7 +1211,7 @@ static void snbep_qpi_enable_event(struct intel_uncore_box *box, struct perf_eve
>   	pci_write_config_dword(pdev, hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops snbep_uncore_qpi_ops = {
> +static const struct intel_uncore_ops snbep_uncore_qpi_ops = {
>   	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
>   	.enable_event		= snbep_qpi_enable_event,
>   	.hw_config		= snbep_qpi_hw_config,
> @@ -1517,11 +1517,11 @@ static void ivbep_uncore_pci_init_box(struct intel_uncore_box *box)
>   	.enable_event	= snbep_uncore_msr_enable_event,	\
>   	.read_counter	= uncore_msr_read_counter
>   
> -static struct intel_uncore_ops ivbep_uncore_msr_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_msr_ops = {
>   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   };
>   
> -static struct intel_uncore_ops ivbep_uncore_pci_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_pci_ops = {
>   	.init_box	= ivbep_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -1757,7 +1757,7 @@ static void ivbep_cbox_enable_event(struct intel_uncore_box *box, struct perf_ev
>   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops ivbep_uncore_cbox_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_cbox_ops = {
>   	.init_box		= ivbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -1785,7 +1785,7 @@ static struct intel_uncore_type ivbep_uncore_cbox = {
>   	.format_group		= &ivbep_uncore_cbox_format_group,
>   };
>   
> -static struct intel_uncore_ops ivbep_uncore_pcu_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_pcu_ops = {
>   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= snbep_pcu_hw_config,
>   	.get_constraint		= snbep_pcu_get_constraint,
> @@ -1873,7 +1873,7 @@ static u64 ivbep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
>   	return count;
>   }
>   
> -static struct intel_uncore_ops ivbep_uncore_irp_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_irp_ops = {
>   	.init_box	= ivbep_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -1893,7 +1893,7 @@ static struct intel_uncore_type ivbep_uncore_irp = {
>   	.format_group		= &ivbep_uncore_format_group,
>   };
>   
> -static struct intel_uncore_ops ivbep_uncore_qpi_ops = {
> +static const struct intel_uncore_ops ivbep_uncore_qpi_ops = {
>   	.init_box	= ivbep_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -2178,7 +2178,7 @@ static int knl_cha_hw_config(struct intel_uncore_box *box,
>   static void hswep_cbox_enable_event(struct intel_uncore_box *box,
>   				    struct perf_event *event);
>   
> -static struct intel_uncore_ops knl_uncore_cha_ops = {
> +static const struct intel_uncore_ops knl_uncore_cha_ops = {
>   	.init_box		= snbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -2272,7 +2272,7 @@ static void knl_uncore_imc_enable_event(struct intel_uncore_box *box,
>   				       hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops knl_uncore_imc_ops = {
> +static const struct intel_uncore_ops knl_uncore_imc_ops = {
>   	.init_box	= snbep_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= knl_uncore_imc_enable_box,
> @@ -2581,7 +2581,7 @@ static int hswep_ubox_hw_config(struct intel_uncore_box *box, struct perf_event
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops hswep_uncore_ubox_ops = {
> +static const struct intel_uncore_ops hswep_uncore_ubox_ops = {
>   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= hswep_ubox_hw_config,
>   	.get_constraint		= uncore_get_constraint,
> @@ -2741,7 +2741,7 @@ static void hswep_cbox_enable_event(struct intel_uncore_box *box,
>   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops hswep_uncore_cbox_ops = {
> +static const struct intel_uncore_ops hswep_uncore_cbox_ops = {
>   	.init_box		= snbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -2788,7 +2788,7 @@ static void hswep_uncore_sbox_msr_init_box(struct intel_uncore_box *box)
>   	}
>   }
>   
> -static struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
> +static const struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
>   	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.init_box		= hswep_uncore_sbox_msr_init_box
>   };
> @@ -2836,7 +2836,7 @@ static int hswep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops hswep_uncore_pcu_ops = {
> +static const struct intel_uncore_ops hswep_uncore_pcu_ops = {
>   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= hswep_pcu_hw_config,
>   	.get_constraint		= snbep_pcu_get_constraint,
> @@ -2941,7 +2941,7 @@ static u64 hswep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
>   	return count;
>   }
>   
> -static struct intel_uncore_ops hswep_uncore_irp_ops = {
> +static const struct intel_uncore_ops hswep_uncore_irp_ops = {
>   	.init_box	= snbep_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -3611,7 +3611,7 @@ static int skx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops skx_uncore_chabox_ops = {
> +static const struct intel_uncore_ops skx_uncore_chabox_ops = {
>   	/* There is no frz_en for chabox ctl */
>   	.init_box		= ivbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
> @@ -3673,7 +3673,7 @@ static void skx_iio_enable_event(struct intel_uncore_box *box,
>   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops skx_uncore_iio_ops = {
> +static const struct intel_uncore_ops skx_uncore_iio_ops = {
>   	.init_box		= ivbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -3931,7 +3931,7 @@ static struct uncore_event_desc skx_uncore_iio_freerunning_events[] = {
>   	{ /* end: all zeroes */ },
>   };
>   
> -static struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
> +static const struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
>   	.read_counter		= uncore_msr_read_counter,
>   	.hw_config		= uncore_freerunning_hw_config,
>   };
> @@ -4006,7 +4006,7 @@ static struct attribute_group skx_uncore_pcu_format_group = {
>   	.attrs = skx_uncore_pcu_formats_attr,
>   };
>   
> -static struct intel_uncore_ops skx_uncore_pcu_ops = {
> +static const struct intel_uncore_ops skx_uncore_pcu_ops = {
>   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= hswep_pcu_hw_config,
>   	.get_constraint		= snbep_pcu_get_constraint,
> @@ -4105,7 +4105,7 @@ static void skx_upi_uncore_pci_init_box(struct intel_uncore_box *box)
>   	pci_write_config_dword(pdev, SKX_UPI_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
>   }
>   
> -static struct intel_uncore_ops skx_upi_uncore_pci_ops = {
> +static const struct intel_uncore_ops skx_upi_uncore_pci_ops = {
>   	.init_box	= skx_upi_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -4136,7 +4136,7 @@ static void skx_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
>   	pci_write_config_dword(pdev, SKX_M2M_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
>   }
>   
> -static struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
> +static const struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
>   	.init_box	= skx_m2m_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -4373,7 +4373,7 @@ static void snr_cha_enable_event(struct intel_uncore_box *box,
>   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
>   }
>   
> -static struct intel_uncore_ops snr_uncore_chabox_ops = {
> +static const struct intel_uncore_ops snr_uncore_chabox_ops = {
>   	.init_box		= ivbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -4560,7 +4560,7 @@ static int snr_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *ev
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops snr_uncore_pcu_ops = {
> +static const struct intel_uncore_ops snr_uncore_pcu_ops = {
>   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
>   	.hw_config		= snr_pcu_hw_config,
>   	.get_constraint		= snbep_pcu_get_constraint,
> @@ -4660,7 +4660,7 @@ static void snr_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
>   	pci_write_config_dword(pdev, box_ctl, IVBEP_PMON_BOX_CTL_INT);
>   }
>   
> -static struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
> +static const struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
>   	.init_box	= snr_m2m_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -4706,7 +4706,7 @@ static void snr_uncore_pci_enable_event(struct intel_uncore_box *box, struct per
>   	pci_write_config_dword(pdev, hwc->config_base + 4, (u32)(hwc->config >> 32));
>   }
>   
> -static struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
> +static const struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
>   	.init_box	= snr_m2m_uncore_pci_init_box,
>   	.disable_box	= snbep_uncore_pci_disable_box,
>   	.enable_box	= snbep_uncore_pci_enable_box,
> @@ -4884,7 +4884,7 @@ static void snr_uncore_mmio_disable_event(struct intel_uncore_box *box,
>   	writel(hwc->config, box->io_addr + hwc->config_base);
>   }
>   
> -static struct intel_uncore_ops snr_uncore_mmio_ops = {
> +static const struct intel_uncore_ops snr_uncore_mmio_ops = {
>   	.init_box	= snr_uncore_mmio_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.disable_box	= snr_uncore_mmio_disable_box,
> @@ -4948,7 +4948,7 @@ static struct uncore_event_desc snr_uncore_imc_freerunning_events[] = {
>   	{ /* end: all zeroes */ },
>   };
>   
> -static struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
> +static const struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
>   	.init_box	= snr_uncore_mmio_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.read_counter	= uncore_mmio_read_counter,
> @@ -5005,7 +5005,7 @@ static int icx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
>   	return 0;
>   }
>   
> -static struct intel_uncore_ops icx_uncore_chabox_ops = {
> +static const struct intel_uncore_ops icx_uncore_chabox_ops = {
>   	.init_box		= ivbep_uncore_msr_init_box,
>   	.disable_box		= snbep_uncore_msr_disable_box,
>   	.enable_box		= snbep_uncore_msr_enable_box,
> @@ -5406,7 +5406,7 @@ static void icx_uncore_imc_init_box(struct intel_uncore_box *box)
>   	__snr_uncore_mmio_init_box(box, box_ctl, mem_offset);
>   }
>   
> -static struct intel_uncore_ops icx_uncore_mmio_ops = {
> +static const struct intel_uncore_ops icx_uncore_mmio_ops = {
>   	.init_box	= icx_uncore_imc_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.disable_box	= snr_uncore_mmio_disable_box,
> @@ -5476,7 +5476,7 @@ static void icx_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
>   	__snr_uncore_mmio_init_box(box, uncore_mmio_box_ctl(box), mem_offset);
>   }
>   
> -static struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
> +static const struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
>   	.init_box	= icx_uncore_imc_freerunning_init_box,
>   	.exit_box	= uncore_mmio_exit_box,
>   	.read_counter	= uncore_mmio_read_counter,
> 

^ permalink raw reply related	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops
  2021-06-07 19:09   ` Liang, Kan
@ 2021-06-08 19:17     ` Rikard Falkeborn
  2021-06-08 19:50       ` Liang, Kan
  0 siblings, 1 reply; 8+ messages in thread
From: Rikard Falkeborn @ 2021-06-08 19:17 UTC (permalink / raw)
  To: Liang, Kan
  Cc: Rikard Falkeborn, Peter Zijlstra, Ingo Molnar,
	Arnaldo Carvalho de Melo, Mark Rutland, Alexander Shishkin,
	Jiri Olsa, Namhyung Kim, Thomas Gleixner, Borislav Petkov, x86,
	H. Peter Anvin, linux-perf-users, linux-kernel,
	Alexander Antonov

On Mon, Jun 07, 2021 at 03:09:11PM -0400, Liang, Kan wrote:
> 
> 
> On 6/5/2021 11:56 AM, Rikard Falkeborn wrote:
> > These are not modified, so make them const to allow the compiler to put
> > them in read-only memory.
> 
> For most of the cases, yes, but the ops are modified for the TGL/RKL. We may
> want to create a read-only ops for the TGL/RKL as below, and make both
> skl_uncore_msr_ops and rkl_uncore_msr_ops const as well.
> 
Thanks for the feedback.

Yes, that was the odd one out, the commit message could have been
clearer. What I meant above was "Constify all intel_uncore_ops that are
not modified", which was all but skl_uncore_msr_ops. So the patch is
correct, but in order to also constify skl_uncore_msr_ops, we could do
as you suggested. Is it worth doing? And if so, in a V2 of this patch,
or as a follow-up patch?

Thanks,
Rikard
> 
> diff --git a/arch/x86/events/intel/uncore_snb.c
> b/arch/x86/events/intel/uncore_snb.c
> index 0f63706..e750514 100644
> --- a/arch/x86/events/intel/uncore_snb.c
> +++ b/arch/x86/events/intel/uncore_snb.c
> @@ -435,14 +435,22 @@ static void rkl_uncore_msr_init_box(struct
> intel_uncore_box *box)
>  		wrmsrl(SKL_UNC_PERF_GLOBAL_CTL, SNB_UNC_GLOBAL_CTL_EN);
>  }
> 
> +static struct intel_uncore_ops rkl_uncore_msr_ops = {
> +	.init_box	= rkl_uncore_msr_init_box,
> +	.enable_box	= skl_uncore_msr_enable_box,
> +	.exit_box	= skl_uncore_msr_exit_box,
> +	.disable_event	= snb_uncore_msr_disable_event,
> +	.enable_event	= snb_uncore_msr_enable_event,
> +	.read_counter	= uncore_msr_read_counter,
> +};
> +
>  void tgl_uncore_cpu_init(void)
>  {
>  	uncore_msr_uncores = tgl_msr_uncores;
>  	icl_uncore_cbox.num_boxes = icl_get_cbox_num();
> -	icl_uncore_cbox.ops = &skl_uncore_msr_ops;
> -	icl_uncore_clockbox.ops = &skl_uncore_msr_ops;
> -	snb_uncore_arb.ops = &skl_uncore_msr_ops;
> -	skl_uncore_msr_ops.init_box = rkl_uncore_msr_init_box;
> +	icl_uncore_cbox.ops = &rkl_uncore_msr_ops;
> +	icl_uncore_clockbox.ops = &rkl_uncore_msr_ops;
> +	snb_uncore_arb.ops = &rkl_uncore_msr_ops;
>  }
> 
>  static void adl_uncore_msr_init_box(struct intel_uncore_box *box)
> 
> Thanks,
> Kan
> 
> 
> > 
> > Signed-off-by: Rikard Falkeborn <rikard.falkeborn@gmail.com>
> > ---
> >   arch/x86/events/intel/uncore.h           |  2 +-
> >   arch/x86/events/intel/uncore_discovery.c |  6 +--
> >   arch/x86/events/intel/uncore_nhmex.c     | 10 ++--
> >   arch/x86/events/intel/uncore_snb.c       | 12 ++---
> >   arch/x86/events/intel/uncore_snbep.c     | 66 ++++++++++++------------
> >   5 files changed, 48 insertions(+), 48 deletions(-)
> > 
> > diff --git a/arch/x86/events/intel/uncore.h b/arch/x86/events/intel/uncore.h
> > index 187d7287039c..83b25a7b8c27 100644
> > --- a/arch/x86/events/intel/uncore.h
> > +++ b/arch/x86/events/intel/uncore.h
> > @@ -77,7 +77,7 @@ struct intel_uncore_type {
> >   	struct event_constraint unconstrainted;
> >   	struct event_constraint *constraints;
> >   	struct intel_uncore_pmu *pmus;
> > -	struct intel_uncore_ops *ops;
> > +	const struct intel_uncore_ops *ops;
> >   	struct uncore_event_desc *event_descs;
> >   	struct freerunning_counters *freerunning;
> >   	const struct attribute_group *attr_groups[4];
> > diff --git a/arch/x86/events/intel/uncore_discovery.c b/arch/x86/events/intel/uncore_discovery.c
> > index aba9bff95413..2b8e9a1d793e 100644
> > --- a/arch/x86/events/intel/uncore_discovery.c
> > +++ b/arch/x86/events/intel/uncore_discovery.c
> > @@ -368,7 +368,7 @@ static void intel_generic_uncore_msr_disable_event(struct intel_uncore_box *box,
> >   	wrmsrl(hwc->config_base, 0);
> >   }
> > -static struct intel_uncore_ops generic_uncore_msr_ops = {
> > +static const struct intel_uncore_ops generic_uncore_msr_ops = {
> >   	.init_box		= intel_generic_uncore_msr_init_box,
> >   	.disable_box		= intel_generic_uncore_msr_disable_box,
> >   	.enable_box		= intel_generic_uncore_msr_enable_box,
> > @@ -433,7 +433,7 @@ static u64 intel_generic_uncore_pci_read_counter(struct intel_uncore_box *box,
> >   	return count;
> >   }
> > -static struct intel_uncore_ops generic_uncore_pci_ops = {
> > +static const struct intel_uncore_ops generic_uncore_pci_ops = {
> >   	.init_box	= intel_generic_uncore_pci_init_box,
> >   	.disable_box	= intel_generic_uncore_pci_disable_box,
> >   	.enable_box	= intel_generic_uncore_pci_enable_box,
> > @@ -516,7 +516,7 @@ static void intel_generic_uncore_mmio_disable_event(struct intel_uncore_box *box
> >   	writel(0, box->io_addr + hwc->config_base);
> >   }
> > -static struct intel_uncore_ops generic_uncore_mmio_ops = {
> > +static const struct intel_uncore_ops generic_uncore_mmio_ops = {
> >   	.init_box	= intel_generic_uncore_mmio_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.disable_box	= intel_generic_uncore_mmio_disable_box,
> > diff --git a/arch/x86/events/intel/uncore_nhmex.c b/arch/x86/events/intel/uncore_nhmex.c
> > index 173e2674be6e..5f7c27d7f428 100644
> > --- a/arch/x86/events/intel/uncore_nhmex.c
> > +++ b/arch/x86/events/intel/uncore_nhmex.c
> > @@ -262,7 +262,7 @@ static void nhmex_uncore_msr_enable_event(struct intel_uncore_box *box, struct p
> >   	.disable_event	= nhmex_uncore_msr_disable_event,	\
> >   	.read_counter	= uncore_msr_read_counter
> > -static struct intel_uncore_ops nhmex_uncore_ops = {
> > +static const struct intel_uncore_ops nhmex_uncore_ops = {
> >   	NHMEX_UNCORE_OPS_COMMON_INIT(),
> >   	.enable_event	= nhmex_uncore_msr_enable_event,
> >   };
> > @@ -413,7 +413,7 @@ static const struct attribute_group nhmex_uncore_bbox_format_group = {
> >   	.attrs = nhmex_uncore_bbox_formats_attr,
> >   };
> > -static struct intel_uncore_ops nhmex_uncore_bbox_ops = {
> > +static const struct intel_uncore_ops nhmex_uncore_bbox_ops = {
> >   	NHMEX_UNCORE_OPS_COMMON_INIT(),
> >   	.enable_event		= nhmex_bbox_msr_enable_event,
> >   	.hw_config		= nhmex_bbox_hw_config,
> > @@ -490,7 +490,7 @@ static const struct attribute_group nhmex_uncore_sbox_format_group = {
> >   	.attrs			= nhmex_uncore_sbox_formats_attr,
> >   };
> > -static struct intel_uncore_ops nhmex_uncore_sbox_ops = {
> > +static const struct intel_uncore_ops nhmex_uncore_sbox_ops = {
> >   	NHMEX_UNCORE_OPS_COMMON_INIT(),
> >   	.enable_event		= nhmex_sbox_msr_enable_event,
> >   	.hw_config		= nhmex_sbox_hw_config,
> > @@ -916,7 +916,7 @@ static struct uncore_event_desc wsmex_uncore_mbox_events[] = {
> >   	{ /* end: all zeroes */ },
> >   };
> > -static struct intel_uncore_ops nhmex_uncore_mbox_ops = {
> > +static const struct intel_uncore_ops nhmex_uncore_mbox_ops = {
> >   	NHMEX_UNCORE_OPS_COMMON_INIT(),
> >   	.enable_event	= nhmex_mbox_msr_enable_event,
> >   	.hw_config	= nhmex_mbox_hw_config,
> > @@ -1179,7 +1179,7 @@ static struct uncore_event_desc nhmex_uncore_rbox_events[] = {
> >   	{ /* end: all zeroes */ },
> >   };
> > -static struct intel_uncore_ops nhmex_uncore_rbox_ops = {
> > +static const struct intel_uncore_ops nhmex_uncore_rbox_ops = {
> >   	NHMEX_UNCORE_OPS_COMMON_INIT(),
> >   	.enable_event		= nhmex_rbox_msr_enable_event,
> >   	.hw_config		= nhmex_rbox_hw_config,
> > diff --git a/arch/x86/events/intel/uncore_snb.c b/arch/x86/events/intel/uncore_snb.c
> > index 0f63706cdadf..5826cff568f4 100644
> > --- a/arch/x86/events/intel/uncore_snb.c
> > +++ b/arch/x86/events/intel/uncore_snb.c
> > @@ -216,7 +216,7 @@ static const struct attribute_group snb_uncore_format_group = {
> >   	.attrs		= snb_uncore_formats_attr,
> >   };
> > -static struct intel_uncore_ops snb_uncore_msr_ops = {
> > +static const struct intel_uncore_ops snb_uncore_msr_ops = {
> >   	.init_box	= snb_uncore_msr_init_box,
> >   	.enable_box	= snb_uncore_msr_enable_box,
> >   	.exit_box	= snb_uncore_msr_exit_box,
> > @@ -341,7 +341,7 @@ void skl_uncore_cpu_init(void)
> >   	snb_uncore_arb.ops = &skl_uncore_msr_ops;
> >   }
> > -static struct intel_uncore_ops icl_uncore_msr_ops = {
> > +static const struct intel_uncore_ops icl_uncore_msr_ops = {
> >   	.disable_event	= snb_uncore_msr_disable_event,
> >   	.enable_event	= snb_uncore_msr_enable_event,
> >   	.read_counter	= uncore_msr_read_counter,
> > @@ -468,7 +468,7 @@ static void adl_uncore_msr_exit_box(struct intel_uncore_box *box)
> >   		wrmsrl(ADL_UNC_PERF_GLOBAL_CTL, 0);
> >   }
> > -static struct intel_uncore_ops adl_uncore_msr_ops = {
> > +static const struct intel_uncore_ops adl_uncore_msr_ops = {
> >   	.init_box	= adl_uncore_msr_init_box,
> >   	.enable_box	= adl_uncore_msr_enable_box,
> >   	.disable_box	= adl_uncore_msr_disable_box,
> > @@ -799,7 +799,7 @@ static struct pmu snb_uncore_imc_pmu = {
> >   	.capabilities	= PERF_PMU_CAP_NO_EXCLUDE,
> >   };
> > -static struct intel_uncore_ops snb_uncore_imc_ops = {
> > +static const struct intel_uncore_ops snb_uncore_imc_ops = {
> >   	.init_box	= snb_uncore_imc_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.enable_box	= snb_uncore_imc_enable_box,
> > @@ -1267,7 +1267,7 @@ static struct uncore_event_desc nhm_uncore_events[] = {
> >   	{ /* end: all zeroes */ },
> >   };
> > -static struct intel_uncore_ops nhm_uncore_msr_ops = {
> > +static const struct intel_uncore_ops nhm_uncore_msr_ops = {
> >   	.disable_box	= nhm_uncore_msr_disable_box,
> >   	.enable_box	= nhm_uncore_msr_enable_box,
> >   	.disable_event	= snb_uncore_msr_disable_event,
> > @@ -1422,7 +1422,7 @@ static void tgl_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
> >   		pr_warn("perf uncore: Failed to ioremap for %s.\n", type->name);
> >   }
> > -static struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
> > +static const struct intel_uncore_ops tgl_uncore_imc_freerunning_ops = {
> >   	.init_box	= tgl_uncore_imc_freerunning_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.read_counter	= uncore_mmio_read_counter,
> > diff --git a/arch/x86/events/intel/uncore_snbep.c b/arch/x86/events/intel/uncore_snbep.c
> > index bb6eb1e5569c..43eabe8d37dc 100644
> > --- a/arch/x86/events/intel/uncore_snbep.c
> > +++ b/arch/x86/events/intel/uncore_snbep.c
> > @@ -775,7 +775,7 @@ static const struct attribute_group snbep_uncore_qpi_format_group = {
> >   	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),			\
> >   	.init_box	= snbep_uncore_msr_init_box		\
> > -static struct intel_uncore_ops snbep_uncore_msr_ops = {
> > +static const struct intel_uncore_ops snbep_uncore_msr_ops = {
> >   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   };
> > @@ -786,7 +786,7 @@ static struct intel_uncore_ops snbep_uncore_msr_ops = {
> >   	.disable_event	= snbep_uncore_pci_disable_event,	\
> >   	.read_counter	= snbep_uncore_pci_read_counter
> > -static struct intel_uncore_ops snbep_uncore_pci_ops = {
> > +static const struct intel_uncore_ops snbep_uncore_pci_ops = {
> >   	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
> >   	.enable_event	= snbep_uncore_pci_enable_event,	\
> >   };
> > @@ -1017,7 +1017,7 @@ static int snbep_cbox_hw_config(struct intel_uncore_box *box, struct perf_event
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops snbep_uncore_cbox_ops = {
> > +static const struct intel_uncore_ops snbep_uncore_cbox_ops = {
> >   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= snbep_cbox_hw_config,
> >   	.get_constraint		= snbep_cbox_get_constraint,
> > @@ -1127,7 +1127,7 @@ static int snbep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops snbep_uncore_pcu_ops = {
> > +static const struct intel_uncore_ops snbep_uncore_pcu_ops = {
> >   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= snbep_pcu_hw_config,
> >   	.get_constraint		= snbep_pcu_get_constraint,
> > @@ -1211,7 +1211,7 @@ static void snbep_qpi_enable_event(struct intel_uncore_box *box, struct perf_eve
> >   	pci_write_config_dword(pdev, hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops snbep_uncore_qpi_ops = {
> > +static const struct intel_uncore_ops snbep_uncore_qpi_ops = {
> >   	SNBEP_UNCORE_PCI_OPS_COMMON_INIT(),
> >   	.enable_event		= snbep_qpi_enable_event,
> >   	.hw_config		= snbep_qpi_hw_config,
> > @@ -1517,11 +1517,11 @@ static void ivbep_uncore_pci_init_box(struct intel_uncore_box *box)
> >   	.enable_event	= snbep_uncore_msr_enable_event,	\
> >   	.read_counter	= uncore_msr_read_counter
> > -static struct intel_uncore_ops ivbep_uncore_msr_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_msr_ops = {
> >   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   };
> > -static struct intel_uncore_ops ivbep_uncore_pci_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_pci_ops = {
> >   	.init_box	= ivbep_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -1757,7 +1757,7 @@ static void ivbep_cbox_enable_event(struct intel_uncore_box *box, struct perf_ev
> >   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops ivbep_uncore_cbox_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_cbox_ops = {
> >   	.init_box		= ivbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -1785,7 +1785,7 @@ static struct intel_uncore_type ivbep_uncore_cbox = {
> >   	.format_group		= &ivbep_uncore_cbox_format_group,
> >   };
> > -static struct intel_uncore_ops ivbep_uncore_pcu_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_pcu_ops = {
> >   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= snbep_pcu_hw_config,
> >   	.get_constraint		= snbep_pcu_get_constraint,
> > @@ -1873,7 +1873,7 @@ static u64 ivbep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
> >   	return count;
> >   }
> > -static struct intel_uncore_ops ivbep_uncore_irp_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_irp_ops = {
> >   	.init_box	= ivbep_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -1893,7 +1893,7 @@ static struct intel_uncore_type ivbep_uncore_irp = {
> >   	.format_group		= &ivbep_uncore_format_group,
> >   };
> > -static struct intel_uncore_ops ivbep_uncore_qpi_ops = {
> > +static const struct intel_uncore_ops ivbep_uncore_qpi_ops = {
> >   	.init_box	= ivbep_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -2178,7 +2178,7 @@ static int knl_cha_hw_config(struct intel_uncore_box *box,
> >   static void hswep_cbox_enable_event(struct intel_uncore_box *box,
> >   				    struct perf_event *event);
> > -static struct intel_uncore_ops knl_uncore_cha_ops = {
> > +static const struct intel_uncore_ops knl_uncore_cha_ops = {
> >   	.init_box		= snbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -2272,7 +2272,7 @@ static void knl_uncore_imc_enable_event(struct intel_uncore_box *box,
> >   				       hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops knl_uncore_imc_ops = {
> > +static const struct intel_uncore_ops knl_uncore_imc_ops = {
> >   	.init_box	= snbep_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= knl_uncore_imc_enable_box,
> > @@ -2581,7 +2581,7 @@ static int hswep_ubox_hw_config(struct intel_uncore_box *box, struct perf_event
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops hswep_uncore_ubox_ops = {
> > +static const struct intel_uncore_ops hswep_uncore_ubox_ops = {
> >   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= hswep_ubox_hw_config,
> >   	.get_constraint		= uncore_get_constraint,
> > @@ -2741,7 +2741,7 @@ static void hswep_cbox_enable_event(struct intel_uncore_box *box,
> >   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops hswep_uncore_cbox_ops = {
> > +static const struct intel_uncore_ops hswep_uncore_cbox_ops = {
> >   	.init_box		= snbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -2788,7 +2788,7 @@ static void hswep_uncore_sbox_msr_init_box(struct intel_uncore_box *box)
> >   	}
> >   }
> > -static struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
> > +static const struct intel_uncore_ops hswep_uncore_sbox_msr_ops = {
> >   	__SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.init_box		= hswep_uncore_sbox_msr_init_box
> >   };
> > @@ -2836,7 +2836,7 @@ static int hswep_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops hswep_uncore_pcu_ops = {
> > +static const struct intel_uncore_ops hswep_uncore_pcu_ops = {
> >   	SNBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= hswep_pcu_hw_config,
> >   	.get_constraint		= snbep_pcu_get_constraint,
> > @@ -2941,7 +2941,7 @@ static u64 hswep_uncore_irp_read_counter(struct intel_uncore_box *box, struct pe
> >   	return count;
> >   }
> > -static struct intel_uncore_ops hswep_uncore_irp_ops = {
> > +static const struct intel_uncore_ops hswep_uncore_irp_ops = {
> >   	.init_box	= snbep_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -3611,7 +3611,7 @@ static int skx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops skx_uncore_chabox_ops = {
> > +static const struct intel_uncore_ops skx_uncore_chabox_ops = {
> >   	/* There is no frz_en for chabox ctl */
> >   	.init_box		= ivbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> > @@ -3673,7 +3673,7 @@ static void skx_iio_enable_event(struct intel_uncore_box *box,
> >   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops skx_uncore_iio_ops = {
> > +static const struct intel_uncore_ops skx_uncore_iio_ops = {
> >   	.init_box		= ivbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -3931,7 +3931,7 @@ static struct uncore_event_desc skx_uncore_iio_freerunning_events[] = {
> >   	{ /* end: all zeroes */ },
> >   };
> > -static struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
> > +static const struct intel_uncore_ops skx_uncore_iio_freerunning_ops = {
> >   	.read_counter		= uncore_msr_read_counter,
> >   	.hw_config		= uncore_freerunning_hw_config,
> >   };
> > @@ -4006,7 +4006,7 @@ static struct attribute_group skx_uncore_pcu_format_group = {
> >   	.attrs = skx_uncore_pcu_formats_attr,
> >   };
> > -static struct intel_uncore_ops skx_uncore_pcu_ops = {
> > +static const struct intel_uncore_ops skx_uncore_pcu_ops = {
> >   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= hswep_pcu_hw_config,
> >   	.get_constraint		= snbep_pcu_get_constraint,
> > @@ -4105,7 +4105,7 @@ static void skx_upi_uncore_pci_init_box(struct intel_uncore_box *box)
> >   	pci_write_config_dword(pdev, SKX_UPI_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
> >   }
> > -static struct intel_uncore_ops skx_upi_uncore_pci_ops = {
> > +static const struct intel_uncore_ops skx_upi_uncore_pci_ops = {
> >   	.init_box	= skx_upi_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -4136,7 +4136,7 @@ static void skx_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
> >   	pci_write_config_dword(pdev, SKX_M2M_PCI_PMON_BOX_CTL, IVBEP_PMON_BOX_CTL_INT);
> >   }
> > -static struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
> > +static const struct intel_uncore_ops skx_m2m_uncore_pci_ops = {
> >   	.init_box	= skx_m2m_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -4373,7 +4373,7 @@ static void snr_cha_enable_event(struct intel_uncore_box *box,
> >   	wrmsrl(hwc->config_base, hwc->config | SNBEP_PMON_CTL_EN);
> >   }
> > -static struct intel_uncore_ops snr_uncore_chabox_ops = {
> > +static const struct intel_uncore_ops snr_uncore_chabox_ops = {
> >   	.init_box		= ivbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -4560,7 +4560,7 @@ static int snr_pcu_hw_config(struct intel_uncore_box *box, struct perf_event *ev
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops snr_uncore_pcu_ops = {
> > +static const struct intel_uncore_ops snr_uncore_pcu_ops = {
> >   	IVBEP_UNCORE_MSR_OPS_COMMON_INIT(),
> >   	.hw_config		= snr_pcu_hw_config,
> >   	.get_constraint		= snbep_pcu_get_constraint,
> > @@ -4660,7 +4660,7 @@ static void snr_m2m_uncore_pci_init_box(struct intel_uncore_box *box)
> >   	pci_write_config_dword(pdev, box_ctl, IVBEP_PMON_BOX_CTL_INT);
> >   }
> > -static struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
> > +static const struct intel_uncore_ops snr_m2m_uncore_pci_ops = {
> >   	.init_box	= snr_m2m_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -4706,7 +4706,7 @@ static void snr_uncore_pci_enable_event(struct intel_uncore_box *box, struct per
> >   	pci_write_config_dword(pdev, hwc->config_base + 4, (u32)(hwc->config >> 32));
> >   }
> > -static struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
> > +static const struct intel_uncore_ops snr_pcie3_uncore_pci_ops = {
> >   	.init_box	= snr_m2m_uncore_pci_init_box,
> >   	.disable_box	= snbep_uncore_pci_disable_box,
> >   	.enable_box	= snbep_uncore_pci_enable_box,
> > @@ -4884,7 +4884,7 @@ static void snr_uncore_mmio_disable_event(struct intel_uncore_box *box,
> >   	writel(hwc->config, box->io_addr + hwc->config_base);
> >   }
> > -static struct intel_uncore_ops snr_uncore_mmio_ops = {
> > +static const struct intel_uncore_ops snr_uncore_mmio_ops = {
> >   	.init_box	= snr_uncore_mmio_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.disable_box	= snr_uncore_mmio_disable_box,
> > @@ -4948,7 +4948,7 @@ static struct uncore_event_desc snr_uncore_imc_freerunning_events[] = {
> >   	{ /* end: all zeroes */ },
> >   };
> > -static struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
> > +static const struct intel_uncore_ops snr_uncore_imc_freerunning_ops = {
> >   	.init_box	= snr_uncore_mmio_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.read_counter	= uncore_mmio_read_counter,
> > @@ -5005,7 +5005,7 @@ static int icx_cha_hw_config(struct intel_uncore_box *box, struct perf_event *ev
> >   	return 0;
> >   }
> > -static struct intel_uncore_ops icx_uncore_chabox_ops = {
> > +static const struct intel_uncore_ops icx_uncore_chabox_ops = {
> >   	.init_box		= ivbep_uncore_msr_init_box,
> >   	.disable_box		= snbep_uncore_msr_disable_box,
> >   	.enable_box		= snbep_uncore_msr_enable_box,
> > @@ -5406,7 +5406,7 @@ static void icx_uncore_imc_init_box(struct intel_uncore_box *box)
> >   	__snr_uncore_mmio_init_box(box, box_ctl, mem_offset);
> >   }
> > -static struct intel_uncore_ops icx_uncore_mmio_ops = {
> > +static const struct intel_uncore_ops icx_uncore_mmio_ops = {
> >   	.init_box	= icx_uncore_imc_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.disable_box	= snr_uncore_mmio_disable_box,
> > @@ -5476,7 +5476,7 @@ static void icx_uncore_imc_freerunning_init_box(struct intel_uncore_box *box)
> >   	__snr_uncore_mmio_init_box(box, uncore_mmio_box_ctl(box), mem_offset);
> >   }
> > -static struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
> > +static const struct intel_uncore_ops icx_uncore_imc_freerunning_ops = {
> >   	.init_box	= icx_uncore_imc_freerunning_init_box,
> >   	.exit_box	= uncore_mmio_exit_box,
> >   	.read_counter	= uncore_mmio_read_counter,
> > 

^ permalink raw reply	[flat|nested] 8+ messages in thread
* Re: [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops
  2021-06-08 19:17     ` Rikard Falkeborn
@ 2021-06-08 19:50       ` Liang, Kan
  0 siblings, 0 replies; 8+ messages in thread
From: Liang, Kan @ 2021-06-08 19:50 UTC (permalink / raw)
  To: Rikard Falkeborn
  Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
	Mark Rutland, Alexander Shishkin, Jiri Olsa, Namhyung Kim,
	Thomas Gleixner, Borislav Petkov, x86, H. Peter Anvin,
	linux-perf-users, linux-kernel, Alexander Antonov



On 6/8/2021 3:17 PM, Rikard Falkeborn wrote:
> On Mon, Jun 07, 2021 at 03:09:11PM -0400, Liang, Kan wrote:
>>
>> On 6/5/2021 11:56 AM, Rikard Falkeborn wrote:
>>> These are not modified, so make them const to allow the compiler to put
>>> them in read-only memory.
>> For most of the cases, yes, but the ops are modified for the TGL/RKL. We may
>> want to create a read-only ops for the TGL/RKL as below, and make both
>> skl_uncore_msr_ops and rkl_uncore_msr_ops const as well.
>>
> Thanks for the feedback.
> 
> Yes, that was the odd one out, the commit message could have been
> clearer. What I meant above was "Constify all intel_uncore_ops that are
> not modified", which was all but skl_uncore_msr_ops. So the patch is
> correct, but in order to also constify skl_uncore_msr_ops, we could do
> as you suggested. Is it worth doing?

Since the patch set changes the intel_uncore_ops *ops to const, I think 
it's better to change them all at once. It will bring confusion if some 
of them are read-only but others can be modified.

> And if so, in a V2 of this patch,
> or as a follow-up patch?

Either is fine for me.

Thanks,
Kan

^ permalink raw reply	[flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-06-08 19:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-05 15:56 [PATCH 0/4] perf/x86/intel: Constify static structs Rikard Falkeborn
2021-06-05 15:56 ` [PATCH 1/4] perf/x86/intel/uncore: Constify intel_uncore_ops Rikard Falkeborn
2021-06-07 19:09   ` Liang, Kan
2021-06-08 19:17     ` Rikard Falkeborn
2021-06-08 19:50       ` Liang, Kan
2021-06-05 15:56 ` [PATCH 2/4] perf/x86/intel: Constify static attribute_group structs Rikard Falkeborn
2021-06-05 15:56 ` [PATCH 3/4] perf/x86/intel/uncore: Constify freerunning_counters Rikard Falkeborn
2021-06-05 15:56 ` [PATCH 4/4] perf/x86/intel/uncore: Constify unmodified static extra_reg structs Rikard Falkeborn

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).