Re: [RFC PATCH 25/31] x86/resctrl: Introduce region-based MBA write implementation on MMIO space

["Luck, Tony" <tony.luck@intel.com>]

On Mon, Aug 03, 2026 at 12:07:38AM +0800, Chen Yu wrote:
> Implement the marc_hw_update() callback to program per-region MBA
> bandwidth values via MMIO.
> 
> The MARC register layout packs 4 regions into each 64-bit register,
> with a 9-bit bandwidth field per region at (region % 4) * 16 bits.
> The MMIO address for a given CLOSID is calculated using the MARC index
> function 1:
> 
>   addr = base + Floor(Region / 4) * 512 + CLOS * 8
> 
> Each controller handles a single (region, type) pair, but a write always
> touches the whole 64-bit register. The callback therefore does a
> read-modify-write: it changes only its own region's field and writes
> every other bit back exactly as it was read, so the reserved bits keep
> their original values. A shadow copy of the register (marc_buf) holds
> the value last written, which spares the read on later updates.
> 
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> ---
>  arch/x86/kernel/cpu/resctrl/erdt.c     | 52 ++++++++++++++++++++++++++
>  arch/x86/kernel/cpu/resctrl/internal.h |  4 ++
>  2 files changed, 56 insertions(+)
> 
> diff --git a/arch/x86/kernel/cpu/resctrl/erdt.c b/arch/x86/kernel/cpu/resctrl/erdt.c
> index 38720c3b4a3d..c25aff805b12 100644
> --- a/arch/x86/kernel/cpu/resctrl/erdt.c
> +++ b/arch/x86/kernel/cpu/resctrl/erdt.c
> @@ -275,6 +275,49 @@ struct erdt_domain_info *erdt_find_domain_info(int cpu)
>  
>  static void marc_hw_update(struct hw_param *m)
>  {
> +	struct rdt_hw_ctrl_domain *hw_dom = resctrl_to_arch_ctrl_dom(m->dom);
> +	struct resctrl_hw_ctrl *hw_ctrl = resctrl_to_arch_ctrl(m->ctrl);
> +	enum resctrl_ctrl_name name = hw_ctrl->r_ctrl.name;
> +	unsigned int offset, region, type, region_offset_bits;
> +	struct erdt_domain_info *d = hw_dom->d_info;
> +	enum erdt_mmio_type mmio_type;
> +	void __iomem *addr;
> +	int closid_idx;
> +	unsigned int i;
> +	u64 val;
> +
> +	if (!d || !d->marc)
> +		return;
> +
> +	offset = name - RESCTRL_CTRL_NAME_REGION0_OPT;
> +	region = offset / RESCTRL_CTRL_REGION_NR_CTRLS;
> +	type = offset % RESCTRL_CTRL_REGION_NR_CTRLS;
> +	mmio_type = ERDT_MMIO_MARC_OPT + type;
> +	region_offset_bits = (region % 4) * 16;
> +
> +	if (d->marc_buf_type != mmio_type) {
> +		memset(d->marc_buf, 0,
> +		       d->marc->mba_reg_size * 512 * sizeof(u64));
> +		d->marc_buf_type = mmio_type;
> +	}
> +
> +	for (i = m->low; i < m->high; i++) {
> +		closid_idx = (region / 4) * 64 + i;
> +		addr = d->base[mmio_type] + closid_idx * 8;
> +
> +		/* The cached value retains the reserved bits to be preserved. */
> +		val = d->marc_buf[closid_idx];
> +		if (!val)
> +			val = readq(addr);
> +
> +		if (WARN_ON_ONCE(!val))
> +			return;
> +
> +		val &= ~(0x1ffULL << region_offset_bits);
> +		val |= (u64)(hw_dom->ctrl_val[i] & 0x1ff) << region_offset_bits;
> +		d->marc_buf[closid_idx] = val;
> +		writeq(val, addr);
> +	}
>  }

There are lots of opinions about bit fields. I'm a fan for h/w registers
accessed in architecture specific code.
 
I think the version below is easier to read than all the inline masks and
shifts.

-Tony

union bw_ctrl {
	u64	reg;
	struct {
		u16	val : 9;
		u16	rsvd : 7;
	} regions[4];
};

static void marc_hw_update(struct hw_param *m)
{
	struct rdt_hw_ctrl_domain *hw_dom = resctrl_to_arch_ctrl_dom(m->dom);
	struct resctrl_hw_ctrl *hw_ctrl = resctrl_to_arch_ctrl(m->ctrl);
	enum resctrl_ctrl_name name = hw_ctrl->r_ctrl.name;
	struct erdt_domain_info *d = hw_dom->d_info;
	unsigned int offset, region, type;
	enum erdt_mmio_type mmio_type;
	union bw_ctrl mmio_ctrl;
	void __iomem *addr;
	int closid_idx;
	unsigned int i;

	if (!d || !d->marc)
		return;

	offset = name - RESCTRL_CTRL_NAME_REGION0_OPT;
	region = offset / RESCTRL_CTRL_REGION_NR_CTRLS;
	type = offset % RESCTRL_CTRL_REGION_NR_CTRLS;
	mmio_type = ERDT_MMIO_MARC_OPT + type;

	if (d->marc_buf_type != mmio_type) {
		memset(d->marc_buf, 0,
		       d->marc->mba_reg_size * 512 * sizeof(u64));
		d->marc_buf_type = mmio_type;
	}

	for (i = m->low; i < m->high; i++) {
		closid_idx = (region / 4) * 64 + i;
		addr = d->base[mmio_type] + closid_idx * 8;

		/* The cached value retains the reserved bits to be preserved. */
		mmio_ctrl.reg = d->marc_buf[closid_idx];
		if (!mmio_ctrl.reg)
			mmio_ctrl.reg = readq(addr);

		if (WARN_ON_ONCE(!mmio_ctrl.reg))
			return;

		mmio_ctrl.regions[region].val = hw_dom->ctrl_val[i];
		d->marc_buf[closid_idx] = mmio_ctrl.reg;
		writeq(mmio_ctrl.reg, addr);
	}
}