Re: [PATCH v6 29/66] target/microblaze: Make mb_cpu_tlb_fill sysemu only

[Edgar E. Iglesias]

On Sun, Oct 31, 2021 at 11:14:04AM +0100, Philippe Mathieu-Daudé wrote:
> On 10/30/21 19:15, Richard Henderson wrote:
> > The fallback code in cpu_loop_exit_sigsegv is sufficient
> > for microblaze linux-user.
> > 
> > Remove the code from cpu_loop that handled the unnamed 0xaa exception.
> > 
> > Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> > ---
> >  target/microblaze/cpu.h          |  8 ++++----
> >  linux-user/microblaze/cpu_loop.c | 10 ----------
> >  target/microblaze/cpu.c          |  2 +-
> >  target/microblaze/helper.c       | 13 +------------
> >  4 files changed, 6 insertions(+), 27 deletions(-)
> > 
> > diff --git a/target/microblaze/cpu.h b/target/microblaze/cpu.h
> > index b7a848bbae..e9cd0b88de 100644
> > --- a/target/microblaze/cpu.h
> > +++ b/target/microblaze/cpu.h
> > @@ -394,10 +394,6 @@ void mb_tcg_init(void);
> >  #define MMU_USER_IDX    2
> >  /* See NB_MMU_MODES further up the file.  */
> >  
> > -bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > -                     MMUAccessType access_type, int mmu_idx,
> > -                     bool probe, uintptr_t retaddr);
> > -
> >  typedef CPUMBState CPUArchState;
> >  typedef MicroBlazeCPU ArchCPU;
> >  
> > @@ -415,6 +411,10 @@ static inline void cpu_get_tb_cpu_state(CPUMBState 
> > *env, target_ulong *pc,
> >  }
> >  
> >  #if !defined(CONFIG_USER_ONLY)
> > +bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > +                     MMUAccessType access_type, int mmu_idx,
> > +                     bool probe, uintptr_t retaddr);
> > +
> >  void mb_cpu_transaction_failed(CPUState *cs, hwaddr physaddr, vaddr addr,
> >                                 unsigned size, MMUAccessType access_type,
> >                                 int mmu_idx, MemTxAttrs attrs,
> > diff --git a/linux-user/microblaze/cpu_loop.c 
> > b/linux-user/microblaze/cpu_loop.c
> > index 52222eb93f..a94467dd2d 100644
> > --- a/linux-user/microblaze/cpu_loop.c
> > +++ b/linux-user/microblaze/cpu_loop.c
> > @@ -37,16 +37,6 @@ void cpu_loop(CPUMBState *env)
> >          process_queued_cpu_work(cs);
> >  
> >          switch (trapnr) {
> > -        case 0xaa:
> > -            {
> > -                info.si_signo = TARGET_SIGSEGV;
> > -                info.si_errno = 0;
> > -                /* XXX: check env->error_code */
> > -                info.si_code = TARGET_SEGV_MAPERR;
> > -                info._sifields._sigfault._addr = 0;
> > -                queue_signal(env, info.si_signo, QEMU_SI_FAULT, &info);
> > -            }
> > -            break;
> >          case EXCP_INTERRUPT:
> >            /* just indicate that signals should be handled asap */
> >            break;
> > diff --git a/target/microblaze/cpu.c b/target/microblaze/cpu.c
> > index 15db277925..b9c888b87e 100644
> > --- a/target/microblaze/cpu.c
> > +++ b/target/microblaze/cpu.c
> > @@ -365,9 +365,9 @@ static const struct SysemuCPUOps mb_sysemu_ops = {
> >  static const struct TCGCPUOps mb_tcg_ops = {
> >      .initialize = mb_tcg_init,
> >      .synchronize_from_tb = mb_cpu_synchronize_from_tb,
> > -    .tlb_fill = mb_cpu_tlb_fill,
> >  
> >  #ifndef CONFIG_USER_ONLY
> > +    .tlb_fill = mb_cpu_tlb_fill,
> >      .cpu_exec_interrupt = mb_cpu_exec_interrupt,
> >      .do_interrupt = mb_cpu_do_interrupt,
> >      .do_transaction_failed = mb_cpu_transaction_failed,
> > diff --git a/target/microblaze/helper.c b/target/microblaze/helper.c
> > index dd2aecd1d5..a607fe68e5 100644
> > --- a/target/microblaze/helper.c
> > +++ b/target/microblaze/helper.c
> > @@ -24,18 +24,7 @@
> >  #include "qemu/host-utils.h"
> >  #include "exec/log.h"
> >  
> > -#if defined(CONFIG_USER_ONLY)
> > -
> > -bool mb_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> > -                     MMUAccessType access_type, int mmu_idx,
> > -                     bool probe, uintptr_t retaddr)
> > -{
> > -    cs->exception_index = 0xaa;
> > -    cpu_loop_exit_restore(cs, retaddr);
> > -}
> > -
> > -#else /* !CONFIG_USER_ONLY */
> > -
> > +#ifndef CONFIG_USER_ONLY
> >  static bool mb_cpu_access_is_secure(MicroBlazeCPU *cpu,
> >                                      MMUAccessType access_type)
> >  {
> > 
> 
> To the best of my knowledge:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> But I'd feel safer with an Ack-by from Edgar :)


Sorry for the delays!

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