On Fri, 19 Dec 2025 18:11:27 +0000
Ben Horgan [off-list ref] wrote:

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

resctrl supports 'MB', as a percentage throttling of traffic somewhere
after the L3. This is the control that mba_sc uses, so ideally the class
chosen should be as close as possible to the counters used for mba_local.

MB's percentage control should be backed either with the fixed point

either's "or" is missing.  (sentence should include the second choice)

fraction MBW_MAX. The bandwidth portion bitmaps is not used as its tricky
to pick which bits to use to avoid contention, and may be possible to
expose this as something other than a percentage in the future.

CC: Zeng Heng <redacted>
Co-developed-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>>
Signed-off-by: Ben Horgan <ben.horgan@arm.com>

As mentioned in earlier review I'd like an overview doc of the heuristics
used to map to the resctrl everything is a xeon view of the world.

At least some of our platforms are far enough from this view that the
heuristics fail (others can provide more info on that than I can).
That's fine as I'd rather not squash something inappropriate into a
viewpoint that doesn't fit. Better to leave those for later solutions!


Otherwise, just minor comments inline.

Jonathan

...

+static u32 get_mba_min(struct mpam_props *cprops)
+{
+	u32 val = 0;
+
+	if (mba_class_use_mbw_max(cprops))
+		val = mbw_max_to_percent(val, cprops);
+	else
+		WARN_ON_ONCE(1);
+
+	return val;

I'd be tempted to handle the error case first to make
clear this is really just a sanity checking wrapper around the max_to_percent()
function.

	if (!mba_class_use_mbw_max(cprops)) {
		WARN_ON_ONCE(1);
		return 0;
	}

	return mbw_max_to_percent(val, cprops);

+}

+/*
+ * topology_matches_l3() - Is the provided class the same shape as L3
+ * @victim:		The class we'd like to pretend is L3.
+ *
+ * resctrl expects all the world's a Xeon, and all counters are on the
+ * L3. We play fast and loose with this, mapping counters on other
+ * classes - provided the CPU->domain mapping is the same kind of shape.
+ *
+ * Using cacheinfo directly would make this work even if resctrl can't
+ * use the L3 - but cacheinfo can't tell us anything about offline CPUs.
+ * Using the L3 resctrl domain list also depends on CPUs being online.
+ * Using the mpam_class we picked for L3 so we can use its domain list
+ * assumes that there are MPAM controls on the L3.
+ * Instead, this path eventually uses the mpam_get_cpumask_from_cache_id()
+ * helper which can tell us about offline CPUs ... but getting the cache_id
+ * to start with relies on at least one CPU per L3 cache being online at
+ * boot.

So the usual mess of maxcpus=1 breaks it for anything other than first L3?
I'm not entirely against that but it may be a little unexpected.

+ *
+ * Walk the victim component list and compare the affinity mask with the
+ * corresponding L3. The topology matches if each victim:component's affinity
+ * mask is the same as the CPU's corresponding L3's. These lists/masks are
+ * computed from firmware tables so don't change at runtime.
+ */
+static bool topology_matches_l3(struct mpam_class *victim)

Probably should be consistent with either a blank line before break or not.
That might also make the diff a little nicer.

 		break;
 	default:
 		break;