qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 7/7] target/xtensa: move non-HELPER functions to


From: Max Filippov
Subject: Re: [Qemu-devel] [PATCH 7/7] target/xtensa: move non-HELPER functions to helper.c
Date: Mon, 17 May 2021 08:25:52 -0700

On Mon, May 17, 2021 at 6:10 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 5/17/21 2:11 PM, Max Filippov wrote:
> > On Mon, May 17, 2021 at 4:50 AM Max Filippov <jcmvbkbc@gmail.com> wrote:
> >>
> >> Hi Philippe,
> >>
> >> On Sun, May 16, 2021 at 10:05 PM Philippe Mathieu-Daudé
> >> <philippe@mathieu-daude.net> wrote:
> >>>
> >>> Hi Max,
> >>>
> >>> On Mon, Jan 14, 2019 at 8:52 AM Max Filippov <jcmvbkbc@gmail.com> wrote:
> >>>>
> >>>> Move remaining non-HELPER functions from op_helper.c to helper.c.
> >>>> No functional changes.
> >>>>
> >>>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
> >>>> ---
> >>>>  target/xtensa/helper.c    | 61 
> >>>> ++++++++++++++++++++++++++++++++++++++++++++---
> >>>>  target/xtensa/op_helper.c | 56 
> >>>> -------------------------------------------
> >>>>  2 files changed, 58 insertions(+), 59 deletions(-)
> >>>
> >>>> +void xtensa_cpu_do_unaligned_access(CPUState *cs,
> >>>> +                                    vaddr addr, MMUAccessType 
> >>>> access_type,
> >>>> +                                    int mmu_idx, uintptr_t retaddr)
> >>>> +{
> >>>> +    XtensaCPU *cpu = XTENSA_CPU(cs);
> >>>> +    CPUXtensaState *env = &cpu->env;
> >>>> +
> >>>> +    if (xtensa_option_enabled(env->config, 
> >>>> XTENSA_OPTION_UNALIGNED_EXCEPTION) &&
> >>>> +        !xtensa_option_enabled(env->config, 
> >>>> XTENSA_OPTION_HW_ALIGNMENT)) {
> >>>
> >>> I know this is a simple code movement, but I wonder, what should
> >>> happen when there is
> >>> an unaligned fault and the options are disabled? Is this an impossible
> >>> case (unreachable)?
> >>
> >> It should be unreachable when XTENSA_OPTION_UNALIGNED_EXCEPTION
> >> is disabled. In that case the translation code generates access on aligned
> >> addresses according to the xtensa ISA, see the function
> >> gen_load_store_alignment in target/xtensa/translate.c
> >
> > There's also a case when both options are enabled, i.e. the
> > xtensa core has support for transparent unaligned access.
> > In that case the helper does nothing and the generic TCG
> > code is supposed to deal with the unaligned access correctly,
>
> IIRC we can simplify as:
>
> -- >8 --
> diff --git a/target/xtensa/helper.c b/target/xtensa/helper.c
> index eeffee297d1..6e8a6cdc99e 100644
> --- a/target/xtensa/helper.c
> +++ b/target/xtensa/helper.c
> @@ -270,13 +270,14 @@ void xtensa_cpu_do_unaligned_access(CPUState *cs,
>      XtensaCPU *cpu = XTENSA_CPU(cs);
>      CPUXtensaState *env = &cpu->env;
>
> -    if (xtensa_option_enabled(env->config,
> XTENSA_OPTION_UNALIGNED_EXCEPTION) &&
> -        !xtensa_option_enabled(env->config, XTENSA_OPTION_HW_ALIGNMENT)) {
> -        cpu_restore_state(CPU(cpu), retaddr, true);
> -        HELPER(exception_cause_vaddr)(env,
> -                                      env->pc, LOAD_STORE_ALIGNMENT_CAUSE,
> -                                      addr);
> -    }
> +    assert(xtensa_option_enabled(env->config,
> +                                 XTENSA_OPTION_UNALIGNED_EXCEPTION));

This part -- yes.

> +    assert(!xtensa_option_enabled(env->config,
> XTENSA_OPTION_HW_ALIGNMENT));

This part -- no, because the call to the TCGCPUOps::do_unaligned_access
is unconditional and would happen on CPUs that have
XTENSA_OPTION_HW_ALIGNMENT enabled. They could have a different
CPUClass::tcg_ops, but I'm not sure it's worth it.

-- 
Thanks.
-- Max



reply via email to

[Prev in Thread] Current Thread [Next in Thread]