From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oo1-f43.google.com (mail-oo1-f43.google.com [209.85.161.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 060E41CD33 for ; Tue, 16 Jan 2024 18:46:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.161.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705430766; cv=none; b=faWa9EpdjWjizqo+K/Lrmd5Up7mA/dLCjco5jUTqmkBqu2vbStyZxqMARF9gGIiFep0ZTXSip2bFRy7DbJB8R1GQ7YwRhnO1gZms8XTrRlZzr5+WWu6auK8b1cXHgeZ6LHoZIje1rK2SO+wQwtV7emk9SpyZmgF6WI9AM/aTA7s= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1705430766; c=relaxed/simple; bh=EBCFvdpGUjiCDYw0UGK+EtkN5tfcMm8xs/4bkbDLJqU=; h=Received:DKIM-Signature:X-Google-DKIM-Signature: X-Gm-Message-State:X-Google-Smtp-Source:X-Received:Received:Date: From:To:Cc:Subject:Message-ID:References:MIME-Version:Content-Type: Content-Disposition:In-Reply-To; b=tHj/NP2m3VQyy1sx0agVYt7/9fofUOAK7VtmBlEiantf+4K6SgIJ+4TchMc+tvLlfdqBO1s/G6zosbgp5hzLxeo/xlI+ItwJ4d1gpx9uTB9AWTGZgvU3dx2aQ3ICkBXFXUUBPbC6fDRnNA9x4m9SUA2Nk6OMZ8Uu5YNtceumYQE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b=SZZ5HXZw; arc=none smtp.client-ip=209.85.161.43 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b="SZZ5HXZw" Received: by mail-oo1-f43.google.com with SMTP id 006d021491bc7-598b8dd877dso2421351eaf.3 for ; Tue, 16 Jan 2024 10:46:02 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1705430762; x=1706035562; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=X7+iPjvVCly4VO5Mo6fUS2dWNQKiaqTW6P9ssk9rj8w=; b=SZZ5HXZwEzFhWPFnVjBK5Cv2MpmPQG6cVbQHuoW67VkFcPAzLseEkYeuoGsbJJe2h8 VxW9uEgZ2KH3bum4bvbHM/QqU26hts5OYqhWtSIL+O4PyHPbxYgZBbbyPGyPQ9Bsh1Rb 1/qh+3fmMX43FrJa+lMx0Hv8RTPBL/dYcfWN3NjeujMdva+/9rT+yD5vDAPFdgwgJVU6 SrnTxAcWyK3Hdj1H66XjGugDChJuB9s03ydDbItyqZFf09CNwMuPDQNQf1W1Tjhd6onT YHjNLky1qUtuqyCDcNydWe97O2ko0DS/w/aJCW/lwbgVG/EwZZnZjUyasA2estolRWW3 SQuA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1705430762; x=1706035562; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=X7+iPjvVCly4VO5Mo6fUS2dWNQKiaqTW6P9ssk9rj8w=; b=e0U4z9hwaJPl+1Q3BiBUCB0+X9sqyV55eVvLMAhUKtKusTvZsTcBMwYX9X1WJ09dfX BX5Yr0OuTqXIVLkDuUdqNubXY09muHOBaYtJ6ohzgJrqr0IdTbJb3THLQAlOzVPth4oC f2OEjMmN/EnfYXg9rMebEXdWDEH+6LA7mwBJkrfTGDaMuo8XEIvH4tdvi0elt+ewyp+u rJe+TRwQleoBxwiNHEmry9hbDQs11o6nGRFIPKm95fkzSqhY3sK8kkpfdtbuWBz7v07W qiCny6DqYJHcCTJF+77tiZLKf1FFe+6/TkAyjD7lE+z+uDyrn0kkzKGoVDETA8C5wFli Dt/A== X-Gm-Message-State: AOJu0YwQuB0U2sJyR122kyVuoy/gj2ikG167NWrqCGzpCmdU8VkHLqym uwIC6UluM9T8uy5awWjxcz+3rZFOdqE1UQ== X-Google-Smtp-Source: AGHT+IG8iwkfIcUByGe/mng68dHcuTCvhprse2hGqd6d0WOzy/Oe4zQZuEtALdDWOdpDACmx9Cd2Ag== X-Received: by 2002:a05:6358:c305:b0:175:5d72:5f7c with SMTP id fk5-20020a056358c30500b001755d725f7cmr4939338rwb.50.1705430761918; Tue, 16 Jan 2024 10:46:01 -0800 (PST) Received: from ghost ([2601:647:5700:6860:60b9:65a4:6399:f8f3]) by smtp.gmail.com with ESMTPSA id j190-20020a6380c7000000b005b529d633b7sm10243898pgd.14.2024.01.16.10.46.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 16 Jan 2024 10:46:01 -0800 (PST) Date: Tue, 16 Jan 2024 10:45:59 -0800 From: Charlie Jenkins To: Alexandre Ghiti Cc: Paul Walmsley , Palmer Dabbelt , Albert Ou , Jonathan Corbet , Conor Dooley , =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= , Atish Patra , Randy Dunlap , linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, Atish Patra Subject: Re: [PATCH v7 1/2] riscv: Include riscv_set_icache_flush_ctx prctl Message-ID: References: <20240112-fencei-v7-0-78f0614e1db0@rivosinc.com> <20240112-fencei-v7-1-78f0614e1db0@rivosinc.com> Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Sun, Jan 14, 2024 at 05:05:20PM +0100, Alexandre Ghiti wrote: > Hi Charlie, > > On 13/01/2024 00:56, Charlie Jenkins wrote: > > Support new prctl with key PR_RISCV_SET_ICACHE_FLUSH_CTX to enable > > optimization of cross modifying code. This prctl enables userspace code > > to use icache flushing instructions such as fence.i with the guarantee > > that the icache will continue to be clean after thread migration. > > > > Signed-off-by: Charlie Jenkins > > Reviewed-by: Atish Patra > > --- > > arch/riscv/include/asm/mmu.h | 2 ++ > > arch/riscv/include/asm/processor.h | 6 ++++ > > arch/riscv/mm/cacheflush.c | 67 ++++++++++++++++++++++++++++++++++++++ > > arch/riscv/mm/context.c | 8 +++-- > > include/uapi/linux/prctl.h | 6 ++++ > > kernel/sys.c | 6 ++++ > > 6 files changed, 92 insertions(+), 3 deletions(-) > > > > diff --git a/arch/riscv/include/asm/mmu.h b/arch/riscv/include/asm/mmu.h > > index 355504b37f8e..60be458e94da 100644 > > --- a/arch/riscv/include/asm/mmu.h > > +++ b/arch/riscv/include/asm/mmu.h > > @@ -19,6 +19,8 @@ typedef struct { > > #ifdef CONFIG_SMP > > /* A local icache flush is needed before user execution can resume. */ > > cpumask_t icache_stale_mask; > > + /* Force local icache flush on all migrations. */ > > + bool force_icache_flush; > > #endif > > #ifdef CONFIG_BINFMT_ELF_FDPIC > > unsigned long exec_fdpic_loadmap; > > diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h > > index f19f861cda54..7eda6c75e0f2 100644 > > --- a/arch/riscv/include/asm/processor.h > > +++ b/arch/riscv/include/asm/processor.h > > @@ -84,6 +84,9 @@ struct thread_struct { > > unsigned long vstate_ctrl; > > struct __riscv_v_ext_state vstate; > > unsigned long align_ctl; > > +#ifdef CONFIG_SMP > > + bool force_icache_flush; > > +#endif > > }; > > /* Whitelist the fstate from the task_struct for hardened usercopy */ > > @@ -145,6 +148,9 @@ extern int set_unalign_ctl(struct task_struct *tsk, unsigned int val); > > #define GET_UNALIGN_CTL(tsk, addr) get_unalign_ctl((tsk), (addr)) > > #define SET_UNALIGN_CTL(tsk, val) set_unalign_ctl((tsk), (val)) > > +#define RISCV_SET_ICACHE_FLUSH_CTX(arg1, arg2) riscv_set_icache_flush_ctx(arg1, arg2) > > +extern int riscv_set_icache_flush_ctx(unsigned long ctx, unsigned long per_thread); > > + > > #endif /* __ASSEMBLY__ */ > > #endif /* _ASM_RISCV_PROCESSOR_H */ > > diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c > > index 55a34f2020a8..ff545f19f07a 100644 > > --- a/arch/riscv/mm/cacheflush.c > > +++ b/arch/riscv/mm/cacheflush.c > > @@ -5,6 +5,7 @@ > > #include > > #include > > +#include > > #include > > #include > > @@ -152,3 +153,69 @@ void __init riscv_init_cbo_blocksizes(void) > > if (cboz_block_size) > > riscv_cboz_block_size = cboz_block_size; > > } > > + > > +/** > > + * riscv_set_icache_flush_ctx() - Enable/disable icache flushing instructions in > > + * userspace. > > + * @ctx: Set the type of icache flushing instructions permitted/prohibited in > > + * userspace. Supported values described below. > > + * > > + * Supported values for ctx: > > + * > > + * * %PR_RISCV_CTX_SW_FENCEI_ON: Allow fence.i in userspace. > > + * > > + * * %PR_RISCV_CTX_SW_FENCEI_OFF: Disallow fence.i in userspace. When ``scope == > > + * PR_RISCV_SCOPE_PER_PROCESS``, this will effect all threads in a process. > > + * Therefore, caution must be taken -- only use this flag when you can > > + * guarantee that no thread in the process will emit fence.i from this point > > + * onward. > > + * > > + * @scope: Set scope of where icache flushing instructions are allowed to be > > + * emitted. Supported values described below. > > + * > > + * Supported values for scope: > > + * > > + * * PR_RISCV_SCOPE_PER_PROCESS: Ensure the icache of any thread in this process > > + * is coherent with instruction storage upon > > + * migration. > > + * > > + * * PR_RISCV_SCOPE_PER_THREAD: Ensure the icache of the current thread is > > + * coherent with instruction storage upon > > + * migration. > > + * > > + * When ``scope == PR_RISCV_SCOPE_PER_PROCESS``, all threads in the process are > > + * permitted to emit icache flushing instructions. Whenever any thread in the > > + * process is migrated, the corresponding hart's icache will be guaranteed to be > > + * consistent with instruction storage. Note this does not enforce any > > + * guarantees outside of migration. If a thread modifies an instruction that > > + * another thread may attempt to execute, the other thread must still emit an > > + * icache flushing instruction before attempting to execute the potentially > > + * modified instruction. This must be performed by the userspace program. > > + * > > + * In per-thread context (eg. ``scope == PR_RISCV_SCOPE_PER_THREAD``), only the > > + * thread calling this function is permitted to emit icache flushing > > + * instructions. When the thread is migrated, the corresponding hart's icache > > + * will be guaranteed to be consistent with instruction storage. > > + * > > + * On kernels configured without SMP, this function is a nop as migrations > > + * across harts will not occur. > > + */ > > +int riscv_set_icache_flush_ctx(unsigned long ctx, unsigned long scope) > > +{ > > +#ifdef CONFIG_SMP > > + switch (ctx) { > > + case PR_RISCV_CTX_SW_FENCEI_ON: > > + switch (scope) { > > + case PR_RISCV_SCOPE_PER_PROCESS: > > + current->mm->context.force_icache_flush = true; > > + break; > > + case PR_RISCV_SCOPE_PER_THREAD: > > + current->thread.force_icache_flush = true; > > + break; > > + default: > > + return -EINVAL; > > + } > > + } > > +#endif > > + return 0; > > +} > > diff --git a/arch/riscv/mm/context.c b/arch/riscv/mm/context.c > > index 217fd4de6134..0146c61be0ab 100644 > > --- a/arch/riscv/mm/context.c > > +++ b/arch/riscv/mm/context.c > > @@ -297,12 +297,14 @@ static inline void set_mm(struct mm_struct *prev, > > * > > * The "cpu" argument must be the current local CPU number. > > */ > > -static inline void flush_icache_deferred(struct mm_struct *mm, unsigned int cpu) > > +static inline void flush_icache_deferred(struct mm_struct *mm, unsigned int cpu, > > + struct task_struct *task) > > { > > #ifdef CONFIG_SMP > > cpumask_t *mask = &mm->context.icache_stale_mask; > > - if (cpumask_test_cpu(cpu, mask)) { > > + if (cpumask_test_cpu(cpu, mask) || mm->context.force_icache_flush || > > + (task && task->thread.force_icache_flush)) { > > > IIUC, if a process/thread enables force_icache_flush, at every context > switch, the process/thread will flush the local icache but, as you said, > that's actually only needed in case of a migration. So we should check that > there has been a migration before flushing the icache. Maybe by storing the > last cpu the process/thread executed on and checking when it is scheduled > back if the current cpu if different from the last cpu, WDYT? Yes that does make sense. I will send an update. > > Thanks, > > Alex > > > > cpumask_clear_cpu(cpu, mask); > > /* > > * Ensure the remote hart's writes are visible to this hart. > > @@ -332,5 +334,5 @@ void switch_mm(struct mm_struct *prev, struct mm_struct *next, > > set_mm(prev, next, cpu); > > - flush_icache_deferred(next, cpu); > > + flush_icache_deferred(next, cpu, task); > > } > > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h > > index 370ed14b1ae0..524d546d697b 100644 > > --- a/include/uapi/linux/prctl.h > > +++ b/include/uapi/linux/prctl.h > > @@ -306,4 +306,10 @@ struct prctl_mm_map { > > # define PR_RISCV_V_VSTATE_CTRL_NEXT_MASK 0xc > > # define PR_RISCV_V_VSTATE_CTRL_MASK 0x1f > > +#define PR_RISCV_SET_ICACHE_FLUSH_CTX 71 > > +# define PR_RISCV_CTX_SW_FENCEI_ON 0 > > +# define PR_RISCV_CTX_SW_FENCEI_OFF 1 > > +# define PR_RISCV_SCOPE_PER_PROCESS 0 > > +# define PR_RISCV_SCOPE_PER_THREAD 1 > > + > > #endif /* _LINUX_PRCTL_H */ > > diff --git a/kernel/sys.c b/kernel/sys.c > > index 420d9cb9cc8e..e806a8a67c36 100644 > > --- a/kernel/sys.c > > +++ b/kernel/sys.c > > @@ -146,6 +146,9 @@ > > #ifndef RISCV_V_GET_CONTROL > > # define RISCV_V_GET_CONTROL() (-EINVAL) > > #endif > > +#ifndef RISCV_SET_ICACHE_FLUSH_CTX > > +# define RISCV_SET_ICACHE_FLUSH_CTX(a, b) (-EINVAL) > > +#endif > > /* > > * this is where the system-wide overflow UID and GID are defined, for > > @@ -2739,6 +2742,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > > case PR_RISCV_V_GET_CONTROL: > > error = RISCV_V_GET_CONTROL(); > > break; > > + case PR_RISCV_SET_ICACHE_FLUSH_CTX: > > + error = RISCV_SET_ICACHE_FLUSH_CTX(arg2, arg3); > > + break; > > default: > > error = -EINVAL; > > break; > >