On Fri, Jun 10, 2022 at 5:20 PM dramforever <dramfore...@live.com> wrote:
>
> >
> >> 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.
The v5 patch is writing pseudoinstruction in [m|h]tinst when
env->two_stage_indirect_lookup is true.
Regards,
Anup
