Re: [Qemu-devel] [PATCH 02/15] Openrisc: add MMU support

[Jia Liu]

Hi Blue,

On Sat, May 19, 2012 at 3:41 PM, Blue Swirl <address@hidden> wrote:
> On Thu, May 17, 2012 at 8:35 AM, Jia Liu <address@hidden> wrote:
>> add the openrisc MMU support.
>>
>> Signed-off-by: Jia Liu <address@hidden>
>> ---
>>  Makefile.target              |    2 +
>>  hw/openrisc_cpudev.h         |   30 ++++++
>>  hw/openrisc_pic.c            |   31 ++++++
>>  target-openrisc/mem.c        |  220 
>> +++++++++++++++++++++++++++++++++++++++++-
>>  target-openrisc/mem_helper.c |   25 +++++
>>  5 files changed, 307 insertions(+), 1 deletion(-)
>>  create mode 100644 hw/openrisc_cpudev.h
>>  create mode 100644 hw/openrisc_pic.c
>>
>> diff --git a/Makefile.target b/Makefile.target
>> index 0ffb29c..79c75f6 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -390,6 +390,8 @@ obj-xtensa-y += core-dc232b.o
>>  obj-xtensa-y += core-dc233c.o
>>  obj-xtensa-y += core-fsf.o
>>
>> +obj-openrisc-y += openrisc_pic.o
>> +
>>  main.o: QEMU_CFLAGS+=$(GPROF_CFLAGS)
>>
>>  monitor.o: hmp-commands.h qmp-commands-old.h
>> diff --git a/hw/openrisc_cpudev.h b/hw/openrisc_cpudev.h
>> new file mode 100644
>> index 0000000..ca7d064
>> --- /dev/null
>> +++ b/hw/openrisc_cpudev.h
>> @@ -0,0 +1,30 @@
>> +/*
>> + * Qemu Openrisc CPU device support.
>> + *
>> + *  Copyright (c) 2011-2012 Jia Liu <address@hidden>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  02110-1301 
>> USA
>> + */
>> +
>> +#ifndef HW_OPENRISC_CPUDEV_H
>> +#define HW_OPENRISC_CPUDEV_H
>> +
>> +/* openrisc_pic.c */
>> +void cpu_openrisc_pic_init(CPUOPENRISCState *env);
>> +
>> +/* openrisc_timer.c*/
>> +void cpu_openrisc_clock_init(CPUOPENRISCState *env);
>
> The implementation is not added by this patch.
>

I'll add it here.

>> +
>> +#endif
>> diff --git a/hw/openrisc_pic.c b/hw/openrisc_pic.c
>> new file mode 100644
>> index 0000000..0abdd50
>> --- /dev/null
>> +++ b/hw/openrisc_pic.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + * Generic  OPENRISC Programmable Interrupt Controller support.
>> + *
>> + *  Copyright (c) 2011-2012 Jia Liu <address@hidden>
>> + *                          Feng Gao <address@hidden>
>> + *
>> + * This library is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU Library General Public
>> + * License as published by the Free Software Foundation; either
>> + * version 2 of the License, or (at your option) any later version.
>> + *
>> + * This library is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + * Library General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU Library General Public
>> + * License along with this library; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston MA  02110-1301 
>> USA
>> + */
>> +
>> +#include "hw.h"
>> +#include "openrisc_cpudev.h"
>> +#include "cpu.h"
>> +
>> +/* Reset PIC */
>> +void cpu_openrisc_pic_reset(CPUOPENRISCState *env)
>> +{
>> +    env->picmr = 0x00000000;
>> +    env->picsr = 0x00000000;
>> +}
>> diff --git a/target-openrisc/mem.c b/target-openrisc/mem.c
>> index 57a0b1b..b26da38 100644
>> --- a/target-openrisc/mem.c
>> +++ b/target-openrisc/mem.c
>> @@ -28,10 +28,224 @@
>>  #endif
>>
>>  #if !defined(CONFIG_USER_ONLY)
>> +enum {
>> +    TLBRET_INVALID = -3,
>> +    TLBRET_NOMATCH = -2,
>> +    TLBRET_BADADDR = -1,
>> +    TLBRET_MATCH = 0
>> +};
>> +
>> +tlb_entry itlb_table[ITLB_WAYS][ITLB_SIZE];
>> +tlb_entry dtlb_table[DTLB_WAYS][DTLB_SIZE];
>
> These should be inside CPUOPENRISCState, not globals.
>

thanks, I'll move them there.

>> +#endif
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>> +/* no MMU emulation */
>> +int get_phys_nommu(CPUOPENRISCState *env, target_phys_addr_t *physical,
>> +                   int *prot, target_ulong address, int rw)
>> +{
>> +    *physical = address;
>> +    *prot = PAGE_READ | PAGE_WRITE;
>> +    return TLBRET_MATCH;
>> +}
>> +int get_phys_code(CPUOPENRISCState *env, target_phys_addr_t *physical,
>> +                  int *prot, target_ulong address, int rw)
>> +{
>> +    int vpn = address >> TARGET_PAGE_BITS;
>> +    int idx = vpn & ITLB_MASK;
>> +    int right = 0;
>> +
>> +    if ((env->itlb[0][idx].mr >> TARGET_PAGE_BITS) != vpn) {
>> +        return TLBRET_NOMATCH;
>> +    }
>> +    if (!(env->itlb[0][idx].mr & 1)) {
>> +        return TLBRET_INVALID;
>> +    }
>> +
>> +    if (env->sr & SR_SM) { /* supervisor mode */
>> +        if (env->itlb[0][idx].tr & SXE) {
>> +            right |= PAGE_EXEC;
>> +        }
>> +    } else {
>> +        if (env->itlb[0][idx].tr & UXE) {
>> +            right |= PAGE_EXEC;
>> +        }
>> +    }
>> +
>> +    if ((rw & 2) && ((right & PAGE_EXEC) == 0)) {
>> +        return TLBRET_BADADDR;
>> +    }
>> +
>> +    *physical = (env->itlb[0][idx].tr & TARGET_PAGE_MASK) |
>> +                (address & (TARGET_PAGE_SIZE-1));
>> +    *prot = right;
>> +    return TLBRET_MATCH;
>> +}
>> +
>> +int get_phys_data(CPUOPENRISCState *env, target_phys_addr_t *physical,
>> +                  int *prot, target_ulong address, int rw)
>> +{
>> +    int vpn = address >> TARGET_PAGE_BITS;
>> +    int idx = vpn & DTLB_MASK;
>> +    int right = 0;
>> +
>> +    if ((env->dtlb[0][idx].mr >> TARGET_PAGE_BITS) != vpn) {
>> +        return TLBRET_NOMATCH;
>> +    }
>> +    if (!(env->dtlb[0][idx].mr & 1)) {
>> +        return TLBRET_INVALID;
>> +    }
>> +
>> +    if (env->sr & SR_SM) { /* supervisor mode */
>> +        if (env->dtlb[0][idx].tr & SRE) {
>> +            right |= PAGE_READ;
>> +        }
>> +        if (env->dtlb[0][idx].tr & SWE) {
>> +            right |= PAGE_WRITE;
>> +        }
>> +    } else {
>> +        if (env->dtlb[0][idx].tr & URE) {
>> +            right |= PAGE_READ;
>> +        }
>> +        if (env->dtlb[0][idx].tr & UWE) {
>> +            right |= PAGE_WRITE;
>> +        }
>> +    }
>> +
>> +    if ((rw & 0) && ((right & PAGE_READ) == 0)) {
>> +        return TLBRET_BADADDR;
>> +    }
>> +    if ((rw & 1) && ((right & PAGE_WRITE) == 0)) {
>> +        return TLBRET_BADADDR;
>> +    }
>> +
>> +    *physical = (env->dtlb[0][idx].tr & TARGET_PAGE_MASK) |
>> +                (address & (TARGET_PAGE_SIZE-1));
>> +    *prot = right;
>> +    return TLBRET_MATCH;
>> +}
>> +
>> +static int get_physical_address(CPUOPENRISCState *env,
>> +                                target_phys_addr_t *physical,
>> +                                int *prot, target_ulong address,
>> +                                int rw)
>> +{
>> +    int ret = TLBRET_MATCH;
>> +
>> +    /* [0x0000--0x2000]: unmapped */
>> +    if (address < 0x2000 && (env->sr & SR_SM)) {
>
> This is pretty strange, isn't the virtual to physical mapping handled
> by TLB so why this shortcut?
>

The Openrisc Arch said that, so I have to obey.

>> +        *physical = address;
>> +        *prot = PAGE_READ | PAGE_WRITE;
>> +        return ret;
>> +    }
>> +
>> +    if (rw == 2) { /* ITLB */
>> +       *physical = 0;
>> +        ret = env->map_address_code(env, physical,
>> +                                    prot, address, rw);
>> +    } else {       /* DTLB */
>> +        ret = env->map_address_data(env, physical,
>> +                                    prot, address, rw);
>> +    }
>> +
>> +    return ret;
>> +}
>> +#endif
>> +
>> +static void raise_mmu_exception(CPUOPENRISCState *env, target_ulong address,
>> +                                int rw, int tlb_error)
>> +{
>> +    int exception = 0;
>> +
>> +    switch (tlb_error) {
>> +    default:
>> +        if (rw == 2) {
>> +            exception = EXCP_IPF;
>> +        } else {
>> +            exception = EXCP_DPF;
>> +        }
>> +        break;
>> +#if !defined(CONFIG_USER_ONLY)
>> +    case TLBRET_BADADDR:
>> +        if (rw == 2) {
>> +            exception = EXCP_IPF;
>> +        } else {
>> +            exception = EXCP_DPF;
>> +        }
>> +        break;
>> +    case TLBRET_INVALID:
>> +    case TLBRET_NOMATCH:
>> +        /* No TLB match for a mapped address */
>> +        if (rw == 2) {
>> +            exception = EXCP_ITLBMISS;
>> +        } else {
>> +            exception = EXCP_DTLBMISS;
>> +        }
>> +        break;
>> +#endif
>> +    }
>> +
>> +    env->exception_index = exception;
>> +    env->eear = address;
>> +}
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>> +int cpu_openrisc_handle_mmu_fault(CPUOPENRISCState *env,
>> +                                  target_ulong address, int rw, int mmu_idx)
>> +{
>> +    int ret = 0;
>> +    target_phys_addr_t physical = 0;
>> +    int prot = 0;
>> +
>> +    ret = get_physical_address(env, &physical, &prot,
>> +                               address, rw);
>> +
>> +    if (ret == TLBRET_MATCH) {
>> +        tlb_set_page(env, address & TARGET_PAGE_MASK,
>> +                     physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
>> +                     mmu_idx, TARGET_PAGE_SIZE);
>> +        ret = 0;
>> +    } else if (ret < 0) {
>> +        raise_mmu_exception(env, address, rw, ret);
>> +        ret = 1;
>> +    }
>> +
>> +    return ret;
>> +}
>> +#else
>> +int cpu_openrisc_handle_mmu_fault(CPUOPENRISCState *env,
>> +                                  target_ulong address, int rw, int mmu_idx)
>> +{
>> +    int ret = 0;
>> +
>> +    raise_mmu_exception(env, address, rw, ret);
>> +    ret = 1;
>> +
>> +    return ret;
>> +}
>> +#endif
>> +
>> +#if !defined(CONFIG_USER_ONLY)
>>  target_phys_addr_t cpu_get_phys_page_debug(CPUOPENRISCState *env,
>>                                            target_ulong addr)
>>  {
>> -    return 0;
>> +    target_phys_addr_t phys_addr;
>> +    int prot;
>> +
>> +    if (get_physical_address(env, &phys_addr, &prot, addr, 0)) {
>> +        return -1;
>> +    }
>> +    return phys_addr;
>> +}
>> +
>> +void openrisc_mmu_init(CPUOPENRISCState *env)
>> +{
>> +    env->map_address_code = &get_phys_nommu;
>> +    env->map_address_data = &get_phys_nommu;
>> +    env->dtlb = dtlb_table;
>> +    env->itlb = itlb_table;
>> +    memset(dtlb_table, 0, sizeof(tlb_entry)*DTLB_SIZE*DTLB_WAYS);
>> +    memset(itlb_table, 0, sizeof(tlb_entry)*ITLB_SIZE*ITLB_WAYS);
>>  }
>>  #endif
>>
>> @@ -40,4 +254,8 @@ void openrisc_reset(CPUOPENRISCState *env)
>>     env->pc = 0x100;
>>     env->sr = SR_FO | SR_SM;
>>     env->exception_index = -1;
>> +#if !defined(CONFIG_USER_ONLY)
>> +    openrisc_mmu_init(env);
>> +    cpu_openrisc_pic_reset(env);
>> +#endif
>>  }
>> diff --git a/target-openrisc/mem_helper.c b/target-openrisc/mem_helper.c
>> index a2d93c7..922dd6b 100644
>> --- a/target-openrisc/mem_helper.c
>> +++ b/target-openrisc/mem_helper.c
>> @@ -42,5 +42,30 @@
>>  void tlb_fill(CPUOPENRISCState *env1, target_ulong addr, int is_write,
>>               int mmu_idx, uintptr_t retaddr)
>>  {
>> +    TranslationBlock *tb;
>> +    CPUOPENRISCState *saved_env;
>> +    unsigned long pc;
>> +    int ret;
>> +
>> +    saved_env = env;
>> +    env = env1;
>
> Please avoid compiling with AREG0 and remove this.
>

Sorry, I'm not very sure about it, may you give me more hits?

>> +
>> +    ret = cpu_openrisc_handle_mmu_fault(env, addr, is_write, mmu_idx);
>> +
>> +    if (ret) {
>> +        if (retaddr) {
>> +            /* now we have a real cpu fault */
>> +            pc = (unsigned long)retaddr;
>> +            tb = tb_find_pc(pc);
>> +            if (tb) {
>> +                /* the PC is inside the translated code. It means that we
>> +                   have a virtual CPU fault */
>> +                cpu_restore_state(tb, env, pc);
>> +            }
>> +        }
>> +        /* Raise Exception... */
>> +        cpu_loop_exit(env);
>> +    }
>> +    env = saved_env;
>>  }
>>  #endif
>> --
>> 1.7.9.5
>>
>>