* [RFC PATCH 0/2] RISC-V: Detect Ssqosid extension and handle sqoscfg CSR @ 2023-04-10 4:36 Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 1/2] RISC-V: Detect the Ssqosid extension Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR Drew Fustini 0 siblings, 2 replies; 6+ messages in thread From: Drew Fustini @ 2023-04-10 4:36 UTC (permalink / raw) To: linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Conor Dooley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi Cc: Drew Fustini This RFC series adds initial support for the Ssqosid extension and the sqoscfg CSR as specified in Chapter 2 of the RISC-V Capacity and Bandwidth Controller QoS Register Interface (CBQRI) specification [1]. QoS (Quality of Service) in this context is concerned with shared resources on an SoC such as cache capacity and memory bandwidth. Intel and AMD already have QoS features on x86, and there is an existing user interface in Linux: the resctrl virtual filesystem [2]. The sqoscfg CSR provides a mechanism by which a software workload (e.g. a process or a set of processes) can be associated with a resource control ID (RCID) and a monitoring counter ID (MCID) that accompanies each request made by the hart to shared resources like cache. CBQRI defines operations to configure resource usage limits, in the form of capacity or bandwidth, for an RCID. CBQRI also defines operations to configure counters to track the resource utilization of an MCID. The CBQRI spec is still in draft state and is undergoing review [3]. It is possible there will be changes to the Ssqosid extension and the CBQRI spec. For example, the CSR address for sqoscfg is not yet finalized. My goal for this RFC is to determine if the 2nd patch is an acceptable approach to handling sqoscfg when switching tasks. This RFC was tested against a QEMU branch that implements the Ssqosid extension [4]. A test driver [5] was used to set thread_struct.sqoscfg for the current process. This allowed __qos_sched_in() to be tested without resctrl. A forthcoming RFC series will enable resctrl to support systems that implement CBQRI and will reference a QEMU branch that implements CBQRI. NOTE: CBQRI was previously known as CMQRI and the github repo with the spec has not yet been renamed [6]. [1] https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf [2] https://docs.kernel.org/x86/resctrl.html [3] https://lists.riscv.org/g/tech-cmqri/message/38 [4] https://github.com/pdp7/qemu/tree/ssqosid_v8.0.0-rc1 [5] https://github.com/pdp7/linux/commits/ssqosid_sqoscfg_v6.3 [6] https://lists.riscv.org/g/tech-cmqri/message/41 Drew Fustini (1): RISC-V: Add support for sqoscfg CSR Kornel Dulęba (1): RISC-V: Detect the Ssqosid extension arch/riscv/Kconfig | 19 ++++++++++++++ arch/riscv/include/asm/csr.h | 8 ++++++ arch/riscv/include/asm/hwcap.h | 2 ++ arch/riscv/include/asm/processor.h | 3 +++ arch/riscv/include/asm/qos.h | 40 ++++++++++++++++++++++++++++++ arch/riscv/include/asm/switch_to.h | 2 ++ arch/riscv/kernel/cpu.c | 1 + arch/riscv/kernel/cpufeature.c | 1 + 8 files changed, 76 insertions(+) create mode 100644 arch/riscv/include/asm/qos.h base-commit: d34a6b715a23ccd9c9d0bc7a475bea59dc3e28b2 -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* [RFC PATCH 1/2] RISC-V: Detect the Ssqosid extension 2023-04-10 4:36 [RFC PATCH 0/2] RISC-V: Detect Ssqosid extension and handle sqoscfg CSR Drew Fustini @ 2023-04-10 4:36 ` Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR Drew Fustini 1 sibling, 0 replies; 6+ messages in thread From: Drew Fustini @ 2023-04-10 4:36 UTC (permalink / raw) To: linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Conor Dooley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi Cc: Drew Fustini From: Kornel Dulęba <mindal@semihalf.com> Detect the Ssqosid extension (Supervisor-mode Quality of Service ID) as defined in the CBQRI (Capacity and Bandwidth QoS Register Interface) specification. Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf Signed-off-by: Kornel Dulęba <mindal@semihalf.com> [dfustini: rebase from v6.0 to v6.3] Signed-off-by: Drew Fustini <dfustini@baylibre.com> --- Note: the Ssqosid extension and CBQRI spec are still in a draft state. arch/riscv/include/asm/hwcap.h | 2 ++ arch/riscv/kernel/cpu.c | 1 + arch/riscv/kernel/cpufeature.c | 1 + 3 files changed, 4 insertions(+) diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h index 9af793970855..6dd717030723 100644 --- a/arch/riscv/include/asm/hwcap.h +++ b/arch/riscv/include/asm/hwcap.h @@ -44,6 +44,8 @@ #define RISCV_ISA_EXT_ZIHINTPAUSE 32 #define RISCV_ISA_EXT_SVNAPOT 33 #define RISCV_ISA_EXT_ZICBOZ 34 +#define RISCV_ISA_EXT_SSQOSID 35 + #define RISCV_ISA_EXT_MAX 64 #define RISCV_ISA_EXT_NAME_LEN_MAX 32 diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index 9203e18320f9..04bd0a21910e 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -190,6 +190,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = { __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE), __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB), __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF), + __RISCV_ISA_EXT_DATA(ssqosid, RISCV_ISA_EXT_SSQOSID), __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC), __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL), __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT), diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 00d7cd2c9043..94bf5556347d 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -226,6 +226,7 @@ void __init riscv_fill_hwcap(void) } else { /* sorted alphabetically */ SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF); + SET_ISA_EXT_MAP("ssqosid", RISCV_ISA_EXT_SSQOSID); SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC); SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL); SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT); -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR 2023-04-10 4:36 [RFC PATCH 0/2] RISC-V: Detect Ssqosid extension and handle sqoscfg CSR Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 1/2] RISC-V: Detect the Ssqosid extension Drew Fustini @ 2023-04-10 4:36 ` Drew Fustini 2023-04-17 19:10 ` Conor Dooley 1 sibling, 1 reply; 6+ messages in thread From: Drew Fustini @ 2023-04-10 4:36 UTC (permalink / raw) To: linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Conor Dooley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi Cc: Drew Fustini Add support for the sqoscfg CSR defined in the Ssqosid ISA extension (Supervisor-mode Quality of Service ID). The CSR contains two fields: - Resource Control ID (RCID) used determine resource allocation - Monitoring Counter ID (MCID) used to track resource usage Requests from a hart to shared resources like cache will be tagged with these IDs. This allows the usage of shared resources to be associated with the task currently running on the hart. A sqoscfg field is added to thread_struct and has the same format as the sqoscfg CSR. This allows the scheduler to set the hart's sqoscfg CSR to contain the RCID and MCID for the task that is being scheduled in. The sqoscfg CSR is only written to if the thread_struct.sqoscfg is different from the current value of the CSR. A per-cpu variable cpu_sqoscfg is used to mirror that state of the CSR. This is because access to L1D hot memory should be several times faster than a CSR read. Also, in the case of virtualization, accesses to this CSR are trapped in the hypervisor. Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf Co-developed-by: Kornel Dulęba <mindal@semihalf.com> Signed-off-by: Kornel Dulęba <mindal@semihalf.com> Signed-off-by: Drew Fustini <dfustini@baylibre.com> --- Note: the Ssqosid extension and CBQRI spec are still in a draft state. The CSR address of sqoscfg is not final. Changes from the original patch [1]: - Rebase from 6.0 to 6.3 - Simplify per-cpu variable from struct to u32 with just sqoscfg - Move qoscfg to thread_struct in arch/riscv/include/asm/processor.h This avoids changing task_struct in /include/linux/sched.h - Reword commit description - Reword Kconfig description [1] https://github.com/rivosinc/linux/commit/8454b793a62be21d39e5826ef5241dfa06198ba9 arch/riscv/Kconfig | 19 ++++++++++++++ arch/riscv/include/asm/csr.h | 8 ++++++ arch/riscv/include/asm/processor.h | 3 +++ arch/riscv/include/asm/qos.h | 40 ++++++++++++++++++++++++++++++ arch/riscv/include/asm/switch_to.h | 2 ++ 5 files changed, 72 insertions(+) create mode 100644 arch/riscv/include/asm/qos.h diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index cc02eb9eee1f..03f22b7fe34b 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -418,6 +418,25 @@ config RISCV_ISA_SVNAPOT If you don't know what to do here, say Y. +config RISCV_ISA_SSQOSID + bool "Ssqosid extension support" + default y + help + Adds support for the Ssqosid ISA extension (Supervisor-mode + Quality of Service ID). + + Ssqosid defines the sqoscfg CSR which allows the system to tag + the running process with RCID (Resource Control ID) and MCID + (Monitoring Counter ID). The RCID is used determine resource + allocation. The MCID is used to track resource usage in event + counters. + + For example, a cache controller may use the RCID to apply a + cache partitioning scheme and use the MCID to track how much + cache a process, or a group of processes, is using. + + If you don't know what to do here, say Y. + config RISCV_ISA_SVPBMT bool "SVPBMT extension support" depends on 64BIT && MMU diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h index 7c2b8cdb7b77..17d04a0cacd6 100644 --- a/arch/riscv/include/asm/csr.h +++ b/arch/riscv/include/asm/csr.h @@ -59,6 +59,13 @@ #define SATP_ASID_MASK _AC(0xFFFF, UL) #endif +/* SQOSCFG fields */ +#define SQOSCFG_RCID_MASK _AC(0x00000FFF, UL) +#define SQOSCFG_MCID_MASK SQOSCFG_RCID_MASK +#define SQOSCFG_MCID_SHIFT 16 +#define SQOSCFG_MASK ((SQOSCFG_MCID_MASK << SQOSCFG_MCID_SHIFT) | \ + SQOSCFG_RCID_MASK) + /* Exception cause high bit - is an interrupt if set */ #define CAUSE_IRQ_FLAG (_AC(1, UL) << (__riscv_xlen - 1)) @@ -245,6 +252,7 @@ #define CSR_STVAL 0x143 #define CSR_SIP 0x144 #define CSR_SATP 0x180 +#define CSR_SQOSCFG 0x181 #define CSR_STIMECMP 0x14D #define CSR_STIMECMPH 0x15D diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h index 94a0590c6971..724b2aa2732d 100644 --- a/arch/riscv/include/asm/processor.h +++ b/arch/riscv/include/asm/processor.h @@ -39,6 +39,9 @@ struct thread_struct { unsigned long s[12]; /* s[0]: frame pointer */ struct __riscv_d_ext_state fstate; unsigned long bad_cause; +#ifdef CONFIG_RISCV_ISA_SSQOSID + u32 sqoscfg; +#endif }; /* Whitelist the fstate from the task_struct for hardened usercopy */ diff --git a/arch/riscv/include/asm/qos.h b/arch/riscv/include/asm/qos.h new file mode 100644 index 000000000000..297e7fb64d80 --- /dev/null +++ b/arch/riscv/include/asm/qos.h @@ -0,0 +1,40 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_QOS_H +#define _ASM_RISCV_QOS_H + +#ifdef CONFIG_RISCV_ISA_SSQOSID + +#include <linux/sched.h> +#include <linux/jump_label.h> + +#include <asm/barrier.h> +#include <asm/csr.h> +#include <asm/hwcap.h> + +/* cached value of sqoscfg csr for each cpu */ +static DEFINE_PER_CPU(u32, cpu_sqoscfg); + +static void __qos_sched_in(struct task_struct *task) +{ + u32 *cpu_sqoscfg_ptr = this_cpu_ptr(&cpu_sqoscfg); + u32 thread_sqoscfg; + + thread_sqoscfg = READ_ONCE(task->thread.sqoscfg); + + if (thread_sqoscfg != *cpu_sqoscfg_ptr) { + *cpu_sqoscfg_ptr = thread_sqoscfg; + csr_write(CSR_SQOSCFG, thread_sqoscfg); + } +} + +static inline void qos_sched_in(struct task_struct *task) +{ + if (riscv_has_extension_likely(RISCV_ISA_EXT_SSQOSID)) + __qos_sched_in(task); +} +#else + +static inline void qos_sched_in(struct task_struct *task) {} + +#endif /* CONFIG_RISCV_ISA_SSQOSID */ +#endif /* _ASM_RISCV_QOS_H */ diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h index 60f8ca01d36e..75d9bfd766af 100644 --- a/arch/riscv/include/asm/switch_to.h +++ b/arch/riscv/include/asm/switch_to.h @@ -12,6 +12,7 @@ #include <asm/processor.h> #include <asm/ptrace.h> #include <asm/csr.h> +#include <asm/qos.h> #ifdef CONFIG_FPU extern void __fstate_save(struct task_struct *save_to); @@ -79,6 +80,7 @@ do { \ if (has_fpu()) \ __switch_to_aux(__prev, __next); \ ((last) = __switch_to(__prev, __next)); \ + qos_sched_in(__next); \ } while (0) #endif /* _ASM_RISCV_SWITCH_TO_H */ -- 2.34.1 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR 2023-04-10 4:36 ` [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR Drew Fustini @ 2023-04-17 19:10 ` Conor Dooley 2023-04-17 20:04 ` Drew Fustini 0 siblings, 1 reply; 6+ messages in thread From: Conor Dooley @ 2023-04-17 19:10 UTC (permalink / raw) To: Drew Fustini Cc: linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Conor Dooley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi [-- Attachment #1.1: Type: text/plain, Size: 7816 bytes --] Hey Drew, (Don't get your hopes up, I don't have anything really meaningful to contribute, sorry.) On Sun, Apr 09, 2023 at 09:36:46PM -0700, Drew Fustini wrote: > Add support for the sqoscfg CSR defined in the Ssqosid ISA extension > (Supervisor-mode Quality of Service ID). The CSR contains two fields: > > - Resource Control ID (RCID) used determine resource allocation > - Monitoring Counter ID (MCID) used to track resource usage > > Requests from a hart to shared resources like cache will be tagged with > these IDs. This allows the usage of shared resources to be associated > with the task currently running on the hart. > > A sqoscfg field is added to thread_struct and has the same format as the > sqoscfg CSR. This allows the scheduler to set the hart's sqoscfg CSR to > contain the RCID and MCID for the task that is being scheduled in. The > sqoscfg CSR is only written to if the thread_struct.sqoscfg is different > from the current value of the CSR. > > A per-cpu variable cpu_sqoscfg is used to mirror that state of the CSR. > This is because access to L1D hot memory should be several times faster > than a CSR read. Also, in the case of virtualization, accesses to this > CSR are trapped in the hypervisor. > > Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf > Co-developed-by: Kornel Dulęba <mindal@semihalf.com> > Signed-off-by: Kornel Dulęba <mindal@semihalf.com> > Signed-off-by: Drew Fustini <dfustini@baylibre.com> > --- > Note: the Ssqosid extension and CBQRI spec are still in a draft state. > The CSR address of sqoscfg is not final. > > Changes from the original patch [1]: > - Rebase from 6.0 to 6.3 > - Simplify per-cpu variable from struct to u32 with just sqoscfg > - Move qoscfg to thread_struct in arch/riscv/include/asm/processor.h > This avoids changing task_struct in /include/linux/sched.h > - Reword commit description > - Reword Kconfig description > > [1] https://github.com/rivosinc/linux/commit/8454b793a62be21d39e5826ef5241dfa06198ba9 > > arch/riscv/Kconfig | 19 ++++++++++++++ > arch/riscv/include/asm/csr.h | 8 ++++++ > arch/riscv/include/asm/processor.h | 3 +++ > arch/riscv/include/asm/qos.h | 40 ++++++++++++++++++++++++++++++ > arch/riscv/include/asm/switch_to.h | 2 ++ > 5 files changed, 72 insertions(+) > create mode 100644 arch/riscv/include/asm/qos.h > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index cc02eb9eee1f..03f22b7fe34b 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -418,6 +418,25 @@ config RISCV_ISA_SVNAPOT > > If you don't know what to do here, say Y. > > +config RISCV_ISA_SSQOSID > + bool "Ssqosid extension support" > + default y > + help > + Adds support for the Ssqosid ISA extension (Supervisor-mode > + Quality of Service ID). Could you add "long form" text in brackets here to the bool line, a la: https://patchwork.kernel.org/project/linux-riscv/patch/20230405-pucker-cogwheel-3a999a94a2f2@wendy/ > + > + Ssqosid defines the sqoscfg CSR which allows the system to tag > + the running process with RCID (Resource Control ID) and MCID > + (Monitoring Counter ID). The RCID is used determine resource > + allocation. The MCID is used to track resource usage in event > + counters. > + > + For example, a cache controller may use the RCID to apply a > + cache partitioning scheme and use the MCID to track how much > + cache a process, or a group of processes, is using. > + > + If you don't know what to do here, say Y. > + > config RISCV_ISA_SVPBMT > bool "SVPBMT extension support" > depends on 64BIT && MMU > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h > index 7c2b8cdb7b77..17d04a0cacd6 100644 > --- a/arch/riscv/include/asm/csr.h > +++ b/arch/riscv/include/asm/csr.h > @@ -59,6 +59,13 @@ > #define SATP_ASID_MASK _AC(0xFFFF, UL) > #endif > > +/* SQOSCFG fields */ > +#define SQOSCFG_RCID_MASK _AC(0x00000FFF, UL) > +#define SQOSCFG_MCID_MASK SQOSCFG_RCID_MASK > +#define SQOSCFG_MCID_SHIFT 16 > +#define SQOSCFG_MASK ((SQOSCFG_MCID_MASK << SQOSCFG_MCID_SHIFT) | \ > + SQOSCFG_RCID_MASK) > + > /* Exception cause high bit - is an interrupt if set */ > #define CAUSE_IRQ_FLAG (_AC(1, UL) << (__riscv_xlen - 1)) > > @@ -245,6 +252,7 @@ > #define CSR_STVAL 0x143 > #define CSR_SIP 0x144 > #define CSR_SATP 0x180 > +#define CSR_SQOSCFG 0x181 > > #define CSR_STIMECMP 0x14D > #define CSR_STIMECMPH 0x15D > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > index 94a0590c6971..724b2aa2732d 100644 > --- a/arch/riscv/include/asm/processor.h > +++ b/arch/riscv/include/asm/processor.h > @@ -39,6 +39,9 @@ struct thread_struct { > unsigned long s[12]; /* s[0]: frame pointer */ > struct __riscv_d_ext_state fstate; > unsigned long bad_cause; > +#ifdef CONFIG_RISCV_ISA_SSQOSID > + u32 sqoscfg; > +#endif > }; > > /* Whitelist the fstate from the task_struct for hardened usercopy */ > diff --git a/arch/riscv/include/asm/qos.h b/arch/riscv/include/asm/qos.h > new file mode 100644 > index 000000000000..297e7fb64d80 > --- /dev/null > +++ b/arch/riscv/include/asm/qos.h > @@ -0,0 +1,40 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef _ASM_RISCV_QOS_H > +#define _ASM_RISCV_QOS_H > + > +#ifdef CONFIG_RISCV_ISA_SSQOSID > + > +#include <linux/sched.h> > +#include <linux/jump_label.h> > + > +#include <asm/barrier.h> > +#include <asm/csr.h> > +#include <asm/hwcap.h> > + > +/* cached value of sqoscfg csr for each cpu */ > +static DEFINE_PER_CPU(u32, cpu_sqoscfg); > + > +static void __qos_sched_in(struct task_struct *task) > +{ > + u32 *cpu_sqoscfg_ptr = this_cpu_ptr(&cpu_sqoscfg); > + u32 thread_sqoscfg; > + > + thread_sqoscfg = READ_ONCE(task->thread.sqoscfg); > + > + if (thread_sqoscfg != *cpu_sqoscfg_ptr) { > + *cpu_sqoscfg_ptr = thread_sqoscfg; > + csr_write(CSR_SQOSCFG, thread_sqoscfg); > + } > +} > + > +static inline void qos_sched_in(struct task_struct *task) "qos" is a pretty generic prefix, no? Do you think we'd be better off prefixing this (and every other extension related thing) with `riscv_`? > +{ > + if (riscv_has_extension_likely(RISCV_ISA_EXT_SSQOSID)) > + __qos_sched_in(task); > +} > +#else > + > +static inline void qos_sched_in(struct task_struct *task) {} > + > +#endif /* CONFIG_RISCV_ISA_SSQOSID */ > +#endif /* _ASM_RISCV_QOS_H */ > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > index 60f8ca01d36e..75d9bfd766af 100644 > --- a/arch/riscv/include/asm/switch_to.h > +++ b/arch/riscv/include/asm/switch_to.h > @@ -12,6 +12,7 @@ > #include <asm/processor.h> > #include <asm/ptrace.h> > #include <asm/csr.h> > +#include <asm/qos.h> > > #ifdef CONFIG_FPU > extern void __fstate_save(struct task_struct *save_to); > @@ -79,6 +80,7 @@ do { \ > if (has_fpu()) \ > __switch_to_aux(__prev, __next); \ > ((last) = __switch_to(__prev, __next)); \ > + qos_sched_in(__next); \ Both FPU and vector do: | if (has_fpu()) \ | __switch_to_fpu(__prev, __next); \ | if (has_vector()) \ | __switch_to_vector(__prev, __next); \ Is it just my OCD that wants ssqosid to be the same? It'd also do away with that seems a bit weird to me: having qos_sched_in() and __qos_sched_in(). Even if you don't make them similar, what's the rationale behind not inverting the extension check & returning early from a single function. This is kinda above my pay grade, so let me know what I've inevitably missed, Conor. > > #endif /* _ASM_RISCV_SWITCH_TO_H */ > -- > 2.34.1 > [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR 2023-04-17 19:10 ` Conor Dooley @ 2023-04-17 20:04 ` Drew Fustini 2023-04-21 9:34 ` Conor Dooley 0 siblings, 1 reply; 6+ messages in thread From: Drew Fustini @ 2023-04-17 20:04 UTC (permalink / raw) To: Conor Dooley Cc: linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Conor Dooley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi On Mon, Apr 17, 2023 at 08:10:20PM +0100, Conor Dooley wrote: > Hey Drew, > > (Don't get your hopes up, I don't have anything really meaningful to > contribute, sorry.) Hi, thanks for the feedback... > > On Sun, Apr 09, 2023 at 09:36:46PM -0700, Drew Fustini wrote: > > Add support for the sqoscfg CSR defined in the Ssqosid ISA extension > > (Supervisor-mode Quality of Service ID). The CSR contains two fields: > > > > - Resource Control ID (RCID) used determine resource allocation > > - Monitoring Counter ID (MCID) used to track resource usage > > > > Requests from a hart to shared resources like cache will be tagged with > > these IDs. This allows the usage of shared resources to be associated > > with the task currently running on the hart. > > > > A sqoscfg field is added to thread_struct and has the same format as the > > sqoscfg CSR. This allows the scheduler to set the hart's sqoscfg CSR to > > contain the RCID and MCID for the task that is being scheduled in. The > > sqoscfg CSR is only written to if the thread_struct.sqoscfg is different > > from the current value of the CSR. > > > > A per-cpu variable cpu_sqoscfg is used to mirror that state of the CSR. > > This is because access to L1D hot memory should be several times faster > > than a CSR read. Also, in the case of virtualization, accesses to this > > CSR are trapped in the hypervisor. > > > > Link: https://github.com/riscv-non-isa/riscv-cmqri/blob/main/riscv-cbqri.pdf > > Co-developed-by: Kornel Dulęba <mindal@semihalf.com> > > Signed-off-by: Kornel Dulęba <mindal@semihalf.com> > > Signed-off-by: Drew Fustini <dfustini@baylibre.com> > > --- > > Note: the Ssqosid extension and CBQRI spec are still in a draft state. > > The CSR address of sqoscfg is not final. > > > > Changes from the original patch [1]: > > - Rebase from 6.0 to 6.3 > > - Simplify per-cpu variable from struct to u32 with just sqoscfg > > - Move qoscfg to thread_struct in arch/riscv/include/asm/processor.h > > This avoids changing task_struct in /include/linux/sched.h > > - Reword commit description > > - Reword Kconfig description > > > > [1] https://github.com/rivosinc/linux/commit/8454b793a62be21d39e5826ef5241dfa06198ba9 > > > > arch/riscv/Kconfig | 19 ++++++++++++++ > > arch/riscv/include/asm/csr.h | 8 ++++++ > > arch/riscv/include/asm/processor.h | 3 +++ > > arch/riscv/include/asm/qos.h | 40 ++++++++++++++++++++++++++++++ > > arch/riscv/include/asm/switch_to.h | 2 ++ > > 5 files changed, 72 insertions(+) > > create mode 100644 arch/riscv/include/asm/qos.h > > > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > > index cc02eb9eee1f..03f22b7fe34b 100644 > > --- a/arch/riscv/Kconfig > > +++ b/arch/riscv/Kconfig > > @@ -418,6 +418,25 @@ config RISCV_ISA_SVNAPOT > > > > If you don't know what to do here, say Y. > > > > +config RISCV_ISA_SSQOSID > > + bool "Ssqosid extension support" > > + default y > > + help > > + Adds support for the Ssqosid ISA extension (Supervisor-mode > > + Quality of Service ID). > > Could you add "long form" text in brackets here to the bool line, a la: > https://patchwork.kernel.org/project/linux-riscv/patch/20230405-pucker-cogwheel-3a999a94a2f2@wendy/ Thanks, that is a good idea. It would make it easier for the person using menuconfig as they won't have to access the help screen just to see what the acronym stands for. > > + > > + Ssqosid defines the sqoscfg CSR which allows the system to tag > > + the running process with RCID (Resource Control ID) and MCID > > + (Monitoring Counter ID). The RCID is used determine resource > > + allocation. The MCID is used to track resource usage in event > > + counters. > > + > > + For example, a cache controller may use the RCID to apply a > > + cache partitioning scheme and use the MCID to track how much > > + cache a process, or a group of processes, is using. > > + > > + If you don't know what to do here, say Y. > > + > > config RISCV_ISA_SVPBMT > > bool "SVPBMT extension support" > > depends on 64BIT && MMU > > diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h > > index 7c2b8cdb7b77..17d04a0cacd6 100644 > > --- a/arch/riscv/include/asm/csr.h > > +++ b/arch/riscv/include/asm/csr.h > > @@ -59,6 +59,13 @@ > > #define SATP_ASID_MASK _AC(0xFFFF, UL) > > #endif > > > > +/* SQOSCFG fields */ > > +#define SQOSCFG_RCID_MASK _AC(0x00000FFF, UL) > > +#define SQOSCFG_MCID_MASK SQOSCFG_RCID_MASK > > +#define SQOSCFG_MCID_SHIFT 16 > > +#define SQOSCFG_MASK ((SQOSCFG_MCID_MASK << SQOSCFG_MCID_SHIFT) | \ > > + SQOSCFG_RCID_MASK) > > + > > /* Exception cause high bit - is an interrupt if set */ > > #define CAUSE_IRQ_FLAG (_AC(1, UL) << (__riscv_xlen - 1)) > > > > @@ -245,6 +252,7 @@ > > #define CSR_STVAL 0x143 > > #define CSR_SIP 0x144 > > #define CSR_SATP 0x180 > > +#define CSR_SQOSCFG 0x181 > > > > #define CSR_STIMECMP 0x14D > > #define CSR_STIMECMPH 0x15D > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > > index 94a0590c6971..724b2aa2732d 100644 > > --- a/arch/riscv/include/asm/processor.h > > +++ b/arch/riscv/include/asm/processor.h > > @@ -39,6 +39,9 @@ struct thread_struct { > > unsigned long s[12]; /* s[0]: frame pointer */ > > struct __riscv_d_ext_state fstate; > > unsigned long bad_cause; > > +#ifdef CONFIG_RISCV_ISA_SSQOSID > > + u32 sqoscfg; > > +#endif > > }; > > > > /* Whitelist the fstate from the task_struct for hardened usercopy */ > > diff --git a/arch/riscv/include/asm/qos.h b/arch/riscv/include/asm/qos.h > > new file mode 100644 > > index 000000000000..297e7fb64d80 > > --- /dev/null > > +++ b/arch/riscv/include/asm/qos.h > > @@ -0,0 +1,40 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef _ASM_RISCV_QOS_H > > +#define _ASM_RISCV_QOS_H > > + > > +#ifdef CONFIG_RISCV_ISA_SSQOSID > > + > > +#include <linux/sched.h> > > +#include <linux/jump_label.h> > > + > > +#include <asm/barrier.h> > > +#include <asm/csr.h> > > +#include <asm/hwcap.h> > > + > > +/* cached value of sqoscfg csr for each cpu */ > > +static DEFINE_PER_CPU(u32, cpu_sqoscfg); > > + > > +static void __qos_sched_in(struct task_struct *task) > > +{ > > + u32 *cpu_sqoscfg_ptr = this_cpu_ptr(&cpu_sqoscfg); > > + u32 thread_sqoscfg; > > + > > + thread_sqoscfg = READ_ONCE(task->thread.sqoscfg); > > + > > + if (thread_sqoscfg != *cpu_sqoscfg_ptr) { > > + *cpu_sqoscfg_ptr = thread_sqoscfg; > > + csr_write(CSR_SQOSCFG, thread_sqoscfg); > > + } > > +} > > + > > +static inline void qos_sched_in(struct task_struct *task) > > "qos" is a pretty generic prefix, no? Do you think we'd be better off > prefixing this (and every other extension related thing) with `riscv_`? That's a fair point. No harm in adding the 'riscv_' to make the context clearer. > > > +{ > > + if (riscv_has_extension_likely(RISCV_ISA_EXT_SSQOSID)) > > + __qos_sched_in(task); > > +} > > +#else > > + > > +static inline void qos_sched_in(struct task_struct *task) {} > > + > > +#endif /* CONFIG_RISCV_ISA_SSQOSID */ > > +#endif /* _ASM_RISCV_QOS_H */ > > diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h > > index 60f8ca01d36e..75d9bfd766af 100644 > > --- a/arch/riscv/include/asm/switch_to.h > > +++ b/arch/riscv/include/asm/switch_to.h > > @@ -12,6 +12,7 @@ > > #include <asm/processor.h> > > #include <asm/ptrace.h> > > #include <asm/csr.h> > > +#include <asm/qos.h> > > > > #ifdef CONFIG_FPU > > extern void __fstate_save(struct task_struct *save_to); > > @@ -79,6 +80,7 @@ do { \ > > if (has_fpu()) \ > > __switch_to_aux(__prev, __next); \ > > ((last) = __switch_to(__prev, __next)); \ > > + qos_sched_in(__next); \ > > Both FPU and vector do: > | if (has_fpu()) \ > | __switch_to_fpu(__prev, __next); \ > | if (has_vector()) \ > | __switch_to_vector(__prev, __next); \ > > Is it just my OCD that wants ssqosid to be the same? Thanks, I had not noticed those changes from the vector patch series [1] until you pointed it out. The handling of sqoscfg could be converted to that pattern too. > It'd also do away with that seems a bit weird to me: having > qos_sched_in() and __qos_sched_in(). > Even if you don't make them similar, what's the rationale behind not > inverting the extension check & returning early from a single function. The goal was so the inline qos_sched_in() would turn into a nop when Ssqosid extensions not present. If Ssqosid was present, then the "real" function __qos_sched_in() would be called. However, having looked at the handling of fpu and vector in the vector series, I think will redo the sqoscfg handling to follow that pattern. thanks, drew [1] https://lore.kernel.org/linux-riscv/20230414155843.12963-1-andy.chiu@sifive.com/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR 2023-04-17 20:04 ` Drew Fustini @ 2023-04-21 9:34 ` Conor Dooley 0 siblings, 0 replies; 6+ messages in thread From: Conor Dooley @ 2023-04-21 9:34 UTC (permalink / raw) To: Drew Fustini Cc: Conor Dooley, linux-riscv, linux-kernel, Palmer Dabbelt, Paul Walmsley, Atish Patra, Björn Töpel, James Morse, Kornel Dulęba, Adrien Ricciardi [-- Attachment #1.1: Type: text/plain, Size: 1860 bytes --] On Mon, Apr 17, 2023 at 01:04:11PM -0700, Drew Fustini wrote: > On Mon, Apr 17, 2023 at 08:10:20PM +0100, Conor Dooley wrote: > > On Sun, Apr 09, 2023 at 09:36:46PM -0700, Drew Fustini wrote: > > > #ifdef CONFIG_FPU > > > extern void __fstate_save(struct task_struct *save_to); > > > @@ -79,6 +80,7 @@ do { \ > > > if (has_fpu()) \ > > > __switch_to_aux(__prev, __next); \ > > > ((last) = __switch_to(__prev, __next)); \ > > > + qos_sched_in(__next); \ > > > > Both FPU and vector do: > > | if (has_fpu()) \ > > | __switch_to_fpu(__prev, __next); \ > > | if (has_vector()) \ > > | __switch_to_vector(__prev, __next); \ > > > > Is it just my OCD that wants ssqosid to be the same? > > Thanks, I had not noticed those changes from the vector patch series [1] > until you pointed it out. The handling of sqoscfg could be converted to > that pattern too. > > > It'd also do away with that seems a bit weird to me: having > > qos_sched_in() and __qos_sched_in(). > > Even if you don't make them similar, what's the rationale behind not > > inverting the extension check & returning early from a single function. > > The goal was so the inline qos_sched_in() would turn into a nop when > Ssqosid extensions not present. If Ssqosid was present, then the "real" > function __qos_sched_in() would be called. > > However, having looked at the handling of fpu and vector in the vector > series, I think will redo the sqoscfg handling to follow that pattern. Aye. With the way they do it, if !CONFIG_..._SSQOSID, DCE should remove the function call entirely & the has_extension_[un]likely() stuff will give you your nop for kernels with CONFIG_..._SSQOSID but no hardware support. I think it looks "nicer"/more obvious if you copy what the fpu/vector stuff does /shrug Cheers, Conor. [-- Attachment #1.2: signature.asc --] [-- Type: application/pgp-signature, Size: 228 bytes --] [-- Attachment #2: Type: text/plain, Size: 161 bytes --] _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-04-21 11:41 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-04-10 4:36 [RFC PATCH 0/2] RISC-V: Detect Ssqosid extension and handle sqoscfg CSR Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 1/2] RISC-V: Detect the Ssqosid extension Drew Fustini 2023-04-10 4:36 ` [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR Drew Fustini 2023-04-17 19:10 ` Conor Dooley 2023-04-17 20:04 ` Drew Fustini 2023-04-21 9:34 ` Conor Dooley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox