qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: <qemu-devel@nongnu.org>, <edgar.iglesias@gmail.com>, <philmd@linaro.org>
Subject: Re: [PATCH v2 04/10] target/microblaze: Implement extended address load/store out of line
Date: Sun, 25 May 2025 21:33:36 +0200	[thread overview]
Message-ID: <aDNwkAvUfFlj7cSp@zapote> (raw)
In-Reply-To: <20250525160220.222154-5-richard.henderson@linaro.org>

On Sun, May 25, 2025 at 05:02:14PM +0100, Richard Henderson wrote:
> Use helpers and address_space_ld/st instead of inline
> loads and stores.  This allows us to perform operations
> on physical addresses wider than virtual addresses.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  target/microblaze/helper.h    | 10 +++++++
>  target/microblaze/op_helper.c | 40 +++++++++++++++++++++++++++
>  target/microblaze/translate.c | 52 +++++++++++++++++++++++++++--------
>  3 files changed, 90 insertions(+), 12 deletions(-)
> 
> diff --git a/target/microblaze/helper.h b/target/microblaze/helper.h
> index 41f56a5601..ef4fad9b91 100644
> --- a/target/microblaze/helper.h
> +++ b/target/microblaze/helper.h
> @@ -28,4 +28,14 @@ DEF_HELPER_FLAGS_3(put, TCG_CALL_NO_RWG, void, i32, i32, i32)
>  DEF_HELPER_FLAGS_3(mmu_read, TCG_CALL_NO_RWG, i32, env, i32, i32)
>  DEF_HELPER_FLAGS_4(mmu_write, TCG_CALL_NO_RWG, void, env, i32, i32, i32)
>  DEF_HELPER_FLAGS_2(unaligned_access, TCG_CALL_NO_WG, noreturn, env, i64)
> +DEF_HELPER_FLAGS_2(lbuea, TCG_CALL_NO_WG, i32, env, i64)
> +DEF_HELPER_FLAGS_2(lhuea_be, TCG_CALL_NO_WG, i32, env, i64)
> +DEF_HELPER_FLAGS_2(lhuea_le, TCG_CALL_NO_WG, i32, env, i64)
> +DEF_HELPER_FLAGS_2(lwea_be, TCG_CALL_NO_WG, i32, env, i64)
> +DEF_HELPER_FLAGS_2(lwea_le, TCG_CALL_NO_WG, i32, env, i64)
> +DEF_HELPER_FLAGS_3(sbea, TCG_CALL_NO_WG, void, env, i32, i64)
> +DEF_HELPER_FLAGS_3(shea_be, TCG_CALL_NO_WG, void, env, i32, i64)
> +DEF_HELPER_FLAGS_3(shea_le, TCG_CALL_NO_WG, void, env, i32, i64)
> +DEF_HELPER_FLAGS_3(swea_be, TCG_CALL_NO_WG, void, env, i32, i64)
> +DEF_HELPER_FLAGS_3(swea_le, TCG_CALL_NO_WG, void, env, i32, i64)
>  #endif
> diff --git a/target/microblaze/op_helper.c b/target/microblaze/op_helper.c
> index 4c39207a55..b8365b3b1d 100644
> --- a/target/microblaze/op_helper.c
> +++ b/target/microblaze/op_helper.c
> @@ -382,6 +382,8 @@ void helper_stackprot(CPUMBState *env, target_ulong addr)
>  }
>  
>  #if !defined(CONFIG_USER_ONLY)
> +#include "system/memory.h"
> +
>  /* Writes/reads to the MMU's special regs end up here.  */
>  uint32_t helper_mmu_read(CPUMBState *env, uint32_t ext, uint32_t rn)
>  {
> @@ -441,4 +443,42 @@ void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
>      mb_transaction_failed_internal(cs, physaddr, addr, size,
>                                     access_type, retaddr);
>  }
> +
> +#define LD_EA(NAME, TYPE, FUNC) \
> +uint32_t HELPER(NAME)(CPUMBState *env, uint64_t ea)                     \
> +{                                                                       \
> +    CPUState *cs = env_cpu(env);                                        \
> +    MemTxResult txres;                                                  \
> +    TYPE ret = FUNC(cs->as, ea, MEMTXATTRS_UNSPECIFIED, &txres);        \
> +    if (unlikely(txres != MEMTX_OK)) {                                  \
> +        mb_transaction_failed_internal(cs, ea, ea, sizeof(TYPE),        \
> +                                       MMU_DATA_LOAD, GETPC());         \
> +    }                                                                   \
> +    return ret;                                                         \
> +}
> +
> +LD_EA(lbuea, uint8_t, address_space_ldub)
> +LD_EA(lhuea_be, uint16_t, address_space_lduw_be)
> +LD_EA(lhuea_le, uint16_t, address_space_lduw_le)
> +LD_EA(lwea_be, uint32_t, address_space_ldl_be)
> +LD_EA(lwea_le, uint32_t, address_space_ldl_le)
> +
> +#define ST_EA(NAME, TYPE, FUNC) \
> +void HELPER(NAME)(CPUMBState *env, uint32_t data, uint64_t ea)          \
> +{                                                                       \
> +    CPUState *cs = env_cpu(env);                                        \
> +    MemTxResult txres;                                                  \
> +    FUNC(cs->as, ea, data, MEMTXATTRS_UNSPECIFIED, &txres);             \
> +    if (unlikely(txres != MEMTX_OK)) {                                  \
> +        mb_transaction_failed_internal(cs, ea, ea, sizeof(TYPE),        \
> +                                       MMU_DATA_STORE, GETPC());        \
> +    }                                                                   \
> +}
> +
> +ST_EA(sbea, uint8_t, address_space_stb)
> +ST_EA(shea_be, uint16_t, address_space_stw_be)
> +ST_EA(shea_le, uint16_t, address_space_stw_le)
> +ST_EA(swea_be, uint32_t, address_space_stl_be)
> +ST_EA(swea_le, uint32_t, address_space_stl_le)
> +
>  #endif
> diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
> index 671b1ae4db..3d9756391e 100644
> --- a/target/microblaze/translate.c
> +++ b/target/microblaze/translate.c
> @@ -700,6 +700,20 @@ static void record_unaligned_ess(DisasContext *dc, int rd,
>  
>      tcg_set_insn_start_param(dc->base.insn_start, 1, iflags);
>  }
> +
> +static void gen_alignment_check_ea(DisasContext *dc, TCGv_i64 ea, int rb,
> +                                   int rd, MemOp size, bool store)
> +{
> +    if (rb && (dc->tb_flags & MSR_EE) && dc->cfg->unaligned_exceptions) {
> +        TCGLabel *over = gen_new_label();
> +
> +        record_unaligned_ess(dc, rd, size, store);
> +
> +        tcg_gen_brcondi_i64(TCG_COND_TSTEQ, ea, (1 << size) - 1, over);
> +        gen_helper_unaligned_access(tcg_env, ea);
> +        gen_set_label(over);
> +    }
> +}
>  #endif
>  
>  static inline MemOp mo_endian(DisasContext *dc)
> @@ -765,10 +779,11 @@ static bool trans_lbuea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_load(dc, arg->rd, addr, MO_UB, MMU_NOMMU_IDX, false);
> +    gen_helper_lbuea(reg_for_write(dc, arg->rd), tcg_env, addr);
> +    return true;
>  #endif
>  }
>  
> @@ -796,10 +811,13 @@ static bool trans_lhuea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_load(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
> +    gen_alignment_check_ea(dc, addr, arg->rb, arg->rd, MO_16, false);
> +    (mo_endian(dc) == MO_BE ? gen_helper_lhuea_be : gen_helper_lhuea_le)
> +        (reg_for_write(dc, arg->rd), tcg_env, addr);
> +    return true;
>  #endif
>  }
>  
> @@ -827,10 +845,13 @@ static bool trans_lwea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_load(dc, arg->rd, addr, MO_UL, MMU_NOMMU_IDX, false);
> +    gen_alignment_check_ea(dc, addr, arg->rb, arg->rd, MO_32, false);
> +    (mo_endian(dc) == MO_BE ? gen_helper_lwea_be : gen_helper_lwea_le)
> +        (reg_for_write(dc, arg->rd), tcg_env, addr);
> +    return true;
>  #endif
>  }
>  
> @@ -918,10 +939,11 @@ static bool trans_sbea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_store(dc, arg->rd, addr, MO_UB, MMU_NOMMU_IDX, false);
> +    gen_helper_sbea(tcg_env, reg_for_read(dc, arg->rd), addr);
> +    return true;
>  #endif
>  }
>  
> @@ -949,10 +971,13 @@ static bool trans_shea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_store(dc, arg->rd, addr, MO_UW, MMU_NOMMU_IDX, false);
> +    gen_alignment_check_ea(dc, addr, arg->rb, arg->rd, MO_16, true);
> +    (mo_endian(dc) == MO_BE ? gen_helper_shea_be : gen_helper_shea_le)
> +        (tcg_env, reg_for_read(dc, arg->rd), addr);
> +    return true;
>  #endif
>  }
>  
> @@ -980,10 +1005,13 @@ static bool trans_swea(DisasContext *dc, arg_typea *arg)
>          return true;
>      }
>  #ifdef CONFIG_USER_ONLY
> -    return true;
> +    g_assert_not_reached();
>  #else
>      TCGv addr = compute_ldst_addr_ea(dc, arg->ra, arg->rb);
> -    return do_store(dc, arg->rd, addr, MO_UL, MMU_NOMMU_IDX, false);
> +    gen_alignment_check_ea(dc, addr, arg->rb, arg->rd, MO_32, true);
> +    (mo_endian(dc) == MO_BE ? gen_helper_swea_be : gen_helper_swea_le)
> +        (tcg_env, reg_for_read(dc, arg->rd), addr);
> +    return true;
>  #endif
>  }
>  
> -- 
> 2.43.0
> 


  reply	other threads:[~2025-05-25 19:34 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-25 16:02 [PATCH v2 00/10] target/microblaze: Always use TARGET_LONG_BITS == 32 Richard Henderson
2025-05-25 16:02 ` [PATCH v2 01/10] target/microblaze: Split out mb_unaligned_access_internal Richard Henderson
2025-05-25 19:26   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 02/10] target/microblaze: Introduce helper_unaligned_access Richard Henderson
2025-05-25 19:27   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 03/10] target/microblaze: Split out mb_transaction_failed_internal Richard Henderson
2025-05-25 19:29   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 04/10] target/microblaze: Implement extended address load/store out of line Richard Henderson
2025-05-25 19:33   ` Edgar E. Iglesias [this message]
2025-05-25 16:02 ` [PATCH v2 05/10] target/microblaze: Use uint64_t for CPUMBState.ear Richard Henderson
2025-05-25 19:34   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 06/10] target/microblaze: Use TCGv_i64 for compute_ldst_addr_ea Richard Henderson
2025-05-25 19:35   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 07/10] target/microblaze: Fix printf format in mmu_translate Richard Henderson
2025-05-25 19:36   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 08/10] target/microblaze: Use TARGET_LONG_BITS == 32 for system mode Richard Henderson
2025-05-25 19:36   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 09/10] target/microblaze: Drop DisasContext.r0 Richard Henderson
2025-05-25 19:38   ` Edgar E. Iglesias
2025-05-25 16:02 ` [PATCH v2 10/10] target/microblaze: Simplify compute_ldst_addr_type{a, b} Richard Henderson
2025-05-25 19:40   ` [PATCH v2 10/10] target/microblaze: Simplify compute_ldst_addr_type{a,b} Edgar E. Iglesias

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=aDNwkAvUfFlj7cSp@zapote \
    --to=edgar.iglesias@amd.com \
    --cc=edgar.iglesias@gmail.com \
    --cc=philmd@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /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).