On 01/05/15 16:24, Yongbok Kim wrote:
> 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 the addresses for the operation.
As far as I can tell mips_cpu_do_unaligned_access() will be still
generating AdE exceptions for MSA loads/stores in MIPS R5 which doesn't
seem to be correct.
>
> Signed-off-by: Yongbok Kim <yongbok....@imgtec.com>
> ---
> target-mips/op_helper.c | 30 ++++++++++++++++++++++++++++++
> 1 files changed, 30 insertions(+), 0 deletions(-)
>
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index dacc92b..89a7de6 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -3571,6 +3571,24 @@ FOP_CONDN_S(sne, (float32_lt(fst1, fst0,
> &env->active_fpu.fp_status)
> /* Element-by-element access macros */
> #define DF_ELEMENTS(df) (MSA_WRLEN / DF_BITS(df))
>
> +#if !defined(CONFIG_USER_ONLY)
> +static bool cpu_mips_validate_msa_block_access(CPUMIPSState *env,
> + target_ulong address, int df, int rw)
> +{
> + int i;
> + for (i = 0; i < DF_ELEMENTS(df); i++) {
Do we really need to check each element, wouldn't byte 0 and byte
(MSA_WRLEN/8 - 1) be enough?
> + if (!cpu_mips_validate_access(env, address + (i << df),
> + address, (1 << df), rw)) {
I believe this would look better if this line was aligned with the first
argument of the function (and would be consistent with the helper below).
> + CPUState *cs = CPU(mips_env_get_cpu(env));
> + helper_raise_exception_err(env, cs->exception_index,
> + env->error_code);
> + return false;
> + }
> + }
> + return true;
> +}
> +#endif
> +
> void helper_msa_ld_df(CPUMIPSState *env, uint32_t df, uint32_t wd, uint32_t
> rs,
> int32_t s10)
> {
> @@ -3578,6 +3596,12 @@ void helper_msa_ld_df(CPUMIPSState *env, uint32_t df,
> uint32_t wd, uint32_t rs,
> target_ulong addr = env->active_tc.gpr[rs] + (s10 << df);
> int i;
>
> +#if !defined(CONFIG_USER_ONLY)
> + if (!cpu_mips_validate_msa_block_access(env, addr, df, MMU_DATA_LOAD)) {
> + return;
> + }
Shouldn't this be called only for cases where page boundary is crossed?
Otherwise I don't think this validation is needed.
> +#endif
> +
> switch (df) {
> case DF_BYTE:
> for (i = 0; i < DF_ELEMENTS(DF_BYTE); i++) {
> @@ -3613,6 +3637,12 @@ void helper_msa_st_df(CPUMIPSState *env, uint32_t df,
> uint32_t wd, uint32_t rs,
> target_ulong addr = env->active_tc.gpr[rs] + (s10 << df);
> int i;
>
> +#if !defined(CONFIG_USER_ONLY)
> + if (!cpu_mips_validate_msa_block_access(env, addr, df, MMU_DATA_STORE)) {
> + return;
> + }
Same.
Thanks,
Leon
