From: Evan Green <evan@rivosinc.com> To: Conor Dooley <conor@kernel.org> Cc: Palmer Dabbelt <palmer@rivosinc.com>, vineetg@rivosinc.com, heiko@sntech.de, slewis@rivosinc.com, Albert Ou <aou@eecs.berkeley.edu>, Andrew Bresticker <abrestic@rivosinc.com>, Andrew Jones <ajones@ventanamicro.com>, Anup Patel <apatel@ventanamicro.com>, Arnd Bergmann <arnd@arndb.de>, Atish Patra <atishp@rivosinc.com>, Celeste Liu <coelacanthus@outlook.com>, Guo Ren <guoren@kernel.org>, Heinrich Schuchardt <heinrich.schuchardt@canonical.com>, Jisheng Zhang <jszhang@kernel.org>, Jonathan Corbet <corbet@lwn.net>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Sunil V L <sunilvl@ventanamicro.com>, Tsukasa OI <research_trasio@irq.a4lg.com>, Xianting Tian <xianting.tian@linux.alibaba.com>, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: Re: [PATCH v2 5/6] RISC-V: hwprobe: Support probing of misaligned access performance Date: Fri, 17 Feb 2023 16:15:34 -0800 [thread overview] Message-ID: <CALs-HsvZaKC5R-rAvkjBDNQGTUJ_LWn-O=KzpFwGtpac32_Xxw@mail.gmail.com> (raw) In-Reply-To: <Y+1VOXyKDDHEuejJ@spud> On Wed, Feb 15, 2023 at 1:57 PM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Feb 06, 2023 at 12:14:54PM -0800, Evan Green wrote: > > This allows userspace to select various routines to use based on the > > performance of misaligned access on the target hardware. > > > > Co-developed-by: Palmer Dabbelt <palmer@rivosinc.com> > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > Signed-off-by: Evan Green <evan@rivosinc.com> > > > > --- > > > > Changes in v2: > > - Fixed logic error in if(of_property_read_string...) that caused crash > > - Include cpufeature.h in cpufeature.h to avoid undeclared variable > > warning. > > - Added a _MASK define > > - Fix random checkpatch complaints > > > > Documentation/riscv/hwprobe.rst | 13 +++++++++++ > > arch/riscv/include/asm/cpufeature.h | 2 ++ > > arch/riscv/include/asm/hwprobe.h | 2 +- > > arch/riscv/include/asm/smp.h | 9 ++++++++ > > arch/riscv/include/uapi/asm/hwprobe.h | 6 ++++++ > > arch/riscv/kernel/cpufeature.c | 31 +++++++++++++++++++++++++-- > > arch/riscv/kernel/sys_riscv.c | 23 ++++++++++++++++++++ > > 7 files changed, 83 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst > > index ce186967861f..0dc75e83e127 100644 > > --- a/Documentation/riscv/hwprobe.rst > > +++ b/Documentation/riscv/hwprobe.rst > > @@ -51,3 +51,16 @@ The following keys are defined: > > not minNum/maxNum") of the RISC-V ISA manual. > > * :RISCV_HWPROBE_IMA_C:: The C extension is supported, as defined by > > version 2.2 of the RISC-V ISA manual. > > +* :RISCV_HWPROBE_KEY_PERF_0:: A bitmask that contains performance information > > This doesn't match what's defined? > > > + about the selected set of processors. > > + * :RISCV_HWPROBE_MISALIGNED_UNKNOWN:: The performance of misaligned > > + accesses is unknown. > > + * :RISCV_HWPROBE_MISALIGNED_EMULATED:: Misaligned accesses are emulated via > > + software, either in or below the kernel. These accesses are always > > + extremely slow. > > + * :RISCV_HWPROBE_MISALIGNED_SLOW:: Misaligned accesses are supported in > > + hardware, but are slower than the cooresponding aligned accesses > > + sequences. > > + * :RISCV_HWPROBE_MISALIGNED_FAST:: Misaligned accesses are supported in > > + hardware and are faster than the cooresponding aligned accesses > > + sequences. > > > diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h > > index 3831b638ecab..6c1759091e44 100644 > > --- a/arch/riscv/include/asm/smp.h > > +++ b/arch/riscv/include/asm/smp.h > > @@ -26,6 +26,15 @@ struct riscv_ipi_ops { > > */ > > extern unsigned long __cpuid_to_hartid_map[NR_CPUS]; > > #define cpuid_to_hartid_map(cpu) __cpuid_to_hartid_map[cpu] > > +static inline long hartid_to_cpuid_map(unsigned long hartid) > > +{ > > + long i; > > + > > + for (i = 0; i < NR_CPUS; ++i) > > I'm never (or not yet?) sure about these things. > Should this be for_each_possible_cpu()? Me neither. I believe it's the same, as for_each_possible_cpu() iterates over a CPU mask of all 1s, and the size of struct cpumask is set by NR_CPUS. Some architectures appear to have an init_cpu_possible() function to further restrict the set, though riscv does not. It's probably better to use for_each_possible_cpu() though in case a call to init_cpu_possible() ever does get added. > > > + if (cpuid_to_hartid_map(i) == hartid) > > + return i; > > + return -1; > > +} > > > > /* print IPI stats */ > > void show_ipi_stats(struct seq_file *p, int prec); > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h > > index ce39d6e74103..5d55e2da2b1f 100644 > > --- a/arch/riscv/include/uapi/asm/hwprobe.h > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h > > @@ -25,5 +25,11 @@ struct riscv_hwprobe { > > #define RISCV_HWPROBE_KEY_IMA_EXT_0 4 > > #define RISCV_HWPROBE_IMA_FD (1 << 0) > > #define RISCV_HWPROBE_IMA_C (1 << 1) > > +#define RISCV_HWPROBE_KEY_CPUPERF_0 5 > > +#define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_EMULATED (1 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_SLOW (2 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_FAST (3 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_MASK (3 << 0) > > Why is it UNKNOWN rather than UNSUPPORTED? > I thought I saw Palmer saying that there is no requirement to support > misaligned accesses any more. > Plenty of old DTs are going to lack this property so would be UNKNOWN, > and I *assume* that the user of the syscall is gonna conflate the two, > but the rationale interests me. Palmer had mentioned on the DT bindings patch that historically it was required but emulated. So because old binaries assumed it was there, the default values for DTs without this needs to imply "supported, but no idea how fast it is". But you bring up an interesting point: should the bindings and these defines have a value that indicates no support at all for unaligned accesses? We could always add the value to the bindings later, but maybe we should leave space in this field now. -Evan
WARNING: multiple messages have this Message-ID (diff)
From: Evan Green <evan@rivosinc.com> To: Conor Dooley <conor@kernel.org> Cc: heiko@sntech.de, Jisheng Zhang <jszhang@kernel.org>, linux-doc@vger.kernel.org, Andrew Bresticker <abrestic@rivosinc.com>, Atish Patra <atishp@rivosinc.com>, Palmer Dabbelt <palmer@rivosinc.com>, Celeste Liu <coelacanthus@outlook.com>, slewis@rivosinc.com, linux-riscv@lists.infradead.org, Jonathan Corbet <corbet@lwn.net>, Xianting Tian <xianting.tian@linux.alibaba.com>, Tsukasa OI <research_trasio@irq.a4lg.com>, Andrew Jones <ajones@ventanamicro.com>, Albert Ou <aou@eecs.berkeley.edu>, Arnd Bergmann <arnd@arndb.de>, vineetg@rivosinc.com, Paul Walmsley <paul.walmsley@sifive.com>, Anup Patel <apatel@ventanamicro.com>, linux-kernel@vger.kernel.org, Palmer Dabbelt <palmer@dabbelt.com>, Heinrich Schuchardt <heinrich.schuchardt@canonical.com>, Guo Ren <guoren@kernel.org> Subject: Re: [PATCH v2 5/6] RISC-V: hwprobe: Support probing of misaligned access performance Date: Fri, 17 Feb 2023 16:15:34 -0800 [thread overview] Message-ID: <CALs-HsvZaKC5R-rAvkjBDNQGTUJ_LWn-O=KzpFwGtpac32_Xxw@mail.gmail.com> (raw) In-Reply-To: <Y+1VOXyKDDHEuejJ@spud> On Wed, Feb 15, 2023 at 1:57 PM Conor Dooley <conor@kernel.org> wrote: > > On Mon, Feb 06, 2023 at 12:14:54PM -0800, Evan Green wrote: > > This allows userspace to select various routines to use based on the > > performance of misaligned access on the target hardware. > > > > Co-developed-by: Palmer Dabbelt <palmer@rivosinc.com> > > Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com> > > Signed-off-by: Evan Green <evan@rivosinc.com> > > > > --- > > > > Changes in v2: > > - Fixed logic error in if(of_property_read_string...) that caused crash > > - Include cpufeature.h in cpufeature.h to avoid undeclared variable > > warning. > > - Added a _MASK define > > - Fix random checkpatch complaints > > > > Documentation/riscv/hwprobe.rst | 13 +++++++++++ > > arch/riscv/include/asm/cpufeature.h | 2 ++ > > arch/riscv/include/asm/hwprobe.h | 2 +- > > arch/riscv/include/asm/smp.h | 9 ++++++++ > > arch/riscv/include/uapi/asm/hwprobe.h | 6 ++++++ > > arch/riscv/kernel/cpufeature.c | 31 +++++++++++++++++++++++++-- > > arch/riscv/kernel/sys_riscv.c | 23 ++++++++++++++++++++ > > 7 files changed, 83 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/riscv/hwprobe.rst b/Documentation/riscv/hwprobe.rst > > index ce186967861f..0dc75e83e127 100644 > > --- a/Documentation/riscv/hwprobe.rst > > +++ b/Documentation/riscv/hwprobe.rst > > @@ -51,3 +51,16 @@ The following keys are defined: > > not minNum/maxNum") of the RISC-V ISA manual. > > * :RISCV_HWPROBE_IMA_C:: The C extension is supported, as defined by > > version 2.2 of the RISC-V ISA manual. > > +* :RISCV_HWPROBE_KEY_PERF_0:: A bitmask that contains performance information > > This doesn't match what's defined? > > > + about the selected set of processors. > > + * :RISCV_HWPROBE_MISALIGNED_UNKNOWN:: The performance of misaligned > > + accesses is unknown. > > + * :RISCV_HWPROBE_MISALIGNED_EMULATED:: Misaligned accesses are emulated via > > + software, either in or below the kernel. These accesses are always > > + extremely slow. > > + * :RISCV_HWPROBE_MISALIGNED_SLOW:: Misaligned accesses are supported in > > + hardware, but are slower than the cooresponding aligned accesses > > + sequences. > > + * :RISCV_HWPROBE_MISALIGNED_FAST:: Misaligned accesses are supported in > > + hardware and are faster than the cooresponding aligned accesses > > + sequences. > > > diff --git a/arch/riscv/include/asm/smp.h b/arch/riscv/include/asm/smp.h > > index 3831b638ecab..6c1759091e44 100644 > > --- a/arch/riscv/include/asm/smp.h > > +++ b/arch/riscv/include/asm/smp.h > > @@ -26,6 +26,15 @@ struct riscv_ipi_ops { > > */ > > extern unsigned long __cpuid_to_hartid_map[NR_CPUS]; > > #define cpuid_to_hartid_map(cpu) __cpuid_to_hartid_map[cpu] > > +static inline long hartid_to_cpuid_map(unsigned long hartid) > > +{ > > + long i; > > + > > + for (i = 0; i < NR_CPUS; ++i) > > I'm never (or not yet?) sure about these things. > Should this be for_each_possible_cpu()? Me neither. I believe it's the same, as for_each_possible_cpu() iterates over a CPU mask of all 1s, and the size of struct cpumask is set by NR_CPUS. Some architectures appear to have an init_cpu_possible() function to further restrict the set, though riscv does not. It's probably better to use for_each_possible_cpu() though in case a call to init_cpu_possible() ever does get added. > > > + if (cpuid_to_hartid_map(i) == hartid) > > + return i; > > + return -1; > > +} > > > > /* print IPI stats */ > > void show_ipi_stats(struct seq_file *p, int prec); > > diff --git a/arch/riscv/include/uapi/asm/hwprobe.h b/arch/riscv/include/uapi/asm/hwprobe.h > > index ce39d6e74103..5d55e2da2b1f 100644 > > --- a/arch/riscv/include/uapi/asm/hwprobe.h > > +++ b/arch/riscv/include/uapi/asm/hwprobe.h > > @@ -25,5 +25,11 @@ struct riscv_hwprobe { > > #define RISCV_HWPROBE_KEY_IMA_EXT_0 4 > > #define RISCV_HWPROBE_IMA_FD (1 << 0) > > #define RISCV_HWPROBE_IMA_C (1 << 1) > > +#define RISCV_HWPROBE_KEY_CPUPERF_0 5 > > +#define RISCV_HWPROBE_MISALIGNED_UNKNOWN (0 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_EMULATED (1 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_SLOW (2 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_FAST (3 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_MASK (3 << 0) > > Why is it UNKNOWN rather than UNSUPPORTED? > I thought I saw Palmer saying that there is no requirement to support > misaligned accesses any more. > Plenty of old DTs are going to lack this property so would be UNKNOWN, > and I *assume* that the user of the syscall is gonna conflate the two, > but the rationale interests me. Palmer had mentioned on the DT bindings patch that historically it was required but emulated. So because old binaries assumed it was there, the default values for DTs without this needs to imply "supported, but no idea how fast it is". But you bring up an interesting point: should the bindings and these defines have a value that indicates no support at all for unaligned accesses? We could always add the value to the bindings later, but maybe we should leave space in this field now. -Evan _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv
next prev parent reply other threads:[~2023-02-18 0:16 UTC|newest] Thread overview: 91+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-02-06 20:14 [PATCH v2 0/6] RISC-V Hardware Probing User Interface Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-06 20:14 ` [PATCH v2 1/6] RISC-V: Move struct riscv_cpuinfo to new header Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-14 21:38 ` Conor Dooley 2023-02-14 21:38 ` Conor Dooley 2023-02-14 21:57 ` Evan Green 2023-02-14 21:57 ` Evan Green 2023-02-06 20:14 ` [PATCH v2 2/6] RISC-V: Add a syscall for HW probing Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-07 6:13 ` Greg KH 2023-02-07 6:13 ` Greg KH 2023-02-07 6:32 ` Conor Dooley 2023-02-07 6:32 ` Conor Dooley 2023-02-09 17:09 ` Evan Green 2023-02-09 17:09 ` Evan Green 2023-02-09 17:13 ` Greg KH 2023-02-09 17:13 ` Greg KH 2023-02-09 17:22 ` Jessica Clarke 2023-02-09 17:22 ` Jessica Clarke 2023-02-10 6:48 ` Greg KH 2023-02-10 6:48 ` Greg KH 2023-02-09 18:41 ` Evan Green 2023-02-09 18:41 ` Evan Green 2023-02-10 6:50 ` Greg KH 2023-02-10 6:50 ` Greg KH 2023-02-07 23:16 ` kernel test robot 2023-02-07 23:16 ` kernel test robot 2023-02-14 23:51 ` Conor Dooley 2023-02-14 23:51 ` Conor Dooley 2023-02-15 8:04 ` Andrew Jones 2023-02-15 8:04 ` Andrew Jones 2023-02-15 20:49 ` Evan Green 2023-02-15 20:49 ` Evan Green 2023-02-15 21:10 ` Conor Dooley 2023-02-15 21:10 ` Conor Dooley 2023-02-15 9:56 ` Arnd Bergmann 2023-02-15 9:56 ` Arnd Bergmann 2023-02-15 21:14 ` Evan Green 2023-02-15 21:14 ` Evan Green 2023-02-15 22:43 ` Jessica Clarke 2023-02-15 22:43 ` Jessica Clarke 2023-02-16 13:28 ` Arnd Bergmann 2023-02-16 13:28 ` Arnd Bergmann 2023-02-16 23:18 ` Evan Green 2023-02-16 23:18 ` Evan Green 2023-02-06 20:14 ` [PATCH v2 3/6] RISC-V: hwprobe: Add support for RISCV_HWPROBE_BASE_BEHAVIOR_IMA Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-08 5:06 ` kernel test robot 2023-02-15 21:25 ` Conor Dooley 2023-02-15 21:25 ` Conor Dooley 2023-02-15 22:09 ` Conor Dooley 2023-02-15 22:09 ` Conor Dooley 2023-02-06 20:14 ` [PATCH v2 4/6] dt-bindings: Add RISC-V misaligned access performance Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-06 21:49 ` Rob Herring 2023-02-06 21:49 ` Rob Herring 2023-02-07 17:05 ` Rob Herring 2023-02-07 17:05 ` Rob Herring 2023-02-08 12:45 ` David Laight 2023-02-08 12:45 ` David Laight 2023-02-09 16:51 ` Palmer Dabbelt 2023-02-09 16:51 ` Palmer Dabbelt 2023-02-28 14:56 ` Rob Herring 2023-02-28 14:56 ` Rob Herring 2023-02-14 21:26 ` Conor Dooley 2023-02-14 21:26 ` Conor Dooley 2023-02-15 20:50 ` Evan Green 2023-02-15 20:50 ` Evan Green 2023-02-06 20:14 ` [PATCH v2 5/6] RISC-V: hwprobe: Support probing of " Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-07 7:02 ` kernel test robot 2023-02-07 7:02 ` kernel test robot 2023-02-15 21:57 ` Conor Dooley 2023-02-15 21:57 ` Conor Dooley 2023-02-18 0:15 ` Evan Green [this message] 2023-02-18 0:15 ` Evan Green 2023-02-06 20:14 ` [PATCH v2 6/6] selftests: Test the new RISC-V hwprobe interface Evan Green 2023-02-06 20:14 ` Evan Green 2023-02-06 21:27 ` Mark Brown 2023-02-06 21:27 ` Mark Brown 2023-02-09 18:44 ` Evan Green 2023-02-09 18:44 ` Evan Green 2023-02-06 21:11 ` [PATCH v2 0/6] RISC-V Hardware Probing User Interface Jessica Clarke 2023-02-06 21:11 ` Jessica Clarke 2023-02-06 22:47 ` Heinrich Schuchardt 2023-02-06 22:47 ` Heinrich Schuchardt 2023-02-09 16:56 ` Palmer Dabbelt 2023-02-09 16:56 ` Palmer Dabbelt 2023-02-06 22:32 ` Conor Dooley 2023-02-06 22:32 ` Conor Dooley
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='CALs-HsvZaKC5R-rAvkjBDNQGTUJ_LWn-O=KzpFwGtpac32_Xxw@mail.gmail.com' \ --to=evan@rivosinc.com \ --cc=abrestic@rivosinc.com \ --cc=ajones@ventanamicro.com \ --cc=aou@eecs.berkeley.edu \ --cc=apatel@ventanamicro.com \ --cc=arnd@arndb.de \ --cc=atishp@rivosinc.com \ --cc=coelacanthus@outlook.com \ --cc=conor@kernel.org \ --cc=corbet@lwn.net \ --cc=guoren@kernel.org \ --cc=heiko@sntech.de \ --cc=heinrich.schuchardt@canonical.com \ --cc=jszhang@kernel.org \ --cc=linux-doc@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-riscv@lists.infradead.org \ --cc=palmer@dabbelt.com \ --cc=palmer@rivosinc.com \ --cc=paul.walmsley@sifive.com \ --cc=research_trasio@irq.a4lg.com \ --cc=slewis@rivosinc.com \ --cc=sunilvl@ventanamicro.com \ --cc=vineetg@rivosinc.com \ --cc=xianting.tian@linux.alibaba.com \ /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.