Re: [PATCH v3 05/26] x86/rectrl: Fake OOBMSM interface

[Reinette Chatre <reinette.chatre@intel.com>]

Hi Tony:

(deja vu ... lets try this comment again  ... x86/rectrl -> x86/resctrl)

On 4/7/25 4:40 PM, Tony Luck wrote:

...

> diff --git a/arch/x86/kernel/cpu/resctrl/fake_intel_aet_features.c b/arch/x86/kernel/cpu/resctrl/fake_intel_aet_features.c
> new file mode 100644
> index 000000000000..5a16db67c7b8
> --- /dev/null
> +++ b/arch/x86/kernel/cpu/resctrl/fake_intel_aet_features.c
> @@ -0,0 +1,87 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +#include <linux/cleanup.h>
> +#include <linux/minmax.h>
> +#include <linux/slab.h>
> +#include "fake_intel_aet_features.h"
> +#include <linux/intel_vsec.h>
> +#include <linux/resctrl.h>
> +
> +#include "internal.h"
> +
> +/* Amount of memory for each fake MMIO space */
> +#define ENERGY_QWORDS	((576 * 2) + 3)
> +#define ENERGY_SIZE	(ENERGY_QWORDS * 8)
> +#define PERF_QWORDS	((576 * 7) + 3)
> +#define PERF_SIZE	(PERF_QWORDS * 8)

Could you please add explanations for the magic numbers?
For example, why are both energy and perf using 576?

> +
> +static long pg[4 * ENERGY_QWORDS + 2 * PERF_QWORDS];
> +
> +/*
> + * Fill the fake MMIO space with all different values,
> + * all with BIT(63) set to indicate valid entries.
> + */
> +static int __init fill(void)
> +{
> +	u64 val = 0;
> +
> +	for (int i = 0; i < sizeof(pg); i += sizeof(val)) {
> +		pg[i / sizeof(val)] = BIT_ULL(63) + val;
> +		val++;
> +	}
> +	return 0;
> +}
> +device_initcall(fill);
> +
> +#define PKG_REGION(_entry, _guid, _addr, _size, _pkg, _num_rmids)	\
> +	[_entry] = { .guid = _guid, .addr = (void __iomem *)_addr, \
> +		     .num_rmids = _num_rmids, \
> +		     .size = _size, .plat_info = { .package_id = _pkg }}
> +
> +/*
> + * Set up a fake return for call to:
> + *   intel_pmt_get_regions_by_feature(FEATURE_PER_RMID_ENERGY_TELEM);
> + * Pretend there are two aggregators on each of the sockets to test
> + * the code that sums over multiple aggregators.
> + */
> +static struct pmt_feature_group fake_energy = {
> +	.count = 4,
> +	.regions = {
> +		PKG_REGION(0, 0x26696143, &pg[0 * ENERGY_QWORDS], ENERGY_SIZE, 0, 64),
> +		PKG_REGION(1, 0x26696143, &pg[1 * ENERGY_QWORDS], ENERGY_SIZE, 0, 64),
> +		PKG_REGION(2, 0x26696143, &pg[2 * ENERGY_QWORDS], ENERGY_SIZE, 1, 64),
> +		PKG_REGION(3, 0x26696143, &pg[3 * ENERGY_QWORDS], ENERGY_SIZE, 1, 64)
> +	}
> +};
> +
> +/*
> + * Fake return for:
> + *   intel_pmt_get_regions_by_feature(FEATURE_PER_RMID_PERF_TELEM);
> + */
> +static struct pmt_feature_group fake_perf = {
> +	.count = 2,
> +	.regions = {
> +		PKG_REGION(0, 0x26557651, &pg[4 * ENERGY_QWORDS + 0 * PERF_QWORDS], PERF_SIZE, 0, 576),
> +		PKG_REGION(1, 0x26557651, &pg[4 * ENERGY_QWORDS + 1 * PERF_QWORDS], PERF_SIZE, 1, 576)
> +	}
> +};
> +
> +struct pmt_feature_group *
> +intel_pmt_get_regions_by_feature(enum pmt_feature_id id)
> +{
> +	switch (id) {
> +	case FEATURE_PER_RMID_ENERGY_TELEM:
> +		return &fake_energy;
> +	case FEATURE_PER_RMID_PERF_TELEM:
> +		return &fake_perf;
> +	default:
> +		return ERR_PTR(-ENOENT);
> +	}
> +	return ERR_PTR(-ENOENT);

Not reachable.

> +}
> +
> +/*
> + * Nothing needed for the "put" function.
> + */
> +void intel_pmt_put_feature_group(struct pmt_feature_group *feature_group)
> +{
> +}
> diff --git a/arch/x86/kernel/cpu/resctrl/Makefile b/arch/x86/kernel/cpu/resctrl/Makefile
> index 909be78ec6da..c56d3acf8ac7 100644
> --- a/arch/x86/kernel/cpu/resctrl/Makefile
> +++ b/arch/x86/kernel/cpu/resctrl/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_X86_CPU_RESCTRL)		+= core.o rdtgroup.o monitor.o
>  obj-$(CONFIG_X86_CPU_RESCTRL)		+= ctrlmondata.o
> +obj-$(CONFIG_INTEL_AET_RESCTRL)		+= fake_intel_aet_features.o
>  obj-$(CONFIG_RESCTRL_FS_PSEUDO_LOCK)	+= pseudo_lock.o
>  
>  # To allow define_trace.h's recursive include:

Reinette