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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.