| Message ID | 20250313034524.3069690-10-richard.henderson@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | accel/tcg, codebase: Build once patches | expand |
On 3/12/25 20:44, Richard Henderson wrote: > Perform aligned atomic reads in translator_ld, if possible. > According to > > https://lore.kernel.org/qemu-devel/20240607101403.1109-1-jim.shu@sifive.com/ > > this is required for RISC-V Ziccif. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > accel/tcg/translator.c | 42 ++++++++++++++++++++++++++++++++++++++---- > 1 file changed, 38 insertions(+), 4 deletions(-) > > diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c > index ef1538b4fc..0260fb1915 100644 > --- a/accel/tcg/translator.c > +++ b/accel/tcg/translator.c > @@ -265,12 +265,14 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db, > > if (likely(((base ^ last) & TARGET_PAGE_MASK) == 0)) { > /* Entire read is from the first page. */ > - memcpy(dest, host + (pc - base), len); > - return true; > + goto do_read; > } > > if (unlikely(((base ^ pc) & TARGET_PAGE_MASK) == 0)) { > - /* Read begins on the first page and extends to the second. */ > + /* > + * Read begins on the first page and extends to the second. > + * The unaligned read is never atomic. > + */ > size_t len0 = -(pc | TARGET_PAGE_MASK); > memcpy(dest, host + (pc - base), len0); > pc += len0; > @@ -329,7 +331,39 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db, > host = db->host_addr[1]; > } > > - memcpy(dest, host + (pc - base), len); > + do_read: > + /* > + * Assume aligned reads should be atomic, if possible. > + * We're not in a position to jump out with EXCP_ATOMIC. > + */ > + host += pc - base; > + switch (len) { > + case 2: > + if (QEMU_IS_ALIGNED(pc, 2)) { > + uint16_t t = qatomic_read((uint16_t *)host); > + stw_he_p(dest, t); > + return true; > + } > + break; > + case 4: > + if (QEMU_IS_ALIGNED(pc, 4)) { > + uint32_t t = qatomic_read((uint32_t *)host); > + stl_he_p(dest, t); > + return true; > + } > + break; > +#ifdef CONFIG_ATOMIC64 > + case 8: > + if (QEMU_IS_ALIGNED(pc, 8)) { > + uint64_t t = qatomic_read__nocheck((uint64_t *)host); > + stl_he_p(dest, t); Should it be stq_he_p? > + return true; > + } > + break; > +#endif > + } > + /* Unaligned or partial read from the second page is not atomic. */ > + memcpy(dest, host, len); > return true; > } >
On 3/13/25 10:14, Pierrick Bouvier wrote: >> +#ifdef CONFIG_ATOMIC64 >> + case 8: >> + if (QEMU_IS_ALIGNED(pc, 8)) { >> + uint64_t t = qatomic_read__nocheck((uint64_t *)host); >> + stl_he_p(dest, t); > > Should it be stq_he_p? Good eyes, thanks. This would have only appeared as disassembly errors on x86_64 guest. r~
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index ef1538b4fc..0260fb1915 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -265,12 +265,14 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db, if (likely(((base ^ last) & TARGET_PAGE_MASK) == 0)) { /* Entire read is from the first page. */ - memcpy(dest, host + (pc - base), len); - return true; + goto do_read; } if (unlikely(((base ^ pc) & TARGET_PAGE_MASK) == 0)) { - /* Read begins on the first page and extends to the second. */ + /* + * Read begins on the first page and extends to the second. + * The unaligned read is never atomic. + */ size_t len0 = -(pc | TARGET_PAGE_MASK); memcpy(dest, host + (pc - base), len0); pc += len0; @@ -329,7 +331,39 @@ static bool translator_ld(CPUArchState *env, DisasContextBase *db, host = db->host_addr[1]; } - memcpy(dest, host + (pc - base), len); + do_read: + /* + * Assume aligned reads should be atomic, if possible. + * We're not in a position to jump out with EXCP_ATOMIC. + */ + host += pc - base; + switch (len) { + case 2: + if (QEMU_IS_ALIGNED(pc, 2)) { + uint16_t t = qatomic_read((uint16_t *)host); + stw_he_p(dest, t); + return true; + } + break; + case 4: + if (QEMU_IS_ALIGNED(pc, 4)) { + uint32_t t = qatomic_read((uint32_t *)host); + stl_he_p(dest, t); + return true; + } + break; +#ifdef CONFIG_ATOMIC64 + case 8: + if (QEMU_IS_ALIGNED(pc, 8)) { + uint64_t t = qatomic_read__nocheck((uint64_t *)host); + stl_he_p(dest, t); + return true; + } + break; +#endif + } + /* Unaligned or partial read from the second page is not atomic. */ + memcpy(dest, host, len); return true; }
Perform aligned atomic reads in translator_ld, if possible. According to https://lore.kernel.org/qemu-devel/20240607101403.1109-1-jim.shu@sifive.com/ this is required for RISC-V Ziccif. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- accel/tcg/translator.c | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-)