[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v4 2/8] ioreq-server: centralize access to ioreq structures
On Wed, Apr 2, 2014 at 4:11 PM, Paul Durrant <paul.durrant@xxxxxxxxxx> wrote: > To simplify creation of the ioreq server abstraction in a subsequent patch, > this patch centralizes all use of the shared ioreq structure and the > buffered ioreq ring to the source module xen/arch/x86/hvm/hvm.c. > > The patch moves an rmb() from inside hvm_io_assist() to hvm_do_resume() > because the former may now be passed a data structure on stack, in which > case the barrier is unnecessary. > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> > Cc: Keir Fraser <keir@xxxxxxx> > Cc: Jan Beulich <jbeulich@xxxxxxxx> > Cc: Jun Nakajima <jun.nakajima@xxxxxxxxx> > Cc: Eddie Dong <eddie.dong@xxxxxxxxx> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx> Reviewed-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx> > --- > xen/arch/x86/hvm/emulate.c | 70 +++++++++------------- > xen/arch/x86/hvm/hvm.c | 118 > +++++++++++++++++++++++++++++++++++-- > xen/arch/x86/hvm/io.c | 104 +++----------------------------- > xen/arch/x86/hvm/vmx/vvmx.c | 13 +++- > xen/include/asm-x86/hvm/hvm.h | 15 ++++- > xen/include/asm-x86/hvm/support.h | 21 ++++--- > 6 files changed, 185 insertions(+), 156 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index 868aa1d..1c71902 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -57,24 +57,11 @@ static int hvmemul_do_io( > int value_is_ptr = (p_data == NULL); > struct vcpu *curr = current; > struct hvm_vcpu_io *vio; > - ioreq_t *p = get_ioreq(curr); > - ioreq_t _ioreq; > + ioreq_t p; > unsigned long ram_gfn = paddr_to_pfn(ram_gpa); > p2m_type_t p2mt; > struct page_info *ram_page; > int rc; > - bool_t has_dm = 1; > - > - /* > - * Domains without a backing DM, don't have an ioreq page. Just > - * point to a struct on the stack, initialising the state as needed. > - */ > - if ( !p ) > - { > - has_dm = 0; > - p = &_ioreq; > - p->state = STATE_IOREQ_NONE; > - } > > /* Check for paged out page */ > ram_page = get_page_from_gfn(curr->domain, ram_gfn, &p2mt, P2M_UNSHARE); > @@ -173,15 +160,6 @@ static int hvmemul_do_io( > return X86EMUL_UNHANDLEABLE; > } > > - if ( p->state != STATE_IOREQ_NONE ) > - { > - gdprintk(XENLOG_WARNING, "WARNING: io already pending (%d)?\n", > - p->state); > - if ( ram_page ) > - put_page(ram_page); > - return X86EMUL_UNHANDLEABLE; > - } > - > vio->io_state = > (p_data == NULL) ? HVMIO_dispatched : HVMIO_awaiting_completion; > vio->io_size = size; > @@ -193,38 +171,38 @@ static int hvmemul_do_io( > if ( vio->mmio_retrying ) > *reps = 1; > > - p->dir = dir; > - p->data_is_ptr = value_is_ptr; > - p->type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO; > - p->size = size; > - p->addr = addr; > - p->count = *reps; > - p->df = df; > - p->data = value; > + p.dir = dir; > + p.data_is_ptr = value_is_ptr; > + p.type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO; > + p.size = size; > + p.addr = addr; > + p.count = *reps; > + p.df = df; > + p.data = value; > > if ( dir == IOREQ_WRITE ) > - hvmtrace_io_assist(is_mmio, p); > + hvmtrace_io_assist(is_mmio, &p); > > if ( is_mmio ) > { > - rc = hvm_mmio_intercept(p); > + rc = hvm_mmio_intercept(&p); > if ( rc == X86EMUL_UNHANDLEABLE ) > - rc = hvm_buffered_io_intercept(p); > + rc = hvm_buffered_io_intercept(&p); > } > else > { > - rc = hvm_portio_intercept(p); > + rc = hvm_portio_intercept(&p); > } > > switch ( rc ) > { > case X86EMUL_OKAY: > case X86EMUL_RETRY: > - *reps = p->count; > - p->state = STATE_IORESP_READY; > + *reps = p.count; > + p.state = STATE_IORESP_READY; > if ( !vio->mmio_retry ) > { > - hvm_io_assist(p); > + hvm_io_assist(&p); > vio->io_state = HVMIO_none; > } > else > @@ -233,7 +211,7 @@ static int hvmemul_do_io( > break; > case X86EMUL_UNHANDLEABLE: > /* If there is no backing DM, just ignore accesses */ > - if ( !has_dm ) > + if ( !hvm_has_dm(curr->domain) ) > { > rc = X86EMUL_OKAY; > vio->io_state = HVMIO_none; > @@ -241,7 +219,7 @@ static int hvmemul_do_io( > else > { > rc = X86EMUL_RETRY; > - if ( !hvm_send_assist_req(curr) ) > + if ( !hvm_send_assist_req(curr, &p) ) > vio->io_state = HVMIO_none; > else if ( p_data == NULL ) > rc = X86EMUL_OKAY; > @@ -260,7 +238,7 @@ static int hvmemul_do_io( > > finish_access: > if ( dir == IOREQ_READ ) > - hvmtrace_io_assist(is_mmio, p); > + hvmtrace_io_assist(is_mmio, &p); > > if ( p_data != NULL ) > memcpy(p_data, &vio->io_data, size); > @@ -1292,3 +1270,13 @@ struct segment_register *hvmemul_get_seg_reg( > hvm_get_segment_register(current, seg, &hvmemul_ctxt->seg_reg[seg]); > return &hvmemul_ctxt->seg_reg[seg]; > } > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 69d0a44..573f845 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -352,6 +352,26 @@ void hvm_migrate_pirqs(struct vcpu *v) > spin_unlock(&d->event_lock); > } > > +static ioreq_t *get_ioreq(struct vcpu *v) > +{ > + struct domain *d = v->domain; > + shared_iopage_t *p = d->arch.hvm_domain.ioreq.va; > + > + ASSERT((v == current) || spin_is_locked(&d->arch.hvm_domain.ioreq.lock)); > + > + return p ? &p->vcpu_ioreq[v->vcpu_id] : NULL; > +} > + > +bool_t hvm_io_pending(struct vcpu *v) > +{ > + ioreq_t *p = get_ioreq(v); > + > + if ( !p ) > + return 0; > + > + return ( p->state != STATE_IOREQ_NONE ); > +} > + > void hvm_do_resume(struct vcpu *v) > { > ioreq_t *p = get_ioreq(v); > @@ -370,11 +390,12 @@ void hvm_do_resume(struct vcpu *v) > switch ( p->state ) > { > case STATE_IORESP_READY: /* IORESP_READY -> NONE */ > + rmb(); /* see IORESP_READY /then/ read contents of ioreq */ > hvm_io_assist(p); > break; > case STATE_IOREQ_READY: /* IOREQ_{READY,INPROCESS} -> IORESP_READY > */ > case STATE_IOREQ_INPROCESS: > - wait_on_xen_event_channel(v->arch.hvm_vcpu.xen_port, > + wait_on_xen_event_channel(p->vp_eport, > (p->state != STATE_IOREQ_READY) && > (p->state != STATE_IOREQ_INPROCESS)); > break; > @@ -1414,7 +1435,87 @@ void hvm_vcpu_down(struct vcpu *v) > } > } > > -bool_t hvm_send_assist_req(struct vcpu *v) > +int hvm_buffered_io_send(struct domain *d, const ioreq_t *p) > +{ > + struct hvm_ioreq_page *iorp = &d->arch.hvm_domain.buf_ioreq; > + buffered_iopage_t *pg = iorp->va; > + buf_ioreq_t bp; > + /* Timeoffset sends 64b data, but no address. Use two consecutive slots. > */ > + int qw = 0; > + > + /* Ensure buffered_iopage fits in a page */ > + BUILD_BUG_ON(sizeof(buffered_iopage_t) > PAGE_SIZE); > + > + /* > + * Return 0 for the cases we can't deal with: > + * - 'addr' is only a 20-bit field, so we cannot address beyond 1MB > + * - we cannot buffer accesses to guest memory buffers, as the guest > + * may expect the memory buffer to be synchronously accessed > + * - the count field is usually used with data_is_ptr and since we don't > + * support data_is_ptr we do not waste space for the count field > either > + */ > + if ( (p->addr > 0xffffful) || p->data_is_ptr || (p->count != 1) ) > + return 0; > + > + bp.type = p->type; > + bp.dir = p->dir; > + switch ( p->size ) > + { > + case 1: > + bp.size = 0; > + break; > + case 2: > + bp.size = 1; > + break; > + case 4: > + bp.size = 2; > + break; > + case 8: > + bp.size = 3; > + qw = 1; > + break; > + default: > + gdprintk(XENLOG_WARNING, "unexpected ioreq size: %u\n", p->size); > + return 0; > + } > + > + bp.data = p->data; > + bp.addr = p->addr; > + > + spin_lock(&iorp->lock); > + > + if ( (pg->write_pointer - pg->read_pointer) >= > + (IOREQ_BUFFER_SLOT_NUM - qw) ) > + { > + /* The queue is full: send the iopacket through the normal path. */ > + spin_unlock(&iorp->lock); > + return 0; > + } > + > + pg->buf_ioreq[pg->write_pointer % IOREQ_BUFFER_SLOT_NUM] = bp; > + > + if ( qw ) > + { > + bp.data = p->data >> 32; > + pg->buf_ioreq[(pg->write_pointer+1) % IOREQ_BUFFER_SLOT_NUM] = bp; > + } > + > + /* Make the ioreq_t visible /before/ write_pointer. */ > + wmb(); > + pg->write_pointer += qw ? 2 : 1; > + > + notify_via_xen_event_channel(d, > d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_EVTCHN]); > + spin_unlock(&iorp->lock); > + > + return 1; > +} > + > +bool_t hvm_has_dm(struct domain *d) > +{ > + return !!d->arch.hvm_domain.ioreq.va; > +} > + > +bool_t hvm_send_assist_req(struct vcpu *v, const ioreq_t *proto_p) > { > ioreq_t *p = get_ioreq(v); > > @@ -1432,14 +1533,23 @@ bool_t hvm_send_assist_req(struct vcpu *v) > return 0; > } > > - prepare_wait_on_xen_event_channel(v->arch.hvm_vcpu.xen_port); > + p->dir = proto_p->dir; > + p->data_is_ptr = proto_p->data_is_ptr; > + p->type = proto_p->type; > + p->size = proto_p->size; > + p->addr = proto_p->addr; > + p->count = proto_p->count; > + p->df = proto_p->df; > + p->data = proto_p->data; > + > + prepare_wait_on_xen_event_channel(p->vp_eport); > > /* > * Following happens /after/ blocking and setting up ioreq contents. > * prepare_wait_on_xen_event_channel() is an implicit barrier. > */ > p->state = STATE_IOREQ_READY; > - notify_via_xen_event_channel(v->domain, v->arch.hvm_vcpu.xen_port); > + notify_via_xen_event_channel(v->domain, p->vp_eport); > > return 1; > } > diff --git a/xen/arch/x86/hvm/io.c b/xen/arch/x86/hvm/io.c > index 5ba38d2..8db300d 100644 > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -46,82 +46,6 @@ > #include <xen/iocap.h> > #include <public/hvm/ioreq.h> > > -int hvm_buffered_io_send(struct domain *d, const ioreq_t *p) > -{ > - struct hvm_ioreq_page *iorp = &d->arch.hvm_domain.buf_ioreq; > - buffered_iopage_t *pg = iorp->va; > - buf_ioreq_t bp; > - /* Timeoffset sends 64b data, but no address. Use two consecutive slots. > */ > - int qw = 0; > - > - /* Ensure buffered_iopage fits in a page */ > - BUILD_BUG_ON(sizeof(buffered_iopage_t) > PAGE_SIZE); > - > - /* > - * Return 0 for the cases we can't deal with: > - * - 'addr' is only a 20-bit field, so we cannot address beyond 1MB > - * - we cannot buffer accesses to guest memory buffers, as the guest > - * may expect the memory buffer to be synchronously accessed > - * - the count field is usually used with data_is_ptr and since we don't > - * support data_is_ptr we do not waste space for the count field > either > - */ > - if ( (p->addr > 0xffffful) || p->data_is_ptr || (p->count != 1) ) > - return 0; > - > - bp.type = p->type; > - bp.dir = p->dir; > - switch ( p->size ) > - { > - case 1: > - bp.size = 0; > - break; > - case 2: > - bp.size = 1; > - break; > - case 4: > - bp.size = 2; > - break; > - case 8: > - bp.size = 3; > - qw = 1; > - break; > - default: > - gdprintk(XENLOG_WARNING, "unexpected ioreq size: %u\n", p->size); > - return 0; > - } > - > - bp.data = p->data; > - bp.addr = p->addr; > - > - spin_lock(&iorp->lock); > - > - if ( (pg->write_pointer - pg->read_pointer) >= > - (IOREQ_BUFFER_SLOT_NUM - qw) ) > - { > - /* The queue is full: send the iopacket through the normal path. */ > - spin_unlock(&iorp->lock); > - return 0; > - } > - > - pg->buf_ioreq[pg->write_pointer % IOREQ_BUFFER_SLOT_NUM] = bp; > - > - if ( qw ) > - { > - bp.data = p->data >> 32; > - pg->buf_ioreq[(pg->write_pointer+1) % IOREQ_BUFFER_SLOT_NUM] = bp; > - } > - > - /* Make the ioreq_t visible /before/ write_pointer. */ > - wmb(); > - pg->write_pointer += qw ? 2 : 1; > - > - notify_via_xen_event_channel(d, > - d->arch.hvm_domain.params[HVM_PARAM_BUFIOREQ_EVTCHN]); > - spin_unlock(&iorp->lock); > - > - return 1; > -} > - > void send_timeoffset_req(unsigned long timeoff) > { > ioreq_t p = { > @@ -143,26 +67,14 @@ void send_timeoffset_req(unsigned long timeoff) > /* Ask ioemu mapcache to invalidate mappings. */ > void send_invalidate_req(void) > { > - struct vcpu *v = current; > - ioreq_t *p = get_ioreq(v); > - > - if ( !p ) > - return; > - > - if ( p->state != STATE_IOREQ_NONE ) > - { > - gdprintk(XENLOG_ERR, "WARNING: send invalidate req with something " > - "already pending (%d)?\n", p->state); > - domain_crash(v->domain); > - return; > - } > - > - p->type = IOREQ_TYPE_INVALIDATE; > - p->size = 4; > - p->dir = IOREQ_WRITE; > - p->data = ~0UL; /* flush all */ > + ioreq_t p = { > + .type = IOREQ_TYPE_INVALIDATE, > + .size = 4, > + .dir = IOREQ_WRITE, > + .data = ~0UL, /* flush all */ > + }; > > - (void)hvm_send_assist_req(v); > + (void)hvm_send_assist_req(current, &p); > } > > int handle_mmio(void) > @@ -265,8 +177,6 @@ void hvm_io_assist(ioreq_t *p) > struct hvm_vcpu_io *vio = &curr->arch.hvm_vcpu.hvm_io; > enum hvm_io_state io_state; > > - rmb(); /* see IORESP_READY /then/ read contents of ioreq */ > - > p->state = STATE_IOREQ_NONE; > > io_state = vio->io_state; > diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c > index 40167d6..0421623 100644 > --- a/xen/arch/x86/hvm/vmx/vvmx.c > +++ b/xen/arch/x86/hvm/vmx/vvmx.c > @@ -1394,7 +1394,6 @@ void nvmx_switch_guest(void) > struct vcpu *v = current; > struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v); > struct cpu_user_regs *regs = guest_cpu_user_regs(); > - const ioreq_t *ioreq = get_ioreq(v); > > /* > * A pending IO emulation may still be not finished. In this case, no > @@ -1404,7 +1403,7 @@ void nvmx_switch_guest(void) > * don't want to continue as this setup is not implemented nor supported > * as of right now. > */ > - if ( !ioreq || ioreq->state != STATE_IOREQ_NONE ) > + if ( hvm_io_pending(v) ) > return; > /* > * a softirq may interrupt us between a virtual vmentry is > @@ -2522,3 +2521,13 @@ void nvmx_set_cr_read_shadow(struct vcpu *v, unsigned > int cr) > /* nvcpu.guest_cr is what L2 write to cr actually. */ > __vmwrite(read_shadow_field, v->arch.hvm_vcpu.nvcpu.guest_cr[cr]); > } > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h > index dcc3483..08a62ea 100644 > --- a/xen/include/asm-x86/hvm/hvm.h > +++ b/xen/include/asm-x86/hvm/hvm.h > @@ -26,6 +26,7 @@ > #include <asm/hvm/asid.h> > #include <public/domctl.h> > #include <public/hvm/save.h> > +#include <public/hvm/ioreq.h> > #include <asm/mm.h> > > /* Interrupt acknowledgement sources. */ > @@ -227,7 +228,7 @@ int prepare_ring_for_helper(struct domain *d, unsigned > long gmfn, > struct page_info **_page, void **_va); > void destroy_ring_for_helper(void **_va, struct page_info *page); > > -bool_t hvm_send_assist_req(struct vcpu *v); > +bool_t hvm_send_assist_req(struct vcpu *v, const ioreq_t *p); > > void hvm_get_guest_pat(struct vcpu *v, u64 *guest_pat); > int hvm_set_guest_pat(struct vcpu *v, u64 guest_pat); > @@ -339,6 +340,8 @@ static inline unsigned long hvm_get_shadow_gs_base(struct > vcpu *v) > void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx, > unsigned int *ecx, unsigned int *edx); > void hvm_migrate_timers(struct vcpu *v); > +bool_t hvm_has_dm(struct domain *d); > +bool_t hvm_io_pending(struct vcpu *v); > void hvm_do_resume(struct vcpu *v); > void hvm_migrate_pirqs(struct vcpu *v); > > @@ -522,3 +525,13 @@ bool_t nhvm_vmcx_hap_enabled(struct vcpu *v); > enum hvm_intblk nhvm_interrupt_blocked(struct vcpu *v); > > #endif /* __ASM_X86_HVM_HVM_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > diff --git a/xen/include/asm-x86/hvm/support.h > b/xen/include/asm-x86/hvm/support.h > index 1dc2f2d..05ef5c5 100644 > --- a/xen/include/asm-x86/hvm/support.h > +++ b/xen/include/asm-x86/hvm/support.h > @@ -22,21 +22,10 @@ > #define __ASM_X86_HVM_SUPPORT_H__ > > #include <xen/types.h> > -#include <public/hvm/ioreq.h> > #include <xen/sched.h> > #include <xen/hvm/save.h> > #include <asm/processor.h> > > -static inline ioreq_t *get_ioreq(struct vcpu *v) > -{ > - struct domain *d = v->domain; > - shared_iopage_t *p = d->arch.hvm_domain.ioreq.va; > - > - ASSERT((v == current) || spin_is_locked(&d->arch.hvm_domain.ioreq.lock)); > - > - return p ? &p->vcpu_ioreq[v->vcpu_id] : NULL; > -} > - > #define HVM_DELIVER_NO_ERROR_CODE -1 > > #ifndef NDEBUG > @@ -144,3 +133,13 @@ int hvm_mov_to_cr(unsigned int cr, unsigned int gpr); > int hvm_mov_from_cr(unsigned int cr, unsigned int gpr); > > #endif /* __ASM_X86_HVM_SUPPORT_H__ */ > + > +/* > + * Local variables: > + * mode: C > + * c-file-style: "BSD" > + * c-basic-offset: 4 > + * tab-width: 4 > + * indent-tabs-mode: nil > + * End: > + */ > -- > 1.7.10.4 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxx > http://lists.xen.org/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |