Re: [RFC PATCH 0/2] Resctrl - rewrite (WIP)

Drew Fustini
2023-06-27T08:43:04Z

On Mon, Jun 19, 2023 at 08:37:00PM -0700, Tony Luck wrote:
> Back in April I posted some RFC patches that added a "driver
> registration" interface to the core resctrl code so that additional
> resource control and monitor features could be added without further
> complicating the core code. Link to that discussion:
>
> https://lore.kernel.org/all/20230420220636.53527-1-tony.luck@intel.com/
>
> Reinette gave the feedback that it would be better to base the module
> registration on the resctrl resource structure. Reinette also pointed
> me to work from James Morse, and some additional discussion happened
> here:
>
> https://lore.kernel.org/all/ZG%2FMZVrWYrCHm%2Ffr@agluck-desk3/
>
> James provided details on where ARM's MPAM has similarities and
> differences from the Intel Resource Director Technology and AMD's
> similar implementation. Drew Fustini was also pulled into that
> conversation to comment on RISC-V CBQRI.
>
> >From those discussions I believed we need a do-over on the core
> /sys/fs/resctrl implementation to make it friendlier for architecural
> variations. Here's what I have so far.
>
> =========================================================================
> | N.B. This is a general direction check. There are many obvious |
> | rough edges (e.g. some careful thought needs to happen on locking |
> | for the files in /sys/fs/resctrl that are "owned" by modules that |
> | can be unloaded). I'm mostly looking for feedback from AMD, ARM and |
> | RISCV on whether this is a foundation to build on, whether some small |
> | tweaks could make it better, or if this is still going to be really |
> | hard for architectures that have radical divergence from the Intel |
> | model. |
> =========================================================================
>
> First patch is my attempt at architecture neutral code. All mention
> of "RDT", "CLOSID" and "RMID" have been expunged. When creating a
> new group this code calls arch_alloc_resctrl_ids() to allocate an
> opaque "resctrl_ids" value.
>
> Q: I made this a "u64" because that neatly allows storage of both an
> x86 CLOSID and RMID (in a handy representation that matches the bit
> layout of the Intel IA32_PQR_ASSOC model specific register). If other
> architectures need something more complex it could be a "typedef
> resctrl_id_t" ... there are a couple of places where we would need
> a comparison function.

This works okay for RISC-V. The Ssqosid extension defines a 32-bit
register sqoscfg (see chapter 2 of CBQRI spec [0]). This contains a
12-bit MCID field (similar to an RMID) and 12-bit RCID field (similar to
an CLOSID).

>
> I broke the code into several source files that handle different
> sub-functions of core code to make it easier to navigate. Much of
> the code here should look familiar as I did a lot of
> s/rdtgroup/resctrl_group/ on functions from the original resctrl
> code.
>
> By itself the core code is useless. Cannot even be built as the
> controlling Kconfig option "CONFIG_RESCTRL2_FS" must be invoked by
> a "select" request from architecture specific code that provides
> the necessary "arch_*()" functions to make everything work.

I would like to try to rebase the RISC-V CBQRI resctrl RFC [1] on top of
this patch series instead of the mpam snapshot branch [2].

I had a patch in my RFC that added config option RISCV_ISA_SSQOSID which
selects ARCH_HAS_CPU_RESCTRL and RESCTRL_FS [3]. It seems I would need
to change that to select CONFIG_RESCTRL2_FS ?

A patch [4] in that RFC adds the "arch_*()" functions in
arch/riscv/kernel/qos/qos_resctrl.c

thanks,
drew

[0] https://github.com/riscv-non-isa/riscv-cbqri/blob/main/riscv-cbqri.pdf
[1] https://lore.kernel.org/linux-riscv/20230419111111.477118-1-dfustini@baylibre.com/
[2] https://gitlab.arm.com/linux-arm/linux-jm/-/tree/mpam/snaphot/20230406
[3] https://lore.kernel.org/linux-riscv/20230419111111.477118-11-dfustini@baylibre.com/
[4] https://lore.kernel.org/linux-riscv/20230419111111.477118-8-dfustini@baylibre.com/