From: "Andreas Färber" <afaerber@suse.de>
To: Yongbok Kim <yongbok.kim@imgtec.com>, qemu-devel@nongnu.org
Cc: peter.maydell@linaro.org, leon.alrae@imgtec.com
Subject: Re: [Qemu-devel] [PATCH v2 2/2] target-mips: Misaligned memory accesses for MSA
Date: Mon, 11 May 2015 15:12:56 +0200 [thread overview]
Message-ID: <5550AAD8.2080601@suse.de> (raw)
In-Reply-To: <1431343850-46198-3-git-send-email-yongbok.kim@imgtec.com>
Am 11.05.2015 um 13:30 schrieb Yongbok Kim:
> MIPS SIMD Architecture vector loads and stores require misalignment support.
> MSA Memory access should work as an atomic operation. Therefore, it has to
> check validity of all addresses for an access if it is spanning into two pages.
>
> Introduced misaligned flag to indicate MSA ld/st is ongoing, is used to allow
> misaligned accesses in the mips_cpu_do_unaligned_access() callback.
> This is crucial to support MSA misaligned accesses in Release 5 cores.
>
> Further clean up to use mmu_idx from cpu_mmu_index() instead of calculating it
> from hflag.
>
> Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
> ---
> target-mips/cpu.h | 5 +++
> target-mips/helper.c | 33 ++++++++++++++++++++++
> target-mips/op_helper.c | 69 ++++++++++++++++++++++++++++++++++------------
> target-mips/translate.c | 4 +++
> 4 files changed, 93 insertions(+), 18 deletions(-)
>
> diff --git a/target-mips/cpu.h b/target-mips/cpu.h
> index f9d2b4c..1bd1229 100644
> --- a/target-mips/cpu.h
> +++ b/target-mips/cpu.h
> @@ -211,6 +211,9 @@ struct TCState {
> MSACSR_FS_MASK)
>
> float_status msa_fp_status;
> +#if !defined(CONFIG_USER_ONLY)
> + bool misaligned; /* indicates misaligned access is allowed */
> +#endif
> };
>
> typedef struct CPUMIPSState CPUMIPSState;
> @@ -760,6 +763,8 @@ int mips_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int rw,
> void r4k_invalidate_tlb (CPUMIPSState *env, int idx, int use_extra);
> hwaddr cpu_mips_translate_address (CPUMIPSState *env, target_ulong address,
> int rw);
> +bool cpu_mips_validate_msa_block_access(CPUMIPSState *env, target_ulong addr,
> + int rw, int mmu_idx);
Can you please adopt the new style of using mips_cpu_... prefix and
MIPSCPU *cpu argument? You're dealing with conversions both in the call
site and inside the implementation anyway.
> #endif
> target_ulong exception_resume_pc (CPUMIPSState *env);
>
> diff --git a/target-mips/helper.c b/target-mips/helper.c
> index 8e3204a..951aea8 100644
> --- a/target-mips/helper.c
> +++ b/target-mips/helper.c
> @@ -391,6 +391,37 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int r
> }
> }
>
> +bool cpu_mips_validate_msa_block_access(CPUMIPSState *env, target_ulong addr,
> + int rw, int mmu_idx)
> +{
> + target_ulong vaddr = addr & TARGET_PAGE_MASK;
> + target_ulong badvaddr = addr;
> +
> + CPUState *cs = CPU(mips_env_get_cpu(env));
This becomes CPU(cpu) then.
CPUMIPSState *env = &cpu->env;
> + int ret;
> +
> + ret = mips_cpu_handle_mmu_fault(cs, vaddr, rw, mmu_idx);
> + if (ret != TLBRET_MATCH) {
> + /* calling raise_mmu_exeception again to correct badvaddr */
> + raise_mmu_exception(env, badvaddr, rw, ret);
> + return false;
> + }
> + if (unlikely(((addr & ~TARGET_PAGE_MASK) + MSA_WRLEN - 1)
> + >= TARGET_PAGE_SIZE)) {
> + vaddr += TARGET_PAGE_SIZE;
> + ret = mips_cpu_handle_mmu_fault(cs, vaddr, rw, mmu_idx);
> + if (ret != TLBRET_MATCH) {
> + if (ret != TLBRET_BADADDR) {
> + badvaddr = vaddr;
> + }
> + /* calling raise_mmu_exeception again to correct badvaddr */
> + raise_mmu_exception(env, badvaddr, rw, ret);
> + return false;
> + }
> + }
> + return true;
> +}
> +
> static const char * const excp_names[EXCP_LAST + 1] = {
> [EXCP_RESET] = "reset",
> [EXCP_SRESET] = "soft reset",
> @@ -497,6 +528,8 @@ void mips_cpu_do_interrupt(CPUState *cs)
> qemu_log("%s enter: PC " TARGET_FMT_lx " EPC " TARGET_FMT_lx " %s exception\n",
> __func__, env->active_tc.PC, env->CP0_EPC, name);
> }
> +
> + env->active_tc.misaligned = false;
> if (cs->exception_index == EXCP_EXT_INTERRUPT &&
> (env->hflags & MIPS_HFLAG_DM)) {
> cs->exception_index = EXCP_DINT;
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index 58f02cf..b3b8d52 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -47,7 +47,9 @@ static inline void QEMU_NORETURN do_raise_exception_err(CPUMIPSState *env,
> /* now we have a real cpu fault */
> cpu_restore_state(cs, pc);
> }
> -
> +#if !defined(CONFIG_USER_ONLY)
> + env->active_tc.misaligned = false;
> +#endif
> cpu_loop_exit(cs);
> }
>
> @@ -2215,9 +2217,12 @@ void mips_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> int error_code = 0;
> int excp;
>
> - if (env->insn_flags & ISA_MIPS32R6) {
> + if ((env->insn_flags & ISA_MIPS32R6) || (env->active_tc.misaligned)) {
> /* Release 6 provides support for misaligned memory access for
> * all ordinary memory reference instructions
> + *
> + * MIPS SIMD Architecture vector loads and stores support misalignment
> + * memory access
> * */
> return;
> }
> @@ -3571,33 +3576,47 @@ void helper_msa_ld_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs,
> wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> target_ulong addr = env->active_tc.gpr[rs] + (s10 << df);
> int i;
> + int mmu_idx = cpu_mmu_index(env);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (unlikely(((addr & ~TARGET_PAGE_MASK) + MSA_WRLEN - 1)
> + >= TARGET_PAGE_SIZE)) {
> + if (!cpu_mips_validate_msa_block_access(env, addr, MMU_DATA_LOAD,
> + mmu_idx)) {
> + CPUState *cs = CPU(mips_env_get_cpu(env));
MIPSCPU *cpu = mips_env_get_cpu(env);
CPUState *cs = CPU(cpu);
Regards,
Andreas
> + helper_raise_exception_err(env, cs->exception_index,
> + env->error_code);
> + return;
> + }
> + }
> + env->active_tc.misaligned = true;
> +#endif
>
> switch (df) {
> case DF_BYTE:
> for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) {
> - pwd->b[i] = do_lbu(env, addr + (i << DF_BYTE),
> - env->hflags & MIPS_HFLAG_KSU);
> + pwd->b[i] = do_lbu(env, addr + (i << DF_BYTE), mmu_idx);
> }
> break;
> case DF_HALF:
> for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) {
> - pwd->h[i] = do_lhu(env, addr + (i << DF_HALF),
> - env->hflags & MIPS_HFLAG_KSU);
> + pwd->h[i] = do_lhu(env, addr + (i << DF_HALF), mmu_idx);
> }
> break;
> case DF_WORD:
> for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) {
> - pwd->w[i] = do_lw(env, addr + (i << DF_WORD),
> - env->hflags & MIPS_HFLAG_KSU);
> + pwd->w[i] = do_lw(env, addr + (i << DF_WORD), mmu_idx);
> }
> break;
> case DF_DOUBLE:
> for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) {
> - pwd->d[i] = do_ld(env, addr + (i << DF_DOUBLE),
> - env->hflags & MIPS_HFLAG_KSU);
> + pwd->d[i] = do_ld(env, addr + (i << DF_DOUBLE), mmu_idx);
> }
> break;
> }
> +#if !defined(CONFIG_USER_ONLY)
> + env->active_tc.misaligned = false;
> +#endif
> }
>
> void helper_msa_st_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs,
> @@ -3606,31 +3625,45 @@ void helper_msa_st_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t rs,
> wr_t *pwd = &(env->active_fpu.fpr[wd].wr);
> target_ulong addr = env->active_tc.gpr[rs] + (s10 << df);
> int i;
> + int mmu_idx = cpu_mmu_index(env);
> +
> +#if !defined(CONFIG_USER_ONLY)
> + if (unlikely(((addr & ~TARGET_PAGE_MASK) + MSA_WRLEN - 1)
> + >= TARGET_PAGE_SIZE)) {
> + if (!cpu_mips_validate_msa_block_access(env, addr, MMU_DATA_STORE,
> + mmu_idx)) {
> + CPUState *cs = CPU(mips_env_get_cpu(env));
> + helper_raise_exception_err(env, cs->exception_index,
> + env->error_code);
> + return;
> + }
> + }
> + env->active_tc.misaligned = true;
> +#endif
>
> switch (df) {
> case DF_BYTE:
> for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) {
> - do_sb(env, addr + (i << DF_BYTE), pwd->b[i],
> - env->hflags & MIPS_HFLAG_KSU);
> + do_sb(env, addr + (i << DF_BYTE), pwd->b[i], mmu_idx);
> }
> break;
> case DF_HALF:
> for (i = 0; i < DF_ELEMENTS(DF_HALF); i++) {
> - do_sh(env, addr + (i << DF_HALF), pwd->h[i],
> - env->hflags & MIPS_HFLAG_KSU);
> + do_sh(env, addr + (i << DF_HALF), pwd->h[i], mmu_idx);
> }
> break;
> case DF_WORD:
> for (i = 0; i < DF_ELEMENTS(DF_WORD); i++) {
> - do_sw(env, addr + (i << DF_WORD), pwd->w[i],
> - env->hflags & MIPS_HFLAG_KSU);
> + do_sw(env, addr + (i << DF_WORD), pwd->w[i], mmu_idx);
> }
> break;
> case DF_DOUBLE:
> for (i = 0; i < DF_ELEMENTS(DF_DOUBLE); i++) {
> - do_sd(env, addr + (i << DF_DOUBLE), pwd->d[i],
> - env->hflags & MIPS_HFLAG_KSU);
> + do_sd(env, addr + (i << DF_DOUBLE), pwd->d[i], mmu_idx);
> }
> break;
> }
> +#if !defined(CONFIG_USER_ONLY)
> + env->active_tc.misaligned = false;
> +#endif
> }
> diff --git a/target-mips/translate.c b/target-mips/translate.c
> index fd063a2..674b1cb 100644
> --- a/target-mips/translate.c
> +++ b/target-mips/translate.c
> @@ -19641,6 +19641,10 @@ void cpu_state_reset(CPUMIPSState *env)
> restore_rounding_mode(env);
> restore_flush_mode(env);
> cs->exception_index = EXCP_NONE;
> +
> +#ifndef CONFIG_USER_ONLY
> + env->active_tc.misaligned = false;
> +#endif
> }
>
> void restore_state_to_opc(CPUMIPSState *env, TranslationBlock *tb, int pc_pos)
>
--
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Dilip Upmanyu, Graham Norton; HRB
21284 (AG Nürnberg)
next prev parent reply other threads:[~2015-05-11 13:13 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-11 11:30 [Qemu-devel] [PATCH v2 0/2] target-mips: Add support for misaligned accesses Yongbok Kim
2015-05-11 11:30 ` [Qemu-devel] [PATCH v2 1/2] target-mips: Misaligned memory accesses for R6 Yongbok Kim
2015-05-11 13:00 ` Andreas Färber
2015-05-11 11:30 ` [Qemu-devel] [PATCH v2 2/2] target-mips: Misaligned memory accesses for MSA Yongbok Kim
2015-05-11 13:12 ` Andreas Färber [this message]
2015-05-11 13:15 ` Yongbok Kim
2015-05-11 13:52 ` Leon Alrae
2015-05-12 9:54 ` Peter Maydell
2015-05-12 15:38 ` Richard Henderson
2015-05-12 9:43 ` Leon Alrae
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=5550AAD8.2080601@suse.de \
--to=afaerber@suse.de \
--cc=leon.alrae@imgtec.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
--cc=yongbok.kim@imgtec.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).