| Message ID | 20220521000400.454525-6-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | semihosting cleanup | expand |
On 5/20/22 17:03, Richard Henderson wrote: > Mirror the interface of the user-only function of the same name. > Use probe_access_flags for the common case of ram, and > cpu_memory_rw_debug for the uncommon case of mmio. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > v3: Use probe_access_flags (pmm) Ho hum. MIPS still selects semihosting without TCG, which means that probe_access_flags is not present. At present I'm assuming that semihosting can't work with KVM, and that we should disable the feature. r~ > --- > include/semihosting/softmmu-uaccess.h | 3 ++ > semihosting/uaccess.c | 49 +++++++++++++++++++++++++++ > 2 files changed, 52 insertions(+) > > diff --git a/include/semihosting/softmmu-uaccess.h b/include/semihosting/softmmu-uaccess.h > index 03300376d3..4f08dfc098 100644 > --- a/include/semihosting/softmmu-uaccess.h > +++ b/include/semihosting/softmmu-uaccess.h > @@ -53,4 +53,7 @@ void softmmu_unlock_user(CPUArchState *env, void *p, > target_ulong addr, target_ulong len); > #define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr); > +#define target_strlen(p) softmmu_strlen_user(env, p) > + > #endif /* SEMIHOSTING_SOFTMMU_UACCESS_H */ > diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c > index 0d3b32b75d..51019b79ff 100644 > --- a/semihosting/uaccess.c > +++ b/semihosting/uaccess.c > @@ -8,6 +8,7 @@ > */ > > #include "qemu/osdep.h" > +#include "exec/exec-all.h" > #include "semihosting/softmmu-uaccess.h" > > void *softmmu_lock_user(CPUArchState *env, target_ulong addr, > @@ -23,6 +24,54 @@ void *softmmu_lock_user(CPUArchState *env, target_ulong addr, > return p; > } > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr) > +{ > + int mmu_idx = cpu_mmu_index(env, false); > + size_t len = 0; > + > + while (1) { > + size_t left_in_page; > + int flags; > + void *h; > + > + /* Find the number of bytes remaining in the page. */ > + left_in_page = -(addr | TARGET_PAGE_MASK); > + > + flags = probe_access_flags(env, addr, MMU_DATA_LOAD, > + mmu_idx, true, &h, 0); > + if (flags & TLB_INVALID_MASK) { > + return -1; > + } > + if (flags & TLB_MMIO) { > + do { > + uint8_t c; > + if (cpu_memory_rw_debug(env_cpu(env), addr, &c, 1, 0)) { > + return -1; > + } > + if (c == 0) { > + return len; > + } > + addr++; > + len++; > + if (len > INT32_MAX) { > + return -1; > + } > + } while (--left_in_page != 0); > + } else { > + char *p = memchr(h, 0, left_in_page); > + if (p) { > + len += p - (char *)h; > + return len <= INT32_MAX ? (ssize_t)len : -1; > + } > + addr += left_in_page; > + len += left_in_page; > + if (len > INT32_MAX) { > + return -1; > + } > + } > + } > +} > + > char *softmmu_lock_user_string(CPUArchState *env, target_ulong addr) > { > /* TODO: Make this something that isn't fixed size. */
On Sat, 21 May 2022 at 05:51, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 5/20/22 17:03, Richard Henderson wrote: > > Mirror the interface of the user-only function of the same name. > > Use probe_access_flags for the common case of ram, and > > cpu_memory_rw_debug for the uncommon case of mmio. > > > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > > --- > > v3: Use probe_access_flags (pmm) > > Ho hum. MIPS still selects semihosting without TCG, which means that > probe_access_flags is not present. > > At present I'm assuming that semihosting can't work with KVM, and that we should disable > the feature. > > > r~ > > > --- > > include/semihosting/softmmu-uaccess.h | 3 ++ > > semihosting/uaccess.c | 49 +++++++++++++++++++++++++++ > > 2 files changed, 52 insertions(+) > > > > diff --git a/include/semihosting/softmmu-uaccess.h b/include/semihosting/softmmu-uaccess.h > > index 03300376d3..4f08dfc098 100644 > > --- a/include/semihosting/softmmu-uaccess.h > > +++ b/include/semihosting/softmmu-uaccess.h > > @@ -53,4 +53,7 @@ void softmmu_unlock_user(CPUArchState *env, void *p, > > target_ulong addr, target_ulong len); > > #define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) > > > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr); > > +#define target_strlen(p) softmmu_strlen_user(env, p) > > + > > #endif /* SEMIHOSTING_SOFTMMU_UACCESS_H */ > > diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c > > index 0d3b32b75d..51019b79ff 100644 > > --- a/semihosting/uaccess.c > > +++ b/semihosting/uaccess.c > > @@ -8,6 +8,7 @@ > > */ > > > > #include "qemu/osdep.h" > > +#include "exec/exec-all.h" > > #include "semihosting/softmmu-uaccess.h" > > > > void *softmmu_lock_user(CPUArchState *env, target_ulong addr, > > @@ -23,6 +24,54 @@ void *softmmu_lock_user(CPUArchState *env, target_ulong addr, > > return p; > > } > > > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr) > > +{ > > + int mmu_idx = cpu_mmu_index(env, false); > > + size_t len = 0; > > + > > + while (1) { > > + size_t left_in_page; > > + int flags; > > + void *h; > > + > > + /* Find the number of bytes remaining in the page. */ > > + left_in_page = -(addr | TARGET_PAGE_MASK); This works but I'm never really a fan of expressions that mix arithmetic and logical operations -- I always have to start writing out bit patterns and checking what 2s-complement representation involves in order to find out what they really do. left_in_page = TARGET_PAGE_SIZE - (addr & ~TARGET_PAGE_MASK); seems to me like it would be clearer. You're correct that MIPS shouldn't be selecting semihosting in a KVM-only compile -- there's no code to handle getting from the guest executing the sdbbp insn to QEMU userspace to the code in QEMU to deal with a semihosting call. So for this patch: Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
diff --git a/include/semihosting/softmmu-uaccess.h b/include/semihosting/softmmu-uaccess.h index 03300376d3..4f08dfc098 100644 --- a/include/semihosting/softmmu-uaccess.h +++ b/include/semihosting/softmmu-uaccess.h @@ -53,4 +53,7 @@ void softmmu_unlock_user(CPUArchState *env, void *p, target_ulong addr, target_ulong len); #define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr); +#define target_strlen(p) softmmu_strlen_user(env, p) + #endif /* SEMIHOSTING_SOFTMMU_UACCESS_H */ diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c index 0d3b32b75d..51019b79ff 100644 --- a/semihosting/uaccess.c +++ b/semihosting/uaccess.c @@ -8,6 +8,7 @@ */ #include "qemu/osdep.h" +#include "exec/exec-all.h" #include "semihosting/softmmu-uaccess.h" void *softmmu_lock_user(CPUArchState *env, target_ulong addr, @@ -23,6 +24,54 @@ void *softmmu_lock_user(CPUArchState *env, target_ulong addr, return p; } +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr) +{ + int mmu_idx = cpu_mmu_index(env, false); + size_t len = 0; + + while (1) { + size_t left_in_page; + int flags; + void *h; + + /* Find the number of bytes remaining in the page. */ + left_in_page = -(addr | TARGET_PAGE_MASK); + + flags = probe_access_flags(env, addr, MMU_DATA_LOAD, + mmu_idx, true, &h, 0); + if (flags & TLB_INVALID_MASK) { + return -1; + } + if (flags & TLB_MMIO) { + do { + uint8_t c; + if (cpu_memory_rw_debug(env_cpu(env), addr, &c, 1, 0)) { + return -1; + } + if (c == 0) { + return len; + } + addr++; + len++; + if (len > INT32_MAX) { + return -1; + } + } while (--left_in_page != 0); + } else { + char *p = memchr(h, 0, left_in_page); + if (p) { + len += p - (char *)h; + return len <= INT32_MAX ? (ssize_t)len : -1; + } + addr += left_in_page; + len += left_in_page; + if (len > INT32_MAX) { + return -1; + } + } + } +} + char *softmmu_lock_user_string(CPUArchState *env, target_ulong addr) { /* TODO: Make this something that isn't fixed size. */
Mirror the interface of the user-only function of the same name. Use probe_access_flags for the common case of ram, and cpu_memory_rw_debug for the uncommon case of mmio. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- v3: Use probe_access_flags (pmm) --- include/semihosting/softmmu-uaccess.h | 3 ++ semihosting/uaccess.c | 49 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+)