| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20230417-culinary-capillary-4f1efd496691@spud>
Date: Mon, 17 Apr 2023 20:10:20 +0100
From: Conor Dooley <conor@...nel.org>
To: Drew Fustini <dfustini@...libre.com>
Cc: linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org,
Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Conor Dooley <conor.dooley@...rochip.com>,
Atish Patra <atishp@...osinc.com>,
Björn Töpel <bjorn@...osinc.com>,
James Morse <james.morse@....com>,
Kornel Dulęba <mindal@...ihalf.com>,
Adrien Ricciardi <aricciardi@...libre.com>
Subject: Re: [RFC PATCH 2/2] RISC-V: Add support for sqoscfg CSR
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@...ihalf.com>
> Signed-off-by: Kornel Dulęba <mindal@...ihalf.com>
> Signed-off-by: Drew Fustini <dfustini@...libre.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
>
Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)
Powered by blists - more mailing lists