On 10/9/20 2:57 AM, Yifei Jiang wrote:
> #define TRANSLATE_FAIL 1
> #define TRANSLATE_SUCCESS 0
> #define MMU_USER_IDX 3
> +#define TRANSLATE_G_STAGE_FAIL 4
Note that you're interleaving TRANSLATE_* around an unrelated define. Perhaps
rearrange to
enum {
TRANSLATE_SUCCESS = 0,
TRANSLATE_FAIL,
TRANSLATE_PMP_FAIL,
TRANSLATE_G_STAGE_FAIL,
};
> +++ b/target/riscv/cpu_helper.c
> @@ -451,7 +451,10 @@ restart:
> mmu_idx, false, true);
>
> if (vbase_ret != TRANSLATE_SUCCESS) {
> - return vbase_ret;
> + env->guest_phys_fault_addr = (base |
> + (addr &
> + (TARGET_PAGE_SIZE - 1))) >> 2;
> + return TRANSLATE_G_STAGE_FAIL;
> }
I don't think you can make this change to cpu state, as this function is also
used by gdb. I think you'll need to add a new (target_ulong *) parameter to
get_physical_address to return this.
The usage in riscv_cpu_tlb_fill could pass &env->guest_phys_fault_addr, and the
usage in riscv_cpu_get_phys_page_debug could pass the address of a local
variable (which it then ignores).
Also, isn't the offset more naturally written idx * ptesize, as seen just a few
lines below?
> + if (ret != TRANSLATE_FAIL && ret != TRANSLATE_G_STAGE_FAIL) {
Should this not be ret == TRANSLATE_SUCCESS?
This looks buggy with TRANSLATE_PMP_FAIL...
r~
