* [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe @ 2023-06-02 9:41 Robbin Ehn 2023-06-02 14:02 ` Andrew Jones 2023-06-03 2:57 ` Richard Henderson 0 siblings, 2 replies; 11+ messages in thread From: Robbin Ehn @ 2023-06-02 9:41 UTC (permalink / raw) To: qemu-devel; +Cc: laurent, qemu-riscv, richard.henderson This patch adds the new syscall for the "RISC-V Hardware Probing Interface" (https://docs.kernel.org/riscv/hwprobe.html). Signed-off-by: Robbin Ehn <rehn@rivosinc.com> --- v1->v2: Moved to syscall.c --- linux-user/riscv/syscall32_nr.h | 1 + linux-user/riscv/syscall64_nr.h | 1 + linux-user/syscall.c | 109 ++++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+) diff --git a/linux-user/riscv/syscall32_nr.h b/linux-user/riscv/syscall32_nr.h index 1327d7dffa..412e58e5b2 100644 --- a/linux-user/riscv/syscall32_nr.h +++ b/linux-user/riscv/syscall32_nr.h @@ -228,6 +228,7 @@ #define TARGET_NR_accept4 242 #define TARGET_NR_arch_specific_syscall 244 #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) #define TARGET_NR_prlimit64 261 #define TARGET_NR_fanotify_init 262 #define TARGET_NR_fanotify_mark 263 diff --git a/linux-user/riscv/syscall64_nr.h b/linux-user/riscv/syscall64_nr.h index 6659751933..29e1eb2075 100644 --- a/linux-user/riscv/syscall64_nr.h +++ b/linux-user/riscv/syscall64_nr.h @@ -251,6 +251,7 @@ #define TARGET_NR_recvmmsg 243 #define TARGET_NR_arch_specific_syscall 244 #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) #define TARGET_NR_wait4 260 #define TARGET_NR_prlimit64 261 #define TARGET_NR_fanotify_init 262 diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 89b58b386b..cd394bbe26 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -8772,6 +8772,74 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) } #endif /* TARGET_NR_getdents64 */ +#if defined(TARGET_RISCV) + +#define RISCV_HWPROBE_KEY_MVENDORID 0 +#define RISCV_HWPROBE_KEY_MARCHID 1 +#define RISCV_HWPROBE_KEY_MIMPID 2 + +#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR 3 +#define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) + +#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_UNSUPPORTED (4 << 0) +#define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) + +struct riscv_hwprobe { + int64_t key; + uint64_t value; +}; + +static void risc_hwprobe_fill_pairs(CPURISCVState *env, + struct riscv_hwprobe *pair, + size_t pair_count) +{ + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env); + + for (; pair_count > 0; pair_count--, pair++) { + pair->value = 0; + switch (pair->key) { + case RISCV_HWPROBE_KEY_MVENDORID: + pair->value = cfg->mvendorid; + break; + case RISCV_HWPROBE_KEY_MARCHID: + pair->value = cfg->marchid; + break; + case RISCV_HWPROBE_KEY_MIMPID: + pair->value = cfg->mimpid; + break; + case RISCV_HWPROBE_KEY_BASE_BEHAVIOR: + pair->value = riscv_has_ext(env, RVI) && + riscv_has_ext(env, RVM) && + riscv_has_ext(env, RVA) ? + RISCV_HWPROBE_BASE_BEHAVIOR_IMA : 0; + break; + case RISCV_HWPROBE_KEY_IMA_EXT_0: + pair->value = riscv_has_ext(env, RVF) && + riscv_has_ext(env, RVD) ? + RISCV_HWPROBE_IMA_FD : 0; + pair->value |= riscv_has_ext(env, RVC) ? + RISCV_HWPROBE_IMA_C : pair->value; + break; + case RISCV_HWPROBE_KEY_CPUPERF_0: + pair->value = RISCV_HWPROBE_MISALIGNED_UNKNOWN; + break; + default: + pair->key = -1; + break; + } + } +} +#endif + #if defined(TARGET_NR_pivot_root) && defined(__NR_pivot_root) _syscall2(int, pivot_root, const char *, new_root, const char *, put_old) #endif @@ -13469,6 +13537,47 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, return ret; #endif +#if defined(TARGET_RISCV) + case TARGET_NR_riscv_hwprobe: + { + struct riscv_hwprobe *host_pairs; + + /* flags must be 0 */ + if (arg5 != 0) { + return -TARGET_EINVAL; + } + + /* check cpu_set */ + if (arg3 != 0) { + int ccpu; + size_t cpu_setsize = CPU_ALLOC_SIZE(arg3); + cpu_set_t *host_cpus = lock_user(VERIFY_READ, arg4, + cpu_setsize, 0); + if (!host_cpus) { + return -TARGET_EFAULT; + } + ccpu = CPU_COUNT_S(cpu_setsize, host_cpus); + unlock_user(host_cpus, arg4, cpu_setsize); + /* no selected cpu */ + if (ccpu == 0) { + return -TARGET_EINVAL; + } + } else if (arg4 != 0) { + return -TARGET_EINVAL; + } + + host_pairs = lock_user(VERIFY_WRITE, arg1, + sizeof(*host_pairs) * (size_t)arg2, 0); + if (host_pairs == NULL) { + return -TARGET_EFAULT; + } + risc_hwprobe_fill_pairs(cpu_env, host_pairs, arg2); + unlock_user(host_pairs, arg1, sizeof(*host_pairs) * (size_t)arg2); + ret = 0; + } + return ret; +#endif + default: qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num); return -TARGET_ENOSYS; -- 2.39.2 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 9:41 [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe Robbin Ehn @ 2023-06-02 14:02 ` Andrew Jones 2023-06-02 14:39 ` Robbin Ehn 2023-06-03 2:58 ` Richard Henderson 2023-06-03 2:57 ` Richard Henderson 1 sibling, 2 replies; 11+ messages in thread From: Andrew Jones @ 2023-06-02 14:02 UTC (permalink / raw) To: Robbin Ehn; +Cc: qemu-devel, laurent, qemu-riscv, richard.henderson On Fri, Jun 02, 2023 at 11:41:11AM +0200, Robbin Ehn wrote: > This patch adds the new syscall for the > "RISC-V Hardware Probing Interface" > (https://docs.kernel.org/riscv/hwprobe.html). > > Signed-off-by: Robbin Ehn <rehn@rivosinc.com> > --- > v1->v2: Moved to syscall.c > --- > linux-user/riscv/syscall32_nr.h | 1 + > linux-user/riscv/syscall64_nr.h | 1 + > linux-user/syscall.c | 109 ++++++++++++++++++++++++++++++++ > 3 files changed, 111 insertions(+) > > diff --git a/linux-user/riscv/syscall32_nr.h b/linux-user/riscv/syscall32_nr.h > index 1327d7dffa..412e58e5b2 100644 > --- a/linux-user/riscv/syscall32_nr.h > +++ b/linux-user/riscv/syscall32_nr.h This file should not be modified, it should be generated, but this is an RFC, so hacking it is OK, but the hack should be in a separate patch. > @@ -228,6 +228,7 @@ > #define TARGET_NR_accept4 242 > #define TARGET_NR_arch_specific_syscall 244 > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > #define TARGET_NR_prlimit64 261 > #define TARGET_NR_fanotify_init 262 > #define TARGET_NR_fanotify_mark 263 > diff --git a/linux-user/riscv/syscall64_nr.h b/linux-user/riscv/syscall64_nr.h > index 6659751933..29e1eb2075 100644 > --- a/linux-user/riscv/syscall64_nr.h > +++ b/linux-user/riscv/syscall64_nr.h Same > @@ -251,6 +251,7 @@ > #define TARGET_NR_recvmmsg 243 > #define TARGET_NR_arch_specific_syscall 244 > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > #define TARGET_NR_wait4 260 > #define TARGET_NR_prlimit64 261 > #define TARGET_NR_fanotify_init 262 > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > index 89b58b386b..cd394bbe26 100644 > --- a/linux-user/syscall.c > +++ b/linux-user/syscall.c > @@ -8772,6 +8772,74 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) > } > #endif /* TARGET_NR_getdents64 */ > > +#if defined(TARGET_RISCV) > + > +#define RISCV_HWPROBE_KEY_MVENDORID 0 > +#define RISCV_HWPROBE_KEY_MARCHID 1 > +#define RISCV_HWPROBE_KEY_MIMPID 2 > + > +#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR 3 > +#define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) > + > +#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_UNSUPPORTED (4 << 0) > +#define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) > + > +struct riscv_hwprobe { > + int64_t key; > + uint64_t value; > +}; The above is all uapi so Linux's arch/riscv/include/uapi/asm/hwprobe.h should be picked up on Linux header update. You'll need to modify the script, scripts/update-linux-headers.sh, to do that by adding a new riscv-specific block. Hacking this by importing the header file manually is fine for an RFC, but that should be a separate patch or part of the syscall define hack patch. And hack patches should be clearly tagged as "NOT FOR MERGE". > + > +static void risc_hwprobe_fill_pairs(CPURISCVState *env, > + struct riscv_hwprobe *pair, > + size_t pair_count) > +{ > + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env); > + > + for (; pair_count > 0; pair_count--, pair++) { > + pair->value = 0; > + switch (pair->key) { > + case RISCV_HWPROBE_KEY_MVENDORID: > + pair->value = cfg->mvendorid; > + break; > + case RISCV_HWPROBE_KEY_MARCHID: > + pair->value = cfg->marchid; > + break; > + case RISCV_HWPROBE_KEY_MIMPID: > + pair->value = cfg->mimpid; > + break; > + case RISCV_HWPROBE_KEY_BASE_BEHAVIOR: > + pair->value = riscv_has_ext(env, RVI) && > + riscv_has_ext(env, RVM) && > + riscv_has_ext(env, RVA) ? > + RISCV_HWPROBE_BASE_BEHAVIOR_IMA : 0; > + break; > + case RISCV_HWPROBE_KEY_IMA_EXT_0: > + pair->value = riscv_has_ext(env, RVF) && > + riscv_has_ext(env, RVD) ? > + RISCV_HWPROBE_IMA_FD : 0; > + pair->value |= riscv_has_ext(env, RVC) ? > + RISCV_HWPROBE_IMA_C : pair->value; > + break; > + case RISCV_HWPROBE_KEY_CPUPERF_0: > + pair->value = RISCV_HWPROBE_MISALIGNED_UNKNOWN; > + break; > + default: > + pair->key = -1; > + break; > + } > + } > +} > +#endif > + > #if defined(TARGET_NR_pivot_root) && defined(__NR_pivot_root) > _syscall2(int, pivot_root, const char *, new_root, const char *, put_old) > #endif > @@ -13469,6 +13537,47 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, > return ret; > #endif > > +#if defined(TARGET_RISCV) > + case TARGET_NR_riscv_hwprobe: > + { The { goes under the c of case, which will shift all the below four spaces left as well. > + struct riscv_hwprobe *host_pairs; > + > + /* flags must be 0 */ > + if (arg5 != 0) { > + return -TARGET_EINVAL; > + } > + > + /* check cpu_set */ > + if (arg3 != 0) { > + int ccpu; > + size_t cpu_setsize = CPU_ALLOC_SIZE(arg3); > + cpu_set_t *host_cpus = lock_user(VERIFY_READ, arg4, > + cpu_setsize, 0); > + if (!host_cpus) { > + return -TARGET_EFAULT; > + } > + ccpu = CPU_COUNT_S(cpu_setsize, host_cpus); > + unlock_user(host_cpus, arg4, cpu_setsize); > + /* no selected cpu */ > + if (ccpu == 0) { > + return -TARGET_EINVAL; > + } > + } else if (arg4 != 0) { > + return -TARGET_EINVAL; > + } I think we want if (arg2 == 0) return 0; here. > + > + host_pairs = lock_user(VERIFY_WRITE, arg1, > + sizeof(*host_pairs) * (size_t)arg2, 0); > + if (host_pairs == NULL) { > + return -TARGET_EFAULT; > + } > + risc_hwprobe_fill_pairs(cpu_env, host_pairs, arg2); > + unlock_user(host_pairs, arg1, sizeof(*host_pairs) * (size_t)arg2); > + ret = 0; > + } > + return ret; > +#endif > + > default: > qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num); > return -TARGET_ENOSYS; > -- > 2.39.2 > > Otherwise this looks good to me. Thanks, drew ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 14:02 ` Andrew Jones @ 2023-06-02 14:39 ` Robbin Ehn 2023-06-02 15:07 ` Andrew Jones 2023-06-03 2:58 ` Richard Henderson 1 sibling, 1 reply; 11+ messages in thread From: Robbin Ehn @ 2023-06-02 14:39 UTC (permalink / raw) To: Andrew Jones; +Cc: qemu-devel, laurent, qemu-riscv, richard.henderson On Fri, 2023-06-02 at 16:02 +0200, Andrew Jones wrote: > On Fri, Jun 02, 2023 at 11:41:11AM +0200, Robbin Ehn wrote: > > This patch adds the new syscall for the > > "RISC-V Hardware Probing Interface" > > (https://docs.kernel.org/riscv/hwprobe.html). > > > > Signed-off-by: Robbin Ehn <rehn@rivosinc.com> > > --- > > v1->v2: Moved to syscall.c > > --- > > linux-user/riscv/syscall32_nr.h | 1 + > > linux-user/riscv/syscall64_nr.h | 1 + > > linux-user/syscall.c | 109 ++++++++++++++++++++++++++++++++ > > 3 files changed, 111 insertions(+) > > > > diff --git a/linux-user/riscv/syscall32_nr.h b/linux-user/riscv/syscall32_nr.h > > index 1327d7dffa..412e58e5b2 100644 > > --- a/linux-user/riscv/syscall32_nr.h > > +++ b/linux-user/riscv/syscall32_nr.h > > This file should not be modified, it should be generated, but this is an > RFC, so hacking it is OK, but the hack should be in a separate patch. Ok, thanks. > > > @@ -228,6 +228,7 @@ > > #define TARGET_NR_accept4 242 > > #define TARGET_NR_arch_specific_syscall 244 > > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > > #define TARGET_NR_prlimit64 261 > > #define TARGET_NR_fanotify_init 262 > > #define TARGET_NR_fanotify_mark 263 > > diff --git a/linux-user/riscv/syscall64_nr.h b/linux-user/riscv/syscall64_nr.h > > index 6659751933..29e1eb2075 100644 > > --- a/linux-user/riscv/syscall64_nr.h > > +++ b/linux-user/riscv/syscall64_nr.h > > Same Ok, thanks. > > > @@ -251,6 +251,7 @@ > > #define TARGET_NR_recvmmsg 243 > > #define TARGET_NR_arch_specific_syscall 244 > > #define TARGET_NR_riscv_flush_icache (TARGET_NR_arch_specific_syscall + 15) > > +#define TARGET_NR_riscv_hwprobe (TARGET_NR_arch_specific_syscall + 14) > > #define TARGET_NR_wait4 260 > > #define TARGET_NR_prlimit64 261 > > #define TARGET_NR_fanotify_init 262 > > diff --git a/linux-user/syscall.c b/linux-user/syscall.c > > index 89b58b386b..cd394bbe26 100644 > > --- a/linux-user/syscall.c > > +++ b/linux-user/syscall.c > > @@ -8772,6 +8772,74 @@ static int do_getdents64(abi_long dirfd, abi_long arg2, abi_long count) > > } > > #endif /* TARGET_NR_getdents64 */ > > > > +#if defined(TARGET_RISCV) > > + > > +#define RISCV_HWPROBE_KEY_MVENDORID 0 > > +#define RISCV_HWPROBE_KEY_MARCHID 1 > > +#define RISCV_HWPROBE_KEY_MIMPID 2 > > + > > +#define RISCV_HWPROBE_KEY_BASE_BEHAVIOR 3 > > +#define RISCV_HWPROBE_BASE_BEHAVIOR_IMA (1 << 0) > > + > > +#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_UNSUPPORTED (4 << 0) > > +#define RISCV_HWPROBE_MISALIGNED_MASK (7 << 0) > > + > > +struct riscv_hwprobe { > > + int64_t key; > > + uint64_t value; > > +}; > > The above is all uapi so Linux's arch/riscv/include/uapi/asm/hwprobe.h > should be picked up on Linux header update. You'll need to modify the > script, scripts/update-linux-headers.sh, to do that by adding a new > riscv-specific block. Hacking this by importing the header file manually > is fine for an RFC, but that should be a separate patch or part of the > syscall define hack patch. And hack patches should be clearly tagged as > "NOT FOR MERGE". Ok, thanks. > > > + > > +static void risc_hwprobe_fill_pairs(CPURISCVState *env, > > + struct riscv_hwprobe *pair, > > + size_t pair_count) > > +{ > > + const RISCVCPUConfig *cfg = riscv_cpu_cfg(env); > > + > > + for (; pair_count > 0; pair_count--, pair++) { > > + pair->value = 0; > > + switch (pair->key) { > > + case RISCV_HWPROBE_KEY_MVENDORID: > > + pair->value = cfg->mvendorid; > > + break; > > + case RISCV_HWPROBE_KEY_MARCHID: > > + pair->value = cfg->marchid; > > + break; > > + case RISCV_HWPROBE_KEY_MIMPID: > > + pair->value = cfg->mimpid; > > + break; > > + case RISCV_HWPROBE_KEY_BASE_BEHAVIOR: > > + pair->value = riscv_has_ext(env, RVI) && > > + riscv_has_ext(env, RVM) && > > + riscv_has_ext(env, RVA) ? > > + RISCV_HWPROBE_BASE_BEHAVIOR_IMA : 0; > > + break; > > + case RISCV_HWPROBE_KEY_IMA_EXT_0: > > + pair->value = riscv_has_ext(env, RVF) && > > + riscv_has_ext(env, RVD) ? > > + RISCV_HWPROBE_IMA_FD : 0; > > + pair->value |= riscv_has_ext(env, RVC) ? > > + RISCV_HWPROBE_IMA_C : pair->value; > > + break; > > + case RISCV_HWPROBE_KEY_CPUPERF_0: > > + pair->value = RISCV_HWPROBE_MISALIGNED_UNKNOWN; > > + break; > > + default: > > + pair->key = -1; > > + break; > > + } > > + } > > +} > > +#endif > > + > > #if defined(TARGET_NR_pivot_root) && defined(__NR_pivot_root) > > _syscall2(int, pivot_root, const char *, new_root, const char *, put_old) > > #endif > > @@ -13469,6 +13537,47 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, > > return ret; > > #endif > > > > +#if defined(TARGET_RISCV) > > + case TARGET_NR_riscv_hwprobe: > > + { > > The { goes under the c of case, which will shift all the below four spaces > left as well. This was an attempt to blend in, i.e. same style as the preceding case. I'll change, thanks. > > > + struct riscv_hwprobe *host_pairs; > > + > > + /* flags must be 0 */ > > + if (arg5 != 0) { > > + return -TARGET_EINVAL; > > + } > > + > > + /* check cpu_set */ > > + if (arg3 != 0) { > > + int ccpu; > > + size_t cpu_setsize = CPU_ALLOC_SIZE(arg3); > > + cpu_set_t *host_cpus = lock_user(VERIFY_READ, arg4, > > + cpu_setsize, 0); > > + if (!host_cpus) { > > + return -TARGET_EFAULT; > > + } > > + ccpu = CPU_COUNT_S(cpu_setsize, host_cpus); > > + unlock_user(host_cpus, arg4, cpu_setsize); > > + /* no selected cpu */ > > + if (ccpu == 0) { > > + return -TARGET_EINVAL; > > + } > > + } else if (arg4 != 0) { > > + return -TARGET_EINVAL; > > + } > > I think we want > > if (arg2 == 0) > return 0; > > here. Ok, thanks. > > > + > > + host_pairs = lock_user(VERIFY_WRITE, arg1, > > + sizeof(*host_pairs) * (size_t)arg2, 0); > > + if (host_pairs == NULL) { > > + return -TARGET_EFAULT; > > + } > > + risc_hwprobe_fill_pairs(cpu_env, host_pairs, arg2); > > + unlock_user(host_pairs, arg1, sizeof(*host_pairs) * (size_t)arg2); > > + ret = 0; > > + } > > + return ret; > > +#endif > > + > > default: > > qemu_log_mask(LOG_UNIMP, "Unsupported syscall: %d\n", num); > > return -TARGET_ENOSYS; > > -- > > 2.39.2 > > > > > > Otherwise this looks good to me. Thank you! /Robbin > > Thanks, > drew ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 14:39 ` Robbin Ehn @ 2023-06-02 15:07 ` Andrew Jones 2023-06-03 3:00 ` Richard Henderson 0 siblings, 1 reply; 11+ messages in thread From: Andrew Jones @ 2023-06-02 15:07 UTC (permalink / raw) To: Robbin Ehn; +Cc: qemu-devel, laurent, qemu-riscv, richard.henderson On Fri, Jun 02, 2023 at 04:39:20PM +0200, Robbin Ehn wrote: > On Fri, 2023-06-02 at 16:02 +0200, Andrew Jones wrote: > > On Fri, Jun 02, 2023 at 11:41:11AM +0200, Robbin Ehn wrote: ... > > > +#if defined(TARGET_RISCV) > > > + case TARGET_NR_riscv_hwprobe: > > > + { > > > > The { goes under the c of case, which will shift all the below four spaces > > left as well. > > This was an attempt to blend in, i.e. same style as the preceding case. > I'll change, thanks. Hmm, I see. This function does have many cases with the indented format, but not all of them, and the rest of the code base doesn't indent. I won't insist on changing this, as long as checkpatch isn't complaining. Thanks, drew ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 15:07 ` Andrew Jones @ 2023-06-03 3:00 ` Richard Henderson 2023-06-05 14:27 ` Robbin Ehn 0 siblings, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-06-03 3:00 UTC (permalink / raw) To: Andrew Jones, Robbin Ehn; +Cc: qemu-devel, laurent, qemu-riscv On 6/2/23 08:07, Andrew Jones wrote: > On Fri, Jun 02, 2023 at 04:39:20PM +0200, Robbin Ehn wrote: >> On Fri, 2023-06-02 at 16:02 +0200, Andrew Jones wrote: >>> On Fri, Jun 02, 2023 at 11:41:11AM +0200, Robbin Ehn wrote: > ... >>>> +#if defined(TARGET_RISCV) >>>> + case TARGET_NR_riscv_hwprobe: >>>> + { >>> >>> The { goes under the c of case, which will shift all the below four spaces >>> left as well. >> >> This was an attempt to blend in, i.e. same style as the preceding case. >> I'll change, thanks. > > Hmm, I see. This function does have many cases with the indented format, > but not all of them, and the rest of the code base doesn't indent. I won't > insist on changing this, as long as checkpatch isn't complaining. Splitting the entire thing out to a helper function is even cleaner. We have lots of those, but certainly not universal. I have, from time to time, tried to clean all of this up, but no one wanted to look at a 100+ RFC patch set which only scratched the surface. r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-03 3:00 ` Richard Henderson @ 2023-06-05 14:27 ` Robbin Ehn 0 siblings, 0 replies; 11+ messages in thread From: Robbin Ehn @ 2023-06-05 14:27 UTC (permalink / raw) To: Richard Henderson, Andrew Jones; +Cc: qemu-devel, laurent, qemu-riscv On Fri, 2023-06-02 at 20:00 -0700, Richard Henderson wrote: > On 6/2/23 08:07, Andrew Jones wrote: > > On Fri, Jun 02, 2023 at 04:39:20PM +0200, Robbin Ehn wrote: > > > On Fri, 2023-06-02 at 16:02 +0200, Andrew Jones wrote: > > > > On Fri, Jun 02, 2023 at 11:41:11AM +0200, Robbin Ehn wrote: > > ... > > > > > +#if defined(TARGET_RISCV) > > > > > + case TARGET_NR_riscv_hwprobe: > > > > > + { > > > > > > > > The { goes under the c of case, which will shift all the below four spaces > > > > left as well. > > > > > > This was an attempt to blend in, i.e. same style as the preceding case. > > > I'll change, thanks. > > > > Hmm, I see. This function does have many cases with the indented format, > > but not all of them, and the rest of the code base doesn't indent. I won't > > insist on changing this, as long as checkpatch isn't complaining. > > Splitting the entire thing out to a helper function is even cleaner. > We have lots of those, but certainly not universal. This was my initial reaction, even move something out of this file. Yes I'll put it in a helper. Thank you! /Robbin > > I have, from time to time, tried to clean all of this up, but no one wanted to look at a > 100+ RFC patch set which only scratched the surface. > > > r~ > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 14:02 ` Andrew Jones 2023-06-02 14:39 ` Robbin Ehn @ 2023-06-03 2:58 ` Richard Henderson 2023-06-03 15:50 ` Andrew Jones 1 sibling, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-06-03 2:58 UTC (permalink / raw) To: Andrew Jones, Robbin Ehn; +Cc: qemu-devel, laurent, qemu-riscv On 6/2/23 07:02, Andrew Jones wrote: >> +struct riscv_hwprobe { >> + int64_t key; >> + uint64_t value; >> +}; > > The above is all uapi so Linux's arch/riscv/include/uapi/asm/hwprobe.h > should be picked up on Linux header update. You'll need to modify the > script, scripts/update-linux-headers.sh, to do that by adding a new > riscv-specific block. Hacking this by importing the header file manually > is fine for an RFC, but that should be a separate patch or part of the > syscall define hack patch. And hack patches should be clearly tagged as > "NOT FOR MERGE". Not true. linux-user/ never looks at linux-headers/. r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-03 2:58 ` Richard Henderson @ 2023-06-03 15:50 ` Andrew Jones 2023-06-03 17:48 ` Richard Henderson 0 siblings, 1 reply; 11+ messages in thread From: Andrew Jones @ 2023-06-03 15:50 UTC (permalink / raw) To: Richard Henderson; +Cc: Robbin Ehn, qemu-devel, laurent, qemu-riscv On Fri, Jun 02, 2023 at 07:58:30PM -0700, Richard Henderson wrote: > On 6/2/23 07:02, Andrew Jones wrote: > > > +struct riscv_hwprobe { > > > + int64_t key; > > > + uint64_t value; > > > +}; > > > > The above is all uapi so Linux's arch/riscv/include/uapi/asm/hwprobe.h > > should be picked up on Linux header update. You'll need to modify the > > script, scripts/update-linux-headers.sh, to do that by adding a new > > riscv-specific block. Hacking this by importing the header file manually > > is fine for an RFC, but that should be a separate patch or part of the > > syscall define hack patch. And hack patches should be clearly tagged as > > "NOT FOR MERGE". > > > Not true. linux-user/ never looks at linux-headers/. Ah, thanks. I should have known better than to try and review a linux-user patch, since I know almost nothing about it! Is uapi like this usually duplicated, as was done in this patch? Thanks, drew ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-03 15:50 ` Andrew Jones @ 2023-06-03 17:48 ` Richard Henderson 0 siblings, 0 replies; 11+ messages in thread From: Richard Henderson @ 2023-06-03 17:48 UTC (permalink / raw) To: Andrew Jones; +Cc: Robbin Ehn, qemu-devel, laurent, qemu-riscv On 6/3/23 08:50, Andrew Jones wrote: > On Fri, Jun 02, 2023 at 07:58:30PM -0700, Richard Henderson wrote: >> On 6/2/23 07:02, Andrew Jones wrote: >>>> +struct riscv_hwprobe { >>>> + int64_t key; >>>> + uint64_t value; >>>> +}; >>> >>> The above is all uapi so Linux's arch/riscv/include/uapi/asm/hwprobe.h >>> should be picked up on Linux header update. You'll need to modify the >>> script, scripts/update-linux-headers.sh, to do that by adding a new >>> riscv-specific block. Hacking this by importing the header file manually >>> is fine for an RFC, but that should be a separate patch or part of the >>> syscall define hack patch. And hack patches should be clearly tagged as >>> "NOT FOR MERGE". >> >> >> Not true. linux-user/ never looks at linux-headers/. > > Ah, thanks. I should have known better than to try and review a linux-user > patch, since I know almost nothing about it! Is uapi like this usually > duplicated, as was done in this patch? Yes, because linux-headers is only for the "native" compiler, whereas linux-user requires the ABI of the guest. If we were doing this all from scratch, we would mechanically parse and rewrite linux-headers. r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-02 9:41 [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe Robbin Ehn 2023-06-02 14:02 ` Andrew Jones @ 2023-06-03 2:57 ` Richard Henderson 2023-06-05 14:23 ` Robbin Ehn 1 sibling, 1 reply; 11+ messages in thread From: Richard Henderson @ 2023-06-03 2:57 UTC (permalink / raw) To: Robbin Ehn, qemu-devel; +Cc: laurent, qemu-riscv On 6/2/23 02:41, Robbin Ehn wrote: > +struct riscv_hwprobe { > + int64_t key; > + uint64_t value; > +}; This needs to use abi_llong and abi_ullong, as the guest may not have the same alignment requirements as the host. > + case RISCV_HWPROBE_KEY_MVENDORID: > + pair->value = cfg->mvendorid; > + break; You must use __get_user and __put_user to handle host vs guest endianness. All over. > + case RISCV_HWPROBE_KEY_CPUPERF_0: > + pair->value = RISCV_HWPROBE_MISALIGNED_UNKNOWN; Is that really what you want to expose here? FAST is always going to be true, in that handling the unaligned access in the host is going to be faster than in the emulated guest. > + default: > + pair->key = -1; > + break; Misalignment. > +#if defined(TARGET_RISCV) > + case TARGET_NR_riscv_hwprobe: > + { > + struct riscv_hwprobe *host_pairs; > + > + /* flags must be 0 */ > + if (arg5 != 0) { > + return -TARGET_EINVAL; > + } > + > + /* check cpu_set */ > + if (arg3 != 0) { > + int ccpu; > + size_t cpu_setsize = CPU_ALLOC_SIZE(arg3); > + cpu_set_t *host_cpus = lock_user(VERIFY_READ, arg4, > + cpu_setsize, 0); > + if (!host_cpus) { > + return -TARGET_EFAULT; > + } > + ccpu = CPU_COUNT_S(cpu_setsize, host_cpus); Where does CPU_ALLOC_SIZE and CPU_COUNT_S come from? > + unlock_user(host_cpus, arg4, cpu_setsize); > + /* no selected cpu */ > + if (ccpu == 0) { > + return -TARGET_EINVAL; > + } I suppose you're just looking to see that the set is not empty? r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe 2023-06-03 2:57 ` Richard Henderson @ 2023-06-05 14:23 ` Robbin Ehn 0 siblings, 0 replies; 11+ messages in thread From: Robbin Ehn @ 2023-06-05 14:23 UTC (permalink / raw) To: Richard Henderson, qemu-devel; +Cc: laurent, qemu-riscv On Fri, 2023-06-02 at 19:57 -0700, Richard Henderson wrote: > > > + case RISCV_HWPROBE_KEY_CPUPERF_0: > > + pair->value = RISCV_HWPROBE_MISALIGNED_UNKNOWN; > > Is that really what you want to expose here? FAST is always going to be true, in that > handling the unaligned access in the host is going to be faster than in the emulated guest. The plan was to add this in the cpu cfg in a later patch. This setting e.g. changes jitted code and therefore it's helpful if such generated code is the same in the emulated guest as it would be on that actual cpu. I'll change to FAST as the hardcoded value until then. > > > Where does CPU_ALLOC_SIZE and CPU_COUNT_S come from? > > > + unlock_user(host_cpus, arg4, cpu_setsize); > > + /* no selected cpu */ > > + if (ccpu == 0) { > > + return -TARGET_EINVAL; > > + } > > I suppose you're just looking to see that the set is not empty? Yes, exactly. Thanks again! I'll send out an update. /Robbin > > > r~ ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-06-05 14:28 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-06-02 9:41 [RFC v2] linux-user/riscv: Add syscall riscv_hwprobe Robbin Ehn 2023-06-02 14:02 ` Andrew Jones 2023-06-02 14:39 ` Robbin Ehn 2023-06-02 15:07 ` Andrew Jones 2023-06-03 3:00 ` Richard Henderson 2023-06-05 14:27 ` Robbin Ehn 2023-06-03 2:58 ` Richard Henderson 2023-06-03 15:50 ` Andrew Jones 2023-06-03 17:48 ` Richard Henderson 2023-06-03 2:57 ` Richard Henderson 2023-06-05 14:23 ` Robbin Ehn
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).