On Fri, Mar 10, 2023 at 10:57 PM Dmitry Isaykin <[email protected]> wrote: > > Adds monitor support for I/O instructions. > > Signed-off-by: Dmitry Isaykin <[email protected]> > Signed-off-by: Anton Belousov <[email protected]> > --- > tools/include/xenctrl.h | 1 + > tools/libs/ctrl/xc_monitor.c | 13 +++++++++++++ > xen/arch/x86/hvm/hvm.c | 5 +++++ > xen/arch/x86/hvm/monitor.c | 21 +++++++++++++++++++++ > xen/arch/x86/hvm/vmx/vmx.c | 2 ++ > xen/arch/x86/include/asm/domain.h | 1 + > xen/arch/x86/include/asm/hvm/monitor.h | 3 +++ > xen/arch/x86/include/asm/hvm/support.h | 3 +++ > xen/arch/x86/include/asm/monitor.h | 3 ++- > xen/arch/x86/monitor.c | 13 +++++++++++++ > xen/include/public/domctl.h | 1 + > xen/include/public/vm_event.h | 10 ++++++++++ > 12 files changed, 75 insertions(+), 1 deletion(-) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 23037874d3..05967ecc92 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2102,6 +2102,7 @@ int xc_monitor_emul_unimplemented(xc_interface *xch, uint32_t domain_id, > bool enable); > int xc_monitor_vmexit(xc_interface *xch, uint32_t domain_id, bool enable, > bool sync); > +int xc_monitor_io(xc_interface *xch, uint32_t domain_id, bool enable); > /** > * This function enables / disables emulation for each REP for a > * REP-compatible instruction. > diff --git a/tools/libs/ctrl/xc_monitor.c b/tools/libs/ctrl/xc_monitor.c > index c5fa62ff30..3cb96f444f 100644 > --- a/tools/libs/ctrl/xc_monitor.c > +++ b/tools/libs/ctrl/xc_monitor.c > @@ -261,6 +261,19 @@ int xc_monitor_vmexit(xc_interface *xch, uint32_t domain_id, bool enable, > return do_domctl(xch, &domctl); > } > > +int xc_monitor_io(xc_interface *xch, uint32_t domain_id, bool enable) > +{ > + DECLARE_DOMCTL; > + > + domctl.cmd = XEN_DOMCTL_monitor_op; > + domctl.domain = domain_id; > + domctl.u.monitor_op.op = enable ? XEN_DOMCTL_MONITOR_OP_ENABLE > + : XEN_DOMCTL_MONITOR_OP_DISABLE; > + domctl.u.monitor_op.event = XEN_DOMCTL_MONITOR_EVENT_IO; > + > + return do_domctl(xch, &domctl); > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 0c81e2afc7..72c9f65626 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -3484,6 +3484,11 @@ int hvm_vmexit_cpuid(struct cpu_user_regs *regs, unsigned int inst_len) > return hvm_monitor_cpuid(inst_len, leaf, subleaf); > } > > +void hvm_io_instruction_intercept(uint16_t port, int dir, unsigned int bytes, unsigned int string_ins) > +{ > + hvm_monitor_io_instruction(port, dir, bytes, string_ins); > +} > + > void hvm_rdtsc_intercept(struct cpu_user_regs *regs) > { > msr_split(regs, hvm_get_guest_tsc(current)); > diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c > index a11cd76f4d..f8b11d1de9 100644 > --- a/xen/arch/x86/hvm/monitor.c > +++ b/xen/arch/x86/hvm/monitor.c > @@ -233,6 +233,27 @@ int hvm_monitor_cpuid(unsigned long insn_length, unsigned int leaf, > return monitor_traps(curr, 1, &req); > } > > +void hvm_monitor_io_instruction(uint16_t port, int dir, > + unsigned int bytes, unsigned int string_ins) > +{ > + struct vcpu *curr = current; > + struct arch_domain *ad = &curr->domain->arch; > + vm_event_request_t req = {}; > + > + if ( !ad->monitor.io_enabled ) > + return; > + > + req.reason = VM_EVENT_REASON_IO_INSTRUCTION; > + req.u.io_instruction.data_size = bytes; > + req.u.io_instruction.port = port; > + req.u.io_instruction.dir = dir; > + req.u.io_instruction.string_ins = string_ins; > + > + set_npt_base(curr, &req); > + > + monitor_traps(curr, true, &req); > +} > + > void hvm_monitor_interrupt(unsigned int vector, unsigned int type, > unsigned int err, uint64_t cr2) > { > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 278b829f73..a64c5078c5 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -4579,6 +4579,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > uint16_t port = (exit_qualification >> 16) & 0xFFFF; > int bytes = (exit_qualification & 0x07) + 1; > int dir = (exit_qualification & 0x08) ? IOREQ_READ : IOREQ_WRITE; > + int str_ins = (exit_qualification & 0x10) ? 1 : 0;
You are already in a branch here where str_ins is checked and known to be 1. > + hvm_io_instruction_intercept(port, dir, bytes, str_ins); IMHO you should have this intercept be called outside the if-else. The function already kind-of indicates str_ins is an input yet right now only called when it's 1. The rest of the plumbing in the patch LGTM. Thanks, Tamas
