> >> In addition, the various V-extension vector load/store instructions do not >> have >> defined transformations, so they should show up in [m|h]tinst as all zeros. > Okay, I will update. Just a clarification/suggestion: It might be easier to only write non-zero for instructions with currently defined transformation. Writing zero is always legal, but writing an undefined transformed instruction is not. >>> @@ -1355,18 +1558,31 @@ void riscv_cpu_do_interrupt(CPUState *cs) >>> if (!async) { >>> /* set tval to badaddr for traps with address information */ >>> switch (cause) { >>> - case RISCV_EXCP_INST_GUEST_PAGE_FAULT: >>> case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT: >>> case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT: >>> - case RISCV_EXCP_INST_ADDR_MIS: >>> - case RISCV_EXCP_INST_ACCESS_FAULT: >>> case RISCV_EXCP_LOAD_ADDR_MIS: >>> case RISCV_EXCP_STORE_AMO_ADDR_MIS: >>> case RISCV_EXCP_LOAD_ACCESS_FAULT: >>> case RISCV_EXCP_STORE_AMO_ACCESS_FAULT: >>> - case RISCV_EXCP_INST_PAGE_FAULT: >>> case RISCV_EXCP_LOAD_PAGE_FAULT: >>> case RISCV_EXCP_STORE_PAGE_FAULT: >>> + write_gva = env->two_stage_lookup; >>> + tval = env->badaddr; >>> + if (env->two_stage_indirect_lookup) { >>> + /* >>> + * special pseudoinstruction for G-stage fault taken while >>> + * doing VS-stage page table walk. >>> + */ >>> + tinst = (riscv_cpu_xlen(env) == 32) ? 0x00002000 : >>> 0x00003000; >>> + } else { >>> + /* transformed instruction for all other load/store faults >>> */ >>> + tinst = riscv_transformed_insn(env, env->bins); >>> + } >>> + break; >>> + case RISCV_EXCP_INST_GUEST_PAGE_FAULT: >>> + case RISCV_EXCP_INST_ADDR_MIS: >>> + case RISCV_EXCP_INST_ACCESS_FAULT: >>> + case RISCV_EXCP_INST_PAGE_FAULT: >>> write_gva = env->two_stage_lookup; >>> tval = env->badaddr; >>> break; >> Instruction guest-page faults should set [m|h]tinst to one of the >> pseudoinstructions if env->two_stage_lookup is true. Otherwise it should set >> [m|h]tinst to zero. >> >> In any case, as this seems to be one of the first implementations of >> [m|h]tinst, it might be worthwhile to confirm with the spec authors and >> clarify >> any unclear bits before this gets released. > This is already handled because tinst is initialized to zero.
If an instruction guest-page fault occurs due to a G-stage fault while doing VS-stage page table walk, i.e. env->two_stage_indirect_lookup is true (I had mistakenly wrote env->two_stage_lookup earlier), and the faulting guest physical address (env->guest_phys_fault_addr) is written to mtval2/htval, [m|h]tinst must be a pseudoinstruction and not zero. This case is not handled in the v5 patch. dramforever