Re: [Qemu-devel] [PATCH v10 08/10] target-avr: adding instruction translation

[Richard Henderson]

On 07/06/2016 03:33 PM, Michael Rolnik wrote:
> Signed-off-by: Michael Rolnik <address@hidden>
> ---
>  target-avr/helper.c         |  118 +-
>  target-avr/helper.h         |    1 +
>  target-avr/translate-inst.c | 2621 
> +++++++++++++++++++++++++++++++++++++++++++
>  target-avr/translate.c      |    2 +-
>  target-avr/translate.h      |  119 ++
>  5 files changed, 2830 insertions(+), 31 deletions(-)
>  create mode 100644 target-avr/translate-inst.c
>  create mode 100644 target-avr/translate.h
> 
> diff --git a/target-avr/helper.c b/target-avr/helper.c
> index 1a324cb..a804f1f 100644
> --- a/target-avr/helper.c
> +++ b/target-avr/helper.c
> @@ -28,6 +28,7 @@
>  #include "exec/cpu_ldst.h"
>  #include "qemu/host-utils.h"
>  #include "exec/helper-proto.h"
> +#include "exec/ioport.h"
>  
>  bool avr_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  {
> @@ -128,7 +129,7 @@ void tlb_fill(CPUState *cs, target_ulong vaddr, int 
> is_write,
>          prot = PAGE_READ | PAGE_EXEC;
>      } else {
>  #if VIRT_BASE_REGS == 0
> -        if (vaddr < VIRT_BASE_REGS + SIZE_REGS) {
> +        if (vaddr < VIRT_BASE_REGS + AVR_REGS) {
>  #else
>          if (VIRT_BASE_REGS <= vaddr && vaddr < VIRT_BASE_REGS + SIZE_REGS) {
>  #endif
> @@ -138,7 +139,7 @@ void tlb_fill(CPUState *cs, target_ulong vaddr, int 
> is_write,
>              */
>              AVRCPU *cpu = AVR_CPU(cs);
>              CPUAVRState *env = &cpu->env;
> -            env->fullwr = 1;
> +            env->fullacc = 1;
>              cpu_loop_exit_restore(cs, retaddr);
>          } else {
>              /*
> @@ -193,6 +194,16 @@ void helper_wdr(CPUAVRState *env)
>      cpu_loop_exit(cs);
>  }
>  
> +/*
> +    This function implements IN instruction
> +
> +    It does the following
> +    a.  if an IO register belongs to CPU, its value is read and returned
> +    b.  otherwise io address is translated to mem address and physical memory
> +        is read.
> +    c.  it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
> +
> +*/
>  target_ulong helper_inb(CPUAVRState *env, uint32_t port)
>  {
>      target_ulong    data    = 0;
> @@ -236,39 +247,29 @@ target_ulong helper_inb(CPUAVRState *env, uint32_t port)
>                  device/board
>              */
>  
> -            cpu_physical_memory_read(PHYS_BASE_REGS + port + OFFS_IOREGS,
> -                                                                    &data, 
> 1);
> +            cpu_physical_memory_read(PHYS_BASE_REGS + port
> +                                              + AVR_CPU_IO_REGS_BASE, &data, 
> 1);
>          }
>      }
>  
> -    if (port < IO_REGS) {
> -        env->io[port] = data;   /*  make a copy for SBI, SBIC, SBIS & CBI   
> */
> +    /*  make a copy */
> +    if (port < AVR_CPU_IO_REGS) {
> +        env->io[port] = data;
>      }
>  
>      return  data;
>  }
>  
> -void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
> -{
> -#if VIRT_BASE_REGS == 0
> -    if (addr <= VIRT_BASE_REGS + SIZE_REGS) {
> -#else
> -    if (VIRT_BASE_REGS <= addr && addr <= VIRT_BASE_REGS + SIZE_REGS) {
> -#endif
> -        /*
> -            write CPU REGS/IO REGS/EXT IO REGS
> -        */
> -        cpu_physical_memory_write(PHYS_BASE_REGS + addr - VIRT_BASE_REGS,
> -                                                                    &data, 
> 1);
> -    } else {
> -        /*
> -            write memory
> -        */
> -        cpu_physical_memory_write(PHYS_BASE_DATA + addr - VIRT_BASE_DATA,
> -                                                                    &data, 
> 1);
> -    }
> -}
> +/*
> +    This function implements OUT instruction
> +
> +    It does the following
> +    a.  if an IO register belongs to CPU, its value is written into the 
> register
> +    b.  otherwise io address is translated to mem address and physical memory
> +        is written.
> +    c.  it caches the value for sake of SBI, SBIC, SBIS & CBI implementation
>  
> +*/
>  void helper_outb(CPUAVRState *env, uint32_t port, uint32_t data)
>  {
>      data    &= 0x000000ff;
> @@ -328,13 +329,70 @@ void helper_outb(CPUAVRState *env, uint32_t port, 
> uint32_t data)
>                  CPU does not know how to write this register, pass it to the
>                  device/board
>              */
> -            cpu_physical_memory_write(PHYS_BASE_REGS + port + OFFS_IOREGS,
> -                                                                    &data, 
> 1);
> +            cpu_physical_memory_write(PHYS_BASE_REGS + port
> +                                              + AVR_CPU_IO_REGS_BASE, &data, 
> 1);
>          }
>      }
>  
> -    if (port < IO_REGS) {
> -        env->io[port] = data;   /*  make a copy for SBI, SBIC, SBIS & CBI   
> */
> +    /*  make a copy */
> +    if (port < AVR_CPU_IO_REGS) {
> +        env->io[port] = data;
> +    }
> +}

Everything above here should be folded back to a previous patch.



> +
> +/*
> +    this function implements LD instruction when there is a posibility to 
> read
> +    from a CPU register
> +*/
> +target_ulong helper_fullrd(CPUAVRState *env, uint32_t addr)
> +{
> +    uint8_t data;
> +    switch (addr) {
> +        /*  CPU registers */
> +        case AVR_CPU_REGS_BASE ... AVR_CPU_REGS_LAST: {
> +            data = env->r[addr - AVR_CPU_REGS_BASE];
> +            break;
> +        }
> +        /*  CPU IO registers & EXT IO registers */
> +        case AVR_CPU_IO_REGS_BASE ... AVR_EXT_IO_REGS_LAST: {
> +            data = helper_inb(env, addr);
> +            break;
> +        }
> +
> +        /*  memory */
> +        default: {
> +            cpu_physical_memory_read(PHYS_BASE_DATA + addr - VIRT_BASE_DATA,
> +                                                                      &data, 
> 1);
> +        }
> +    }
> +
> +    return  data;
> +}
> +
> +/*
> +    this function implements LD instruction when there is a posibility to 
> write
> +    into a CPU register
> +*/
> +void helper_fullwr(CPUAVRState *env, uint32_t data, uint32_t addr)
> +{
> +    switch (addr) {
> +        /*  CPU registers */
> +        case AVR_CPU_REGS_BASE ... AVR_CPU_REGS_LAST: {
> +            env->r[addr - AVR_CPU_REGS_BASE] = data;
> +            break;
> +        }
> +
> +        /*  CPU IO registers & EXT IO registers */
> +        case AVR_CPU_IO_REGS_BASE ... AVR_EXT_IO_REGS_LAST: {
> +            helper_outb(env, data, addr);
> +            break;
> +        }
> +
> +        /*  memory */
> +        default: {
> +            cpu_physical_memory_write(PHYS_BASE_DATA + addr - VIRT_BASE_DATA,
> +                                                                      &data, 
> 1);
> +        }
>      }
>  }
>  
> diff --git a/target-avr/helper.h b/target-avr/helper.h
> index 19e0ab7..6036315 100644
> --- a/target-avr/helper.h
> +++ b/target-avr/helper.h
> @@ -25,3 +25,4 @@ DEF_HELPER_1(unsupported, void, env)
>  DEF_HELPER_3(outb, void, env, i32, i32)
>  DEF_HELPER_2(inb, tl, env, i32)
>  DEF_HELPER_3(fullwr, void, env, i32, i32)
> +DEF_HELPER_2(fullrd, tl, env, i32)

You messed up in which patch you wanted to add fullwr.
Either move the helpers back, or move the declaration forward.


r~