Message ID | 20210818191920.390759-64-richard.henderson@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | Unaligned access for user-only | expand |
On Wed, 18 Aug 2021 at 21:15, Richard Henderson <richard.henderson@linaro.org> wrote: > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > tcg/tci.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/tcg/tci.c b/tcg/tci.c > index e76087ccac..985c8a91cb 100644 > --- a/tcg/tci.c > +++ b/tcg/tci.c > @@ -296,7 +296,7 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, > uintptr_t ra = (uintptr_t)tb_ptr; > > #ifdef CONFIG_SOFTMMU > - switch (mop) { > + switch (mop & (MO_BSWAP | MO_SSIZE)) { > case MO_UB: > return helper_ret_ldub_mmu(env, taddr, oi, ra); > case MO_SB: > @@ -326,10 +326,14 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, > } > #else > void *haddr = g2h(env_cpu(env), taddr); > + unsigned a_mask = (1u << get_alignment_bits(mop)) - 1; > uint64_t ret; > > set_helper_retaddr(ra); > - switch (mop) { > + if (taddr & a_mask) { > + helper_unaligned_ld(env, taddr); > + } > + switch (mop & (MO_BSWAP | MO_SSIZE)) { > case MO_UB: > ret = ldub_p(haddr); > break; > @@ -377,11 +381,11 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, > static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, > MemOpIdx oi, const void *tb_ptr) > { > - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); > + MemOp mop = get_memop(oi); > uintptr_t ra = (uintptr_t)tb_ptr; Don't you need this bit in tci_qemu_st() as well ? -- PMM
On 8/20/21 3:14 AM, Peter Maydell wrote: >> @@ -296,7 +296,7 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, >> uintptr_t ra = (uintptr_t)tb_ptr; >> >> #ifdef CONFIG_SOFTMMU >> - switch (mop) { >> + switch (mop & (MO_BSWAP | MO_SSIZE)) { >> case MO_UB: >> return helper_ret_ldub_mmu(env, taddr, oi, ra); >> case MO_SB: >> @@ -326,10 +326,14 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, >> } >> #else >> void *haddr = g2h(env_cpu(env), taddr); >> + unsigned a_mask = (1u << get_alignment_bits(mop)) - 1; >> uint64_t ret; >> >> set_helper_retaddr(ra); >> - switch (mop) { >> + if (taddr & a_mask) { >> + helper_unaligned_ld(env, taddr); >> + } >> + switch (mop & (MO_BSWAP | MO_SSIZE)) { >> case MO_UB: >> ret = ldub_p(haddr); >> break; >> @@ -377,11 +381,11 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, >> static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, >> MemOpIdx oi, const void *tb_ptr) >> { >> - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); >> + MemOp mop = get_memop(oi); >> uintptr_t ra = (uintptr_t)tb_ptr; > > Don't you need this bit in tci_qemu_st() as well ? Which bit isn't present in st as well? There's missing hunks in your reply, but afaics they're the same. r~
On Sun, 22 Aug 2021 at 08:59, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 8/20/21 3:14 AM, Peter Maydell wrote: > >> @@ -377,11 +381,11 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, > >> static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, > >> MemOpIdx oi, const void *tb_ptr) > >> { > >> - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); > >> + MemOp mop = get_memop(oi); > >> uintptr_t ra = (uintptr_t)tb_ptr; > > > > Don't you need this bit in tci_qemu_st() as well ? > > Which bit isn't present in st as well? > There's missing hunks in your reply, but afaics they're the same. https://patchew.org/QEMU/20210818191920.390759-1-richard.henderson@linaro.org/20210818191920.390759-64-richard.henderson@linaro.org/ I had the function name wrong, but only the tci_qemu_st() change has this bit: - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); + MemOp mop = get_memop(oi); -- PMM
On 8/22/21 5:32 AM, Peter Maydell wrote: > On Sun, 22 Aug 2021 at 08:59, Richard Henderson > <richard.henderson@linaro.org> wrote: >> >> On 8/20/21 3:14 AM, Peter Maydell wrote: >>>> @@ -377,11 +381,11 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, >>>> static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, >>>> MemOpIdx oi, const void *tb_ptr) >>>> { >>>> - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); >>>> + MemOp mop = get_memop(oi); >>>> uintptr_t ra = (uintptr_t)tb_ptr; >>> >>> Don't you need this bit in tci_qemu_st() as well ? >> >> Which bit isn't present in st as well? >> There's missing hunks in your reply, but afaics they're the same. > > https://patchew.org/QEMU/20210818191920.390759-1-richard.henderson@linaro.org/20210818191920.390759-64-richard.henderson@linaro.org/ > > I had the function name wrong, but only the tci_qemu_st() change > has this bit: > > - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); > + MemOp mop = get_memop(oi); Ah yes, thanks. r~
diff --git a/tcg/tci.c b/tcg/tci.c index e76087ccac..985c8a91cb 100644 --- a/tcg/tci.c +++ b/tcg/tci.c @@ -296,7 +296,7 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, uintptr_t ra = (uintptr_t)tb_ptr; #ifdef CONFIG_SOFTMMU - switch (mop) { + switch (mop & (MO_BSWAP | MO_SSIZE)) { case MO_UB: return helper_ret_ldub_mmu(env, taddr, oi, ra); case MO_SB: @@ -326,10 +326,14 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, } #else void *haddr = g2h(env_cpu(env), taddr); + unsigned a_mask = (1u << get_alignment_bits(mop)) - 1; uint64_t ret; set_helper_retaddr(ra); - switch (mop) { + if (taddr & a_mask) { + helper_unaligned_ld(env, taddr); + } + switch (mop & (MO_BSWAP | MO_SSIZE)) { case MO_UB: ret = ldub_p(haddr); break; @@ -377,11 +381,11 @@ static uint64_t tci_qemu_ld(CPUArchState *env, target_ulong taddr, static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, MemOpIdx oi, const void *tb_ptr) { - MemOp mop = get_memop(oi) & (MO_BSWAP | MO_SSIZE); + MemOp mop = get_memop(oi); uintptr_t ra = (uintptr_t)tb_ptr; #ifdef CONFIG_SOFTMMU - switch (mop) { + switch (mop & (MO_BSWAP | MO_SIZE)) { case MO_UB: helper_ret_stb_mmu(env, taddr, val, oi, ra); break; @@ -408,9 +412,13 @@ static void tci_qemu_st(CPUArchState *env, target_ulong taddr, uint64_t val, } #else void *haddr = g2h(env_cpu(env), taddr); + unsigned a_mask = (1u << get_alignment_bits(mop)) - 1; set_helper_retaddr(ra); - switch (mop) { + if (taddr & a_mask) { + helper_unaligned_st(env, taddr); + } + switch (mop & (MO_BSWAP | MO_SIZE)) { case MO_UB: stb_p(haddr, val); break;
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- tcg/tci.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) -- 2.25.1