Messages in this thread | | | From | "Shaopeng Tan (Fujitsu)" <> | Subject | RE: [PATCH v4 02/24] x86/resctrl: Access per-rmid structures by index | Date | Thu, 8 Jun 2023 07:52:43 +0000 |
| |
Hello James,
> Because of the differences between Intel RDT/AMD QoS and Arm's MPAM > monitors, RMID values on arm64 are not unique unless the CLOSID is also > included. Bitmaps like rmid_busy_llc need to be sized by the number of unique > entries for this resource. > > Add helpers to encode/decode the CLOSID and RMID to an index. The > domain's rmid_busy_llc and rmid_ptrs[] are then sized by index, as are the > domain mbm_local and mbm_total arrays. > On x86, the index is always just the RMID, so all these structures remain the > same size. > > The index gives resctrl a unique value it can use to store monitor values, and > allows MPAM to decode the CLOSID when reading the hardware counters. > > Tested-by: Shaopeng Tan <tan.shaopeng@fujitsu.com> > Signed-off-by: James Morse <james.morse@arm.com> > --- > Changes since v1: > * Added X86_BAD_CLOSID macro to make it clear what this value means > * Added second WARN_ON() for closid checking, and made both _ONCE() > > Changes since v2: > * Added RESCTRL_RESERVED_CLOSID > * Removed a newline > * Repharsed some comments > * Renamed a variable 'ignore'd > * Moved X86_RESCTRL_BAD_CLOSID to a previous patch > > Changes since v3: > * Changed a variable name > * Fixed various typos > --- > arch/x86/include/asm/resctrl.h | 17 ++++++ > arch/x86/kernel/cpu/resctrl/core.c | 2 +- > arch/x86/kernel/cpu/resctrl/internal.h | 1 + > arch/x86/kernel/cpu/resctrl/monitor.c | 84 > +++++++++++++++++--------- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 7 ++- > include/linux/resctrl.h | 3 + > 6 files changed, 83 insertions(+), 31 deletions(-) > > diff --git a/arch/x86/include/asm/resctrl.h b/arch/x86/include/asm/resctrl.h > index e906070285fb..dd9b638d43c8 100644 > --- a/arch/x86/include/asm/resctrl.h > +++ b/arch/x86/include/asm/resctrl.h > @@ -101,6 +101,23 @@ static inline void resctrl_sched_in(struct task_struct > *tsk) > __resctrl_sched_in(tsk); > } > > +static inline u32 resctrl_arch_system_num_rmid_idx(void) > +{ > + /* RMID are independent numbers for x86. num_rmid_idx == > num_rmid */ > + return boot_cpu_data.x86_cache_max_rmid + 1; } > + > +static inline void resctrl_arch_rmid_idx_decode(u32 idx, u32 *closid, > +u32 *rmid) { > + *rmid = idx; > + *closid = X86_RESCTRL_BAD_CLOSID; > +} > + > +static inline u32 resctrl_arch_rmid_idx_encode(u32 ignored, u32 rmid) { > + return rmid; > +} > + > void resctrl_cpu_detect(struct cpuinfo_x86 *c); > > #else > diff --git a/arch/x86/kernel/cpu/resctrl/core.c > b/arch/x86/kernel/cpu/resctrl/core.c > index 030d3b409768..4bea032d072e 100644 > --- a/arch/x86/kernel/cpu/resctrl/core.c > +++ b/arch/x86/kernel/cpu/resctrl/core.c > @@ -600,7 +600,7 @@ static void clear_closid_rmid(int cpu) > state->default_rmid = 0; > state->cur_closid = 0; > state->cur_rmid = 0; > - wrmsr(MSR_IA32_PQR_ASSOC, 0, 0); > + wrmsr(MSR_IA32_PQR_ASSOC, 0, RESCTRL_RESERVED_CLOSID); > } > > static int resctrl_online_cpu(unsigned int cpu) diff --git > a/arch/x86/kernel/cpu/resctrl/internal.h > b/arch/x86/kernel/cpu/resctrl/internal.h > index f2da908bb079..d571da4848a4 100644 > --- a/arch/x86/kernel/cpu/resctrl/internal.h > +++ b/arch/x86/kernel/cpu/resctrl/internal.h > @@ -7,6 +7,7 @@ > #include <linux/kernfs.h> > #include <linux/fs_context.h> > #include <linux/jump_label.h> > +#include <asm/resctrl.h> > > #define L3_QOS_CDP_ENABLE 0x01ULL > > diff --git a/arch/x86/kernel/cpu/resctrl/monitor.c > b/arch/x86/kernel/cpu/resctrl/monitor.c > index 86574adedd64..bcc25f5339c0 100644 > --- a/arch/x86/kernel/cpu/resctrl/monitor.c > +++ b/arch/x86/kernel/cpu/resctrl/monitor.c > @@ -142,12 +142,29 @@ static inline u64 get_corrected_mbm_count(u32 rmid, > unsigned long val) > return val; > } > > -static inline struct rmid_entry *__rmid_entry(u32 closid, u32 rmid) > +/* > + * x86 and arm64 differ in their handling of monitoring. > + * x86's RMID are an independent number, there is only one source of > +traffic > + * with an RMID value of '1'. > + * arm64's PMG extend the PARTID/CLOSID space, there are multiple > +sources of > + * traffic with a PMG value of '1', one for each CLOSID, meaning the > +RMID > + * value is no longer unique. > + * To account for this, resctrl uses an index. On x86 this is just the > +RMID, > + * on arm64 it encodes the CLOSID and RMID. This gives a unique number. > + * > + * The domain's rmid_busy_llc and rmid_ptrs are sized by index. The > +arch code > + * must accept an attempt to read every index. > + */ > +static inline struct rmid_entry *__rmid_entry(u32 idx) > { > struct rmid_entry *entry; > + u32 closid, rmid; > > - entry = &rmid_ptrs[rmid]; > - WARN_ON(entry->rmid != rmid); > + entry = &rmid_ptrs[idx]; > + resctrl_arch_rmid_idx_decode(idx, &closid, &rmid); > + > + WARN_ON_ONCE(entry->closid != closid); > + WARN_ON_ONCE(entry->rmid != rmid); > > return entry; > } > @@ -277,8 +294,9 @@ int resctrl_arch_rmid_read(struct rdt_resource *r, struct > rdt_domain *d, void __check_limbo(struct rdt_domain *d, bool force_free) { > struct rdt_resource *r = > &rdt_resources_all[RDT_RESOURCE_L3].r_resctrl; > + u32 idx_limit = resctrl_arch_system_num_rmid_idx(); > struct rmid_entry *entry; > - u32 crmid = 1, nrmid; > + u32 idx, cur_idx = 1; > bool rmid_dirty; > u64 val = 0; > > @@ -289,12 +307,11 @@ void __check_limbo(struct rdt_domain *d, bool > force_free) > * RMID and move it to the free list when the counter reaches 0. > */ > for (;;) { > - nrmid = find_next_bit(d->rmid_busy_llc, r->num_rmid, > crmid); > - if (nrmid >= r->num_rmid) > + idx = find_next_bit(d->rmid_busy_llc, idx_limit, cur_idx); > + if (idx >= idx_limit) > break; > > - entry = __rmid_entry(X86_RESCTRL_BAD_CLOSID, nrmid);// > temporary > - > + entry = __rmid_entry(idx); > if (resctrl_arch_rmid_read(r, d, entry->closid, entry->rmid, > QOS_L3_OCCUP_EVENT_ID, > &val)) { > rmid_dirty = true; > @@ -303,19 +320,21 @@ void __check_limbo(struct rdt_domain *d, bool > force_free) > } > > if (force_free || !rmid_dirty) { > - clear_bit(entry->rmid, d->rmid_busy_llc); > + clear_bit(idx, d->rmid_busy_llc); > if (!--entry->busy) { > rmid_limbo_count--; > list_add_tail(&entry->list, &rmid_free_lru); > } > } > - crmid = nrmid + 1; > + cur_idx = idx + 1; > } > } > > bool has_busy_rmid(struct rdt_resource *r, struct rdt_domain *d) { > - return find_first_bit(d->rmid_busy_llc, r->num_rmid) != > r->num_rmid; > + u32 idx_limit = resctrl_arch_system_num_rmid_idx(); > + > + return find_first_bit(d->rmid_busy_llc, idx_limit) != idx_limit; > }
"struct rdt_resource *r" is not used in this function.
Best regards, Shaopeng TAN
| |