Hi Tony,
On 5/21/25 3:50 PM, Tony Luck wrote:
> Counters for telemetry events are in MMIO space. Each telemetry_region
> structure returned in the pmt_feature_group returned from OOBMSM
> contains the base MMIO address for the counters.
>
> Scan all the telemetry_region structures again and gather these
> addresses into a more convenient structure with addresses for
> each aggregator indexed by package id. Note that there may be
> multiple aggregators per package.
>
> Completed structure for each event group looks like this:
The depiction is very useful but please note that the above description
only equips reader with "a more convenient structure" to interpret
what below represents. It is a leap.
>
> +---------------------+---------------------+
> pkginfo** -->| pkginfo[0] | pkginfo[1] |
> +---------------------+---------------------+
> | |
> v v
> +----------------+ +----------------+
> |struct mmio_info| |struct mmio_info|
> +----------------+ +----------------+
> | count = N | | count = N |
I think renaming "count" to "num_regions" will help a lot to
explain this data.
> | addrs[0] | | addrs[0] |
> | addrs[0] | | addrs[0] |
[0] -> [1]
> | ... | | ... |
> | addrs[N-1] | | addrs[N-1] |
> +----------------+ +----------------+
>
> Signed-off-by: Tony Luck <tony.luck@intel.com>
> ---
> arch/x86/kernel/cpu/resctrl/intel_aet.c | 65 ++++++++++++++++++++++++-
> 1 file changed, 64 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/cpu/resctrl/intel_aet.c b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> index ffcb54be54ea..2316198eb69e 100644
> --- a/arch/x86/kernel/cpu/resctrl/intel_aet.c
> +++ b/arch/x86/kernel/cpu/resctrl/intel_aet.c
> @@ -21,17 +21,32 @@
>
> #include "internal.h"
>
> +/**
> + * struct mmio_info - Array of MMIO addresses for one event group for a package
"MMIO address information for one event group of a package."?
> + * @count: Number of addresses on this package
"@num_regions: Number of telemetry regions on this package."?
> + * @addrs: The MMIO addresses
"Array of MMIO addresses, one per telemetry region on this package."?
> + *
> + * Provides convenient access to all MMIO addresses of one event group
> + * for one package. Used when reading event data on a package.
> + */
> +struct mmio_info {
> + int count;
> + void __iomem *addrs[] __counted_by(count);
> +};
> +
> /**
> * struct event_group - All information about a group of telemetry events.
> * @pfg: Points to the aggregated telemetry space information
> * within the OOBMSM driver that contains data for all
> * telemetry regions.
> + * @pkginfo: Per-package MMIO addresses of telemetry regions belonging to this group
Sentence end with period.
> * @guid: Unique number per XML description file.
> * @mmio_size: Number of bytes of MMIO registers for this group.
> */
> struct event_group {
> /* Data fields used by this code. */
> struct pmt_feature_group *pfg;
> + struct mmio_info **pkginfo;
>
> /* Remaining fields initialized from XML file. */
> u32 guid;
> @@ -80,6 +95,20 @@ static bool skip_this_region(struct telemetry_region *tr, struct event_group *e)
> return false;
> }
>
> +static void free_mmio_info(struct mmio_info **mmi)
> +{
> + int num_pkgs = topology_max_packages();
> +
> + if (!mmi)
> + return;
> +
> + for (int i = 0; i < num_pkgs; i++)
> + kfree(mmi[i]);
> + kfree(mmi);
> +}
> +
> +DEFINE_FREE(mmio_info, struct mmio_info **, free_mmio_info(_T))
> +
> /*
> * Configure events from one pmt_feature_group.
> * 1) Count how many per package.
> @@ -87,10 +116,17 @@ static bool skip_this_region(struct telemetry_region *tr, struct event_group *e)
> */
> static int configure_events(struct event_group *e, struct pmt_feature_group *p)
> {
> + struct mmio_info __free(mmio_info) **pkginfo = NULL;
struct mmio_info **pkginfo __free(mmio_info) = NULL;
(checkpatch.pl)
> int *pkgcounts __free(kfree) = NULL;
> struct telemetry_region *tr;
> + struct mmio_info *mmi;
> int num_pkgs;
>
> + if (e->pkginfo) {
> + pr_warn("Duplicate telemetry information for guid 0x%x\n", e->guid);
This could be triggered by user space so may be safer with a "once".
> + return -EINVAL;
> + }
> +
> num_pkgs = topology_max_packages();
> pkgcounts = kcalloc(num_pkgs, sizeof(*pkgcounts), GFP_KERNEL);
> if (!pkgcounts)
> @@ -104,7 +140,33 @@ static int configure_events(struct event_group *e, struct pmt_feature_group *p)
> pkgcounts[tr->plat_info.package_id]++;
> }
>
> - return -EINVAL;
> + /* Allocate array for per-package struct mmio_info data */
> + pkginfo = kcalloc(num_pkgs, sizeof(*pkginfo), GFP_KERNEL);
> + if (!pkginfo)
> + return -ENOMEM;
> +
> + /*
> + * Allocate per-package mmio_info structures and initialize
> + * count of telemetry_regions in each one.
> + */
> + for (int i = 0; i < num_pkgs; i++) {
> + pkginfo[i] = kzalloc(struct_size(pkginfo[i], addrs, pkgcounts[i]), GFP_KERNEL);
> + if (!pkginfo[i])
> + return -ENOMEM;
> + pkginfo[i]->count = pkgcounts[i];
> + }
> +
> + /* Save MMIO address(es) for each telemetry region in per-package structures */
> + for (int i = 0; i < p->count; i++) {
> + tr = &p->regions[i];
> + if (skip_this_region(tr, e))
> + continue;
> + mmi = pkginfo[tr->plat_info.package_id];
> + mmi->addrs[--pkgcounts[tr->plat_info.package_id]] = tr->addr;
> + }
> + e->pkginfo = no_free_ptr(pkginfo);
> +
> + return 0;
> }
>
> DEFINE_FREE(intel_pmt_put_feature_group, struct pmt_feature_group *, \
> @@ -168,5 +230,6 @@ void __exit intel_aet_exit(void)
> intel_pmt_put_feature_group((*peg)->pfg);
> (*peg)->pfg = NULL;
> }
> + free_mmio_info((*peg)->pkginfo);
> }
> }
Reinette