[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [XEN v7 2/2] xen/arm64: io: Support instructions (for which ISS is not valid) on emulated MMIO region using MMIO/ioreq handler
Hi Stefano, Thanks for your feedback. On 09/02/2022 01:50, Stefano Stabellini wrote: On Sat, 5 Feb 2022, Ayan Kumar Halder wrote: <snip> @@ -95,6 +97,7 @@ enum io_state try_fwd_ioserv(struct cpu_user_regs *regs, bool arch_ioreq_complete_mmio(void) { struct vcpu *v = current; + struct instr_details *dabt_instr = v->io.info.dabt_instr; struct cpu_user_regs *regs = guest_cpu_user_regs(); const union hsr hsr = { .bits = regs->hsr };I don't think that v->io.info.dabt_instr should be a pointer because at this point the original structure is long gone. The original field was a local variable mmio_info_t info in do_trap_stage2_abort_guest. So by the time arch_ioreq_complete_mmio is called, the original variable has been deallocated. Instead, v->io.info.dabt_instr should be a "struct instr_details" (no pointer). I see what you mean.arch_ioreq_complete_mmio() is called from leave_hypervisor_to_guest(). That is after do_trap_stage2_abort_guest () has been invoked. So the original variable is no longer valid. <snip> + + /* + * When the instruction needs to be retried by the guest after + * resolving the translation fault. + */ + else if ( info.dabt_instr.state == INSTR_RETRY ) + goto set_page_tables; As discussed with Julien on IRC, when hsr_dabt.s1ptw == 1, Xen should only invoke p2m_resolve_translation_fault(). If it returns an error, it should send an abort to the guest. The reason being there is no need to invoke try_map_mmio() as the gpa is not a mmio address. Also, Xen does not support the use case when the guest places the page tables in the MMIO region. I will wait for Julien's comments before sending out a v8 patch. - Ayan + + state = try_handle_mmio(regs, &info);switch ( state ){ case IO_ABORT: goto inject_abt; case IO_HANDLED: + /* + * If the instruction was decoded and has executed successfully + * on the MMIO region, then Xen should execute the next part of + * the instruction. (for eg increment the rn if it is a + * post-indexing instruction. + */ + post_increment_register(&info.dabt_instr); advance_pc(regs, hsr); return; case IO_RETRY: @@ -1985,18 +2056,12 @@ static void do_trap_stage2_abort_guest(struct cpu_user_regs *regs, } }- /*- * First check if the translation fault can be resolved by the - * P2M subsystem. If that's the case nothing else to do. - */ - if ( p2m_resolve_translation_fault(current->domain, - gaddr_to_gfn(gpa)) ) - return; - - if ( is_data && try_map_mmio(gaddr_to_gfn(gpa)) ) + set_page_tables: + if ( check_p2m(is_data, gpa) ) return;break;+ } default: gprintk(XENLOG_WARNING, "Unsupported FSC: HSR=%#"PRIregister" DFSC=%#x\n", diff --git a/xen/arch/x86/include/asm/ioreq.h b/xen/arch/x86/include/asm/ioreq.h index d06ce9a6ea..ecfe7f9fdb 100644 --- a/xen/arch/x86/include/asm/ioreq.h +++ b/xen/arch/x86/include/asm/ioreq.h @@ -26,6 +26,9 @@ #include <asm/hvm/ioreq.h> #endif+struct arch_vcpu_io {+}; + #endif /* __ASM_X86_IOREQ_H__ *//*diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 37f78cc4c4..afe5508be8 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -160,6 +160,8 @@ struct vcpu_io { /* I/O request in flight to device model. */ enum vio_completion completion; ioreq_t req; + /* Arch specific info pertaining to the io request */ + struct arch_vcpu_io info; };struct vcpu-- 2.17.1
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |