[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 05/21] RISC-V CPU Helpers
From: |
Christoph Hellwig |
Subject: |
Re: [Qemu-devel] [PATCH v2 05/21] RISC-V CPU Helpers |
Date: |
Thu, 11 Jan 2018 09:08:38 +0100 |
User-agent: |
Mutt/1.5.17 (2007-11-01) |
#ifdef CONFIG_USER_ONLY
int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
{
return 0;
}
bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
{
return false;
}
int riscv_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
int access_type, int mmu_idx)
{
cs->exception_index = QEMU_USER_EXCP_FAULT;
return TRANSLATE_FAIL;
}
void riscv_cpu_do_interrupt(CPUState *cs)
{
cs->exception_index = EXCP_NONE; /* mark handled to qemu */
}
#else
... real implementation here ...
#endif
To keep the code flow straight?
> +int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
> +{
> +#ifdef CONFIG_USER_ONLY
> + return 0;
> +#else
Can you do a large section of USER ONLY stubs instead of sprinkling
the ifdefs? E.g.
> +static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> + int *prot, target_ulong address,
> + int access_type, int mmu_idx)
> +{
> + /* NOTE: the env->pc value visible here will not be
> + * correct, but the value visible to the exception handler
> + * (riscv_cpu_do_interrupt) is correct */
> +
> + const int mode = mmu_idx;
Why don't you just name the actual argument mode and mark it const
if really needed?
> +
> + *prot = 0;
> + CPUState *cs = CPU(riscv_env_get_cpu(env));
Clearing out prot just before declaring cs looks a little odd. Especially
the prot clearing can easily be moved past the next branch.
> + if (access_type == MMU_INST_FETCH) { /* inst access */
> + exception = page_fault_exceptions ?
> + RISCV_EXCP_INST_PAGE_FAULT : RISCV_EXCP_INST_ACCESS_FAULT;
> + env->badaddr = address;
> + } else if (access_type == MMU_DATA_STORE) { /* store access */
> + exception = page_fault_exceptions ?
> + RISCV_EXCP_STORE_PAGE_FAULT : RISCV_EXCP_STORE_AMO_ACCESS_FAULT;
> + env->badaddr = address;
> + } else if (access_type == MMU_DATA_LOAD) { /* load access */
> + exception = page_fault_exceptions ?
> + RISCV_EXCP_LOAD_PAGE_FAULT : RISCV_EXCP_LOAD_ACCESS_FAULT;
> + env->badaddr = address;
> + } else {
> + g_assert_not_reached();
Why isn't this a switch statement?
> + if (access_type == MMU_INST_FETCH) {
> + cs->exception_index = RISCV_EXCP_INST_ADDR_MIS;
> + env->badaddr = addr;
> + } else if (access_type == MMU_DATA_STORE) {
> + cs->exception_index = RISCV_EXCP_STORE_AMO_ADDR_MIS;
> + env->badaddr = addr;
> + } else if (access_type == MMU_DATA_LOAD) {
> + cs->exception_index = RISCV_EXCP_LOAD_ADDR_MIS;
> + env->badaddr = addr;
> + } else {
> + g_assert_not_reached();
> + }
Same here.
> + case CSR_SPTBR:
And just as comment on V2: please don't use CSR_SPTBR here, but
CSR_SAPT, and don't even define the CSR_SPTBR cpp symbol.
- [Qemu-devel] [PATCH v2 00/21] RISC-V QEMU Port Submission v2, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 01/21] RISC-V Maintainers, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 02/21] RISC-V ELF Machine Definition, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 07/21] RISC-V GDB Stub, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 06/21] RISC-V FPU Support, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 03/21] RISC-V CPU Core Definition, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 12/21] RISC-V HART Array, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 05/21] RISC-V CPU Helpers, Michael Clark, 2018/01/10
- Re: [Qemu-devel] [PATCH v2 05/21] RISC-V CPU Helpers,
Christoph Hellwig <=
- [Qemu-devel] [PATCH v2 09/21] RISC-V Physical Memory Protection, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 13/21] SiFive RISC-V CLINT Block, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 11/21] RISC-V HTIF Console, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 18/21] SiFive RISC-V PRCI Block, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 17/21] SiFive RISC-V UART Device, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 14/21] SiFive RISC-V PLIC Block, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 21/21] RISC-V Build Infrastructure, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 19/21] SiFive Freedom E300 RISC-V Machine, Michael Clark, 2018/01/10
- [Qemu-devel] [PATCH v2 15/21] RISC-V Spike Machines, Michael Clark, 2018/01/10