Re: [PATCH 05/11] accel/tcg: Modifies memory access functions to use CPUState

[Anton Johansson]

On 9/12/23 21:34, Richard Henderson wrote:

> On 9/12/23 08:34, Anton Johansson wrote:
> > do_[ld|st]*() and mmu_lookup*() are changed to use CPUState over
> > CPUArchState, moving the target-dependence to the target-facing facing
> > cpu_[ld|st] functions.
> >
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >   accel/tcg/cputlb.c | 324 ++++++++++++++++++++++-----------------------
> >   1 file changed, 161 insertions(+), 163 deletions(-)
>
> So... what's your ultimate plan here?
>
> At the moment through patches 5-11, all you do is take CPUArchState, 
> discard knowledge of it via CPUState, and then recover knowledge of it 
> via cpu->tlb_ptr.
>
> I agree that *something* has to happen in order to allow these entry 
> points to be used by multiple cpu types simultaneously, but there must 
> be a plan.
>
> Is it to have tcg generated code perform env_cpu() inline before the 
> call?  That's just pointer arithmetic, so it's certainly an easy option.
My bad, I wasn't expressing this clearly enough, but yes that is
more or less the idea.  We need full access to env for two things:
getting to CPUState, and getting the mmu_idx.  This needs to happen
on the target side, somehow.  My ideas here are:

    For non-helpers (e.g. cpu_ldb_mmu):
        - These functions are just thin wrappers around
          do_[ld|st]*_mmu(), env_cpu() and cpu_mmu_index(),
          and can be compiled with each target;
        - or make these functions take CPUState, and explicitly
          call env_cpu on the target side. Functions needing
          to call cpu_mmu_index() can be compiled with the target
          (~20 small functions);
        - or, Minimize calls to env_cpu() and cpu_mmu_index(), as
          we have done, and make them non-inline so they can work
          on opaque pointers.

    For helpers (helper_ldub_mmu):
        - Either compile with the target as with non-helpers
          since these are also thin wrappers;
        - or allow helpers to only accept CPUState and
          automatically convert env -> cpu when calling
          (what you suggested)

Other ideas and approaches are ofc very welcome!:)

--
Anton Johansson,
rev.ng Labs Srl.