From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH v14 12/17] pvh: Use PV handlers for cpuid, and IO Date: Mon, 4 Nov 2013 16:20:42 +0000 Message-ID: <5277C95A.8060208@eu.citrix.com> References: <1383567306-6636-1-git-send-email-george.dunlap@eu.citrix.com> <1383567306-6636-13-git-send-email-george.dunlap@eu.citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1383567306-6636-13-git-send-email-george.dunlap@eu.citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: xen-devel@lists.xen.org, "Nakajima, Jun" , "Dong, Eddie" List-Id: xen-devel@lists.xenproject.org On 04/11/13 12:15, George Dunlap wrote: > For cpuid, this means putting hooks into the vmexit handler to call it instead > of the hvm one. > > For IO, this now means putting a hook into the emulation code to call the PV > guest_io_{read,write} functions. > > NB at this point this won't do the full "copy and execute on the stack > with full GPRs" work-around; this may need to be sorted out for dom0 to allow > these instructions to happen in guest context. > > Signed-off-by: George Dunlap > Signed-off-by: Mukesh Rathor > --- > v14-hvmio: > - Use HVM path to emulate IO, calling into PV just for the final IO > - Don't support forced invalid ops any more (so we can avoid PV emulation altogether) > v13: > - Remove unnecessary privilege check in PIO path, update related comment > - Move ? and : to end of line rather than beginning, as requested > > CC: Jan Beulich > CC: Tim Deegan > CC: Keir Fraser > --- > xen/arch/x86/hvm/emulate.c | 75 ++++++++++++++++++++++++++++++++++----- > xen/arch/x86/hvm/vmx/vmx.c | 3 +- > xen/arch/x86/traps.c | 6 ++-- > xen/include/asm-x86/processor.h | 2 ++ > xen/include/asm-x86/traps.h | 8 +++++ > 5 files changed, 81 insertions(+), 13 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index a41eaa1..0d767c2 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -16,14 +16,14 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > > -static void hvmtrace_io_assist(int is_mmio, ioreq_t *p) > -{ > +static void trace_io_assist(int is_mmio, int dir, int data_valid, paddr_t addr, unsigned int data) { > unsigned int size, event; > unsigned char buffer[12]; > > @@ -31,22 +31,28 @@ static void hvmtrace_io_assist(int is_mmio, ioreq_t *p) > return; > > if ( is_mmio ) > - event = p->dir ? TRC_HVM_IOMEM_READ : TRC_HVM_IOMEM_WRITE; > + event = dir ? TRC_HVM_IOMEM_READ : TRC_HVM_IOMEM_WRITE; > else > - event = p->dir ? TRC_HVM_IOPORT_READ : TRC_HVM_IOPORT_WRITE; > + event = dir ? TRC_HVM_IOPORT_READ : TRC_HVM_IOPORT_WRITE; > > - *(uint64_t *)buffer = p->addr; > - size = (p->addr != (u32)p->addr) ? 8 : 4; > + *(uint64_t *)buffer = addr; > + size = (addr != (u32)addr) ? 8 : 4; > if ( size == 8 ) > event |= TRC_64_FLAG; > > - if ( !p->data_is_ptr ) > + if ( data_valid ) > { > - *(uint32_t *)&buffer[size] = p->data; > + *(uint32_t *)&buffer[size] = data; > size += 4; > } > > trace_var(event, 0/*!cycles*/, size, buffer); > + > +} > + > +static void hvmtrace_io_assist(int is_mmio, ioreq_t *p) > +{ > + trace_io_assist(is_mmio, p->dir, !p->data_is_ptr, p->addr, p->data); > } > > static int hvmemul_do_io( > @@ -140,6 +146,9 @@ static int hvmemul_do_io( > } > } > > + if ( is_pvh_vcpu(curr) ) > + ASSERT(vio->io_state == HVMIO_none); > + > switch ( vio->io_state ) > { > case HVMIO_none: > @@ -284,11 +293,59 @@ static int hvmemul_do_io( > return X86EMUL_OKAY; > } > > +static int pvhemul_do_pio( > + unsigned long port, int size, paddr_t ram_gpa, int dir, void *p_data) > +{ > + paddr_t value = ram_gpa; > + struct vcpu *curr = current; > + struct cpu_user_regs *regs = guest_cpu_user_regs(); > + > + /* > + * Weird-sized accesses have undefined behaviour: we discard writes > + * and read all-ones. > + */ > + if ( unlikely((size > sizeof(long)) || (size & (size - 1))) ) > + { > + gdprintk(XENLOG_WARNING, "bad mmio size %d\n", size); > + ASSERT(p_data != NULL); /* cannot happen with a REP prefix */ > + if ( dir == IOREQ_READ ) > + memset(p_data, ~0, size); > + return X86EMUL_UNHANDLEABLE; > + } > + > + if ( dir == IOREQ_WRITE ) { > + if ( (p_data != NULL) ) > + { > + memcpy(&value, p_data, size); > + p_data = NULL; > + } > + > + if ( dir == IOREQ_WRITE ) > + trace_io_assist(0, dir, 1, port, value); > + > + guest_io_write(port, size, value, curr, regs); > + } > + else > + { > + value = guest_io_read(port, size, curr, regs); > + trace_io_assist(0, dir, 1, port, value); > + if ( (p_data != NULL) ) > + memcpy(p_data, &value, size); > + memcpy(®s->eax, &value, size); > + } > + > + return X86EMUL_OKAY; > +} > + > + > int hvmemul_do_pio( > unsigned long port, unsigned long *reps, int size, > paddr_t ram_gpa, int dir, int df, void *p_data) > { > - return hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data); > + return is_hvm_vcpu(current) ? > + hvmemul_do_io(0, port, reps, size, ram_gpa, dir, df, p_data) : > + pvhemul_do_pio(port, size, ram_gpa, dir, p_data); > + > } > > static int hvmemul_do_mmio( > diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c > index 94e9e21..5d1e367 100644 > --- a/xen/arch/x86/hvm/vmx/vmx.c > +++ b/xen/arch/x86/hvm/vmx/vmx.c > @@ -56,6 +56,7 @@ > #include > #include > #include > +#include > > enum handler_return { HNDL_done, HNDL_unhandled, HNDL_exception_raised }; > > @@ -2694,8 +2695,8 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs) > break; > } > case EXIT_REASON_CPUID: > + is_pvh_vcpu(v) ? pv_cpuid(regs) : vmx_do_cpuid(regs); > update_guest_eip(); /* Safe: CPUID */ > - vmx_do_cpuid(regs); > break; > case EXIT_REASON_HLT: > update_guest_eip(); /* Safe: HLT */ > diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c > index edb7a6a..6c278bc 100644 > --- a/xen/arch/x86/traps.c > +++ b/xen/arch/x86/traps.c > @@ -729,7 +729,7 @@ int cpuid_hypervisor_leaves( uint32_t idx, uint32_t sub_idx, > return 1; > } > > -static void pv_cpuid(struct cpu_user_regs *regs) > +void pv_cpuid(struct cpu_user_regs *regs) > { > uint32_t a, b, c, d; > > @@ -1681,7 +1681,7 @@ static int pci_cfg_ok(struct domain *d, int write, int size) > return 1; > } > > -static uint32_t guest_io_read( > +uint32_t guest_io_read( > unsigned int port, unsigned int bytes, > struct vcpu *v, struct cpu_user_regs *regs) > { > @@ -1748,7 +1748,7 @@ static uint32_t guest_io_read( > return data; > } > > -static void guest_io_write( > +void guest_io_write( > unsigned int port, unsigned int bytes, uint32_t data, > struct vcpu *v, struct cpu_user_regs *regs) > { > diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h > index 893afa3..551036d 100644 > --- a/xen/include/asm-x86/processor.h > +++ b/xen/include/asm-x86/processor.h > @@ -567,6 +567,8 @@ void microcode_set_module(unsigned int); > int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len); > int microcode_resume_cpu(int cpu); > > +void pv_cpuid(struct cpu_user_regs *regs); > + > #endif /* !__ASSEMBLY__ */ > > #endif /* __ASM_X86_PROCESSOR_H */ > diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h > index 82cbcee..a26b318 100644 > --- a/xen/include/asm-x86/traps.h > +++ b/xen/include/asm-x86/traps.h > @@ -49,4 +49,12 @@ extern int guest_has_trap_callback(struct domain *d, uint16_t vcpuid, > extern int send_guest_trap(struct domain *d, uint16_t vcpuid, > unsigned int trap_nr); > > +uint32_t guest_io_read( > + unsigned int port, unsigned int bytes, > + struct vcpu *v, struct cpu_user_regs *regs); > +void guest_io_write( > + unsigned int port, unsigned int bytes, uint32_t data, > + struct vcpu *v, struct cpu_user_regs *regs); > + > + > #endif /* ASM_TRAP_H */