From: Deepak Gupta <debug@rivosinc.com> To: Samuel Holland <samuel.holland@sifive.com> Cc: Conor Dooley <conor@kernel.org>, Palmer Dabbelt <palmer@dabbelt.com>, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, tech-j-ext@lists.risc-v.org, kasan-dev@googlegroups.com, Evgenii Stepanov <eugenis@google.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Rob Herring <robh+dt@kernel.org>, Andrew Jones <ajones@ventanamicro.com>, Guo Ren <guoren@kernel.org>, Heiko Stuebner <heiko@sntech.de>, Paul Walmsley <paul.walmsley@sifive.com> Subject: Re: [RISC-V] [tech-j-ext] [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Date: Fri, 22 Mar 2024 10:13:48 -0700 [thread overview] Message-ID: <CAKC1njRBbzM+gWowg1LOjq5GzVn4q+vJP9JUswVYfWmEw+yHSg@mail.gmail.com> (raw) In-Reply-To: <d9452ab4-a783-4bcf-ac25-40baa4f31fac@sifive.com> On Thu, Mar 21, 2024 at 5:13 PM Samuel Holland <samuel.holland@sifive.com> wrote: > > On 2024-03-19 11:39 PM, Deepak Gupta wrote: > >>>> --- a/arch/riscv/include/asm/switch_to.h > >>>> +++ b/arch/riscv/include/asm/switch_to.h > >>>> @@ -69,6 +69,17 @@ static __always_inline bool has_fpu(void) { return false; } > >>>> #define __switch_to_fpu(__prev, __next) do { } while (0) > >>>> #endif > >>>> > >>>> +static inline void sync_envcfg(struct task_struct *task) > >>>> +{ > >>>> + csr_write(CSR_ENVCFG, this_cpu_read(riscv_cpu_envcfg) | task->thread.envcfg); > >>>> +} > >>>> + > >>>> +static inline void __switch_to_envcfg(struct task_struct *next) > >>>> +{ > >>>> + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_XLINUXENVCFG)) > >>> > >>> I've seen `riscv_cpu_has_extension_unlikely` generating branchy code > >>> even if ALTERNATIVES was turned on. > >>> Can you check disasm on your end as well. IMHO, `entry.S` is a better > >>> place to pick up *envcfg. > >> > >> The branchiness is sort of expected, since that function is implemented by > >> switching on/off a branch instruction, so the alternate code is necessarily a > >> separate basic block. It's a tradeoff so we don't have to write assembly code > >> for every bit of code that depends on an extension. However, the cost should be > >> somewhat lowered since the branch is unconditional and so entirely predictable. > >> > >> If the branch turns out to be problematic for performance, then we could use > >> ALTERNATIVE directly in sync_envcfg() to NOP out the CSR write. > > > > Yeah I lean towards using alternatives directly. > > One thing to note here: we can't use alternatives directly if the behavior needs > to be different on different harts (i.e. a subset of harts implement the envcfg > CSR). I think we need some policy about which ISA extensions are allowed to be > asymmetric across harts, or else we add too much complexity. As I've responded on the same thread . We are adding too much complexity by assuming that heterogeneous ISA exists (which it doesn't today). And even if it exists, it wouldn't work. Nobody wants to spend a lot of time figuring out which harts have which ISA and which packages are compiled with which ISA. Most of the end users do `sudo apt get install blah blah` And then expect it to just work. It doesn't work for other architectures and even when someone tried, they had to disable certain ISA features to make sure that all cores have the same ISA feature (search AVX12 Intel Alder Lake Disable). > > Regards, > Samuel >
WARNING: multiple messages have this Message-ID (diff)
From: Deepak Gupta <debug@rivosinc.com> To: Samuel Holland <samuel.holland@sifive.com> Cc: Conor Dooley <conor@kernel.org>, Palmer Dabbelt <palmer@dabbelt.com>, linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, Catalin Marinas <catalin.marinas@arm.com>, linux-kernel@vger.kernel.org, tech-j-ext@lists.risc-v.org, kasan-dev@googlegroups.com, Evgenii Stepanov <eugenis@google.com>, Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>, Rob Herring <robh+dt@kernel.org>, Andrew Jones <ajones@ventanamicro.com>, Guo Ren <guoren@kernel.org>, Heiko Stuebner <heiko@sntech.de>, Paul Walmsley <paul.walmsley@sifive.com> Subject: Re: [RISC-V] [tech-j-ext] [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Date: Fri, 22 Mar 2024 10:13:48 -0700 [thread overview] Message-ID: <CAKC1njRBbzM+gWowg1LOjq5GzVn4q+vJP9JUswVYfWmEw+yHSg@mail.gmail.com> (raw) In-Reply-To: <d9452ab4-a783-4bcf-ac25-40baa4f31fac@sifive.com> On Thu, Mar 21, 2024 at 5:13 PM Samuel Holland <samuel.holland@sifive.com> wrote: > > On 2024-03-19 11:39 PM, Deepak Gupta wrote: > >>>> --- a/arch/riscv/include/asm/switch_to.h > >>>> +++ b/arch/riscv/include/asm/switch_to.h > >>>> @@ -69,6 +69,17 @@ static __always_inline bool has_fpu(void) { return false; } > >>>> #define __switch_to_fpu(__prev, __next) do { } while (0) > >>>> #endif > >>>> > >>>> +static inline void sync_envcfg(struct task_struct *task) > >>>> +{ > >>>> + csr_write(CSR_ENVCFG, this_cpu_read(riscv_cpu_envcfg) | task->thread.envcfg); > >>>> +} > >>>> + > >>>> +static inline void __switch_to_envcfg(struct task_struct *next) > >>>> +{ > >>>> + if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_XLINUXENVCFG)) > >>> > >>> I've seen `riscv_cpu_has_extension_unlikely` generating branchy code > >>> even if ALTERNATIVES was turned on. > >>> Can you check disasm on your end as well. IMHO, `entry.S` is a better > >>> place to pick up *envcfg. > >> > >> The branchiness is sort of expected, since that function is implemented by > >> switching on/off a branch instruction, so the alternate code is necessarily a > >> separate basic block. It's a tradeoff so we don't have to write assembly code > >> for every bit of code that depends on an extension. However, the cost should be > >> somewhat lowered since the branch is unconditional and so entirely predictable. > >> > >> If the branch turns out to be problematic for performance, then we could use > >> ALTERNATIVE directly in sync_envcfg() to NOP out the CSR write. > > > > Yeah I lean towards using alternatives directly. > > One thing to note here: we can't use alternatives directly if the behavior needs > to be different on different harts (i.e. a subset of harts implement the envcfg > CSR). I think we need some policy about which ISA extensions are allowed to be > asymmetric across harts, or else we add too much complexity. As I've responded on the same thread . We are adding too much complexity by assuming that heterogeneous ISA exists (which it doesn't today). And even if it exists, it wouldn't work. Nobody wants to spend a lot of time figuring out which harts have which ISA and which packages are compiled with which ISA. Most of the end users do `sudo apt get install blah blah` And then expect it to just work. It doesn't work for other architectures and even when someone tried, they had to disable certain ISA features to make sure that all cores have the same ISA feature (search AVX12 Intel Alder Lake Disable). > > Regards, > Samuel > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2024-03-22 17:13 UTC|newest] Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-03-19 21:58 [RFC PATCH 0/9] riscv: Userspace pointer masking and tagged address ABI Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 1/9] dt-bindings: riscv: Add pointer masking ISA extensions Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 2/9] riscv: Add ISA extension parsing for pointer masking Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 3/9] riscv: Add CSR definitions " Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 4/9] riscv: Define is_compat_thread() Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 5/9] riscv: Split per-CPU and per-thread envcfg bits Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 23:55 ` [RISC-V] [tech-j-ext] " Deepak Gupta 2024-03-19 23:55 ` Deepak Gupta 2024-03-20 2:20 ` Samuel Holland 2024-03-20 2:20 ` Samuel Holland 2024-03-20 4:39 ` Deepak Gupta 2024-03-20 4:39 ` Deepak Gupta 2024-03-22 0:13 ` Samuel Holland 2024-03-22 0:13 ` Samuel Holland 2024-03-22 17:13 ` Deepak Gupta [this message] 2024-03-22 17:13 ` Deepak Gupta 2024-03-23 9:35 ` Andrew Jones 2024-03-23 9:35 ` Andrew Jones 2024-03-23 20:37 ` Deepak Gupta 2024-03-23 20:37 ` Deepak Gupta 2024-03-22 8:09 ` Andrew Jones 2024-03-22 8:09 ` Andrew Jones 2024-03-22 16:52 ` Deepak Gupta 2024-03-22 16:52 ` Deepak Gupta 2024-03-20 8:06 ` Conor Dooley 2024-03-20 8:06 ` Conor Dooley [not found] ` <17BE5F38AFE245E5.29196@lists.riscv.org> 2024-03-20 23:27 ` Deepak Gupta 2024-03-20 23:27 ` Deepak Gupta 2024-03-22 3:43 ` Samuel Holland 2024-03-22 3:43 ` Samuel Holland 2024-03-22 7:58 ` Andrew Jones 2024-03-22 7:58 ` Andrew Jones 2024-03-28 1:58 ` Deepak Gupta 2024-03-28 1:58 ` Deepak Gupta [not found] ` <17C0CB122DBB0EAE.6770@lists.riscv.org> 2024-03-28 19:34 ` Deepak Gupta 2024-03-28 19:34 ` Deepak Gupta 2024-03-19 21:58 ` [RFC PATCH 6/9] riscv: Add support for userspace pointer masking Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 7/9] riscv: Add support for the tagged address ABI Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 8/9] riscv: Allow ptrace control of " Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-19 21:58 ` [RFC PATCH 9/9] selftests: riscv: Add a pointer masking test Samuel Holland 2024-03-19 21:58 ` Samuel Holland 2024-03-20 17:21 ` Conor Dooley 2024-03-20 17:21 ` Conor Dooley 2024-03-20 18:04 ` Samuel Holland 2024-03-20 18:04 ` Samuel Holland
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAKC1njRBbzM+gWowg1LOjq5GzVn4q+vJP9JUswVYfWmEw+yHSg@mail.gmail.com \ --to=debug@rivosinc.com \ --cc=ajones@ventanamicro.com \ --cc=catalin.marinas@arm.com \ --cc=conor@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=eugenis@google.com \ --cc=guoren@kernel.org \ --cc=heiko@sntech.de \ --cc=kasan-dev@googlegroups.com \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=paul.walmsley@sifive.com \ --cc=robh+dt@kernel.org \ --cc=samuel.holland@sifive.com \ --cc=tech-j-ext@lists.risc-v.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.