Received-SPF: pass (google.com: domain of linux-kernel+bounces-160236-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
From: Dave Martin <Dave.Martin@arm.com>
To: x86@kernel.org,
	linux-kernel@vger.kernel.org
Cc: James Morse <james.morse@arm.com>,
	Fenghua Yu <fenghua.yu@intel.com>,
	Reinette Chatre <reinette.chatre@intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	H Peter Anvin <hpa@zytor.com>,
	Babu Moger <Babu.Moger@amd.com>,
	shameerali.kolothum.thodi@huawei.com,
	D Scott Phillips OS <scott@os.amperecomputing.com>,
	carl@os.amperecomputing.com,
	lcherian@marvell.com,
	bobo.shaobowang@huawei.com,
	"Shaopeng Tan (Fujitsu)" <tan.shaopeng@fujitsu.com>,
	baolin.wang@linux.alibaba.com,
	Jamie Iles <quic_jiles@quicinc.com>,
	Xin Hao <xhao@linux.alibaba.com>,
	Peter Newman <peternewman@google.com>,
	dfustini@baylibre.com,
	amitsinght@marvell.com,
	David Hildenbrand <david@redhat.com>,
	Rex Nie <rex.nie@jaguarmicro.com>
Subject: [PATCH v2 18/35] x86/resctrl: Allow resctrl_arch_mon_event_config_write() to return an error
Date: Fri, 26 Apr 2024 16:08:47 +0100
Message-Id: <20240426150904.8854-18-Dave.Martin@arm.com>
In-Reply-To: <20240426150904.8854-1-Dave.Martin@arm.com>
References: <20240426150537.8094-1-Dave.Martin@arm.com>
 <20240426150904.8854-1-Dave.Martin@arm.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: James Morse <james.morse@arm.com>

resctrl_arch_mon_event_config_write() writes a bitmap of events provided
by user-space into the configuration register for the monitors.

This assumes that all architectures support all the features each bit
corresponds to.

MPAM can filter monitors based on read, write, or both, but there are
many more options in the existing bitmap. To allow this interface to
work for machines with MPAM, allow the architecture helper to return
an error if an incompatible bitmap is set.

When valid values are provided, there is no change in behaviour. If
an invalid value is provided, currently it is silently ignored, but
last_cmd_status is updated. After this change, the parser will stop
at the first invalid value and return an error to user-space. This
matches the way changes to the schemata file are made.

Signed-off-by: James Morse <james.morse@arm.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>

---

FYI: James Morse (et al.):

 * Can this patch be dropped?

   It looks like the core code will handling checking cfg values
   gracefully if the arch backend initialises rdt_resource::
   mbm_cfg_mask to something suitable?

   Is there any reason why we can't detect bad values globally before
   cross-calling?

Changes in v2:

 * [Whitespace only] Re-tabbed resctrl_mon_config_info::err field
   declaration in <linux/resctrl.h> to match the prevailing style.

   No functional change.
---
 arch/x86/kernel/cpu/resctrl/rdtgroup.c | 20 ++++++++++++++++----
 include/linux/resctrl.h                |  1 +
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
index ef9c30a2eace..38436e4c1741 100644
--- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c
+++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c
@@ -1683,13 +1683,16 @@ void resctrl_arch_mon_event_config_write(void *info)
 	index = mon_event_config_index_get(mon_info->evtid);
 	if (index == INVALID_CONFIG_INDEX) {
 		pr_warn_once("Invalid event id %d\n", mon_info->evtid);
+		mon_info->err = -EINVAL;
 		return;
 	}
 	wrmsr(MSR_IA32_EVT_CFG_BASE + index, mon_info->mon_config, 0);
+
+	mon_info->err = 0;
 }
 
-static void mbm_config_write_domain(struct rdt_resource *r,
-				    struct rdt_domain *d, u32 evtid, u32 val)
+static int mbm_config_write_domain(struct rdt_resource *r,
+				   struct rdt_domain *d, u32 evtid, u32 val)
 {
 	struct resctrl_mon_config_info mon_info = {0};
 
@@ -1702,7 +1705,7 @@ static void mbm_config_write_domain(struct rdt_resource *r,
 	mon_info.evtid = evtid;
 	mondata_config_read(&mon_info);
 	if (mon_info.mon_config == val)
-		return;
+		return 0;
 
 	mon_info.mon_config = val;
 
@@ -1714,6 +1717,10 @@ static void mbm_config_write_domain(struct rdt_resource *r,
 	 */
 	smp_call_function_any(&d->cpu_mask, resctrl_arch_mon_event_config_write,
 			      &mon_info, 1);
+	if (mon_info.err) {
+		rdt_last_cmd_puts("Invalid event configuration\n");
+		return mon_info.err;
+	}
 
 	/*
 	 * When an Event Configuration is changed, the bandwidth counters
@@ -1725,6 +1732,8 @@ static void mbm_config_write_domain(struct rdt_resource *r,
 	 * mbm_local and mbm_total counts for all the RMIDs.
 	 */
 	resctrl_arch_reset_rmid_all(r, d);
+
+	return 0;
 }
 
 static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
@@ -1732,6 +1741,7 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
 	char *dom_str = NULL, *id_str;
 	unsigned long dom_id, val;
 	struct rdt_domain *d;
+	int err;
 
 	/* Walking r->domains, ensure it can't race with cpuhp */
 	lockdep_assert_cpus_held();
@@ -1763,7 +1773,9 @@ static int mon_config_write(struct rdt_resource *r, char *tok, u32 evtid)
 
 	list_for_each_entry(d, &r->domains, list) {
 		if (d->id == dom_id) {
-			mbm_config_write_domain(r, d, evtid, val);
+			err = mbm_config_write_domain(r, d, evtid, val);
+			if (err)
+				return err;
 			goto next;
 		}
 	}
diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index 2f71ae690640..63d377e8cb87 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -200,6 +200,7 @@ struct resctrl_mon_config_info {
 	struct rdt_domain	*d;
 	u32			evtid;
 	u32			mon_config;
+	int			err;
 };
 
 /**
-- 
2.34.1