[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 06/17] x86/hvm: revert 82ed8716b "fix direct PCI port I/O emulation retry...
> -----Original Message----- > From: Paul Durrant [mailto:paul.durrant@xxxxxxxxxx] > Sent: 11 June 2015 16:43 > To: xen-devel@xxxxxxxxxxxxxxxxxxxx > Cc: Paul Durrant; Keir (Xen.org); Jan Beulich; Andrew Cooper > Subject: [PATCH v2 06/17] x86/hvm: revert 82ed8716b "fix direct PCI port I/O > emulation retry... > > ...and error handling" > > NOTE: A straight reversion was not possible because of subsequent changes > in the code so this at least partially a manual reversion. > > By limiting hvmemul_do_io_addr() to reps falling within the pages on which > a reference has already been taken, we can guarantee that calls to > hvm_copy_to/from_guest_phys() will not hit the HVMCOPY_gfn_paged_out > or HVMCOPY_gfn_shared cases. Thus we can remove the retry logic from > the intercept code and simplify it significantly. > > Normally hvmemul_do_io_addr() will only reference single page at a time. > It will, however, take an extra page reference for I/O spanning a page > boundary. > > It is still important to know, upon returning from x86_emulate(), whether > the number of reps was reduced so the mmio_retry flag is retained for that > purpose. > > Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx> Unfortunately, during further testing with XenServer, I found an issue with this patch and also subsequent patches in the series. I will therefore be posting a v3 of the series. Paul > Cc: Keir Fraser <keir@xxxxxxx> > Cc: Jan Beulich <jbeulich@xxxxxxxx> > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > --- > xen/arch/x86/hvm/emulate.c | 83 ++++++++++++++++++++++++++++-- > ---------- > xen/arch/x86/hvm/intercept.c | 52 +++++-------------------- > xen/include/asm-x86/hvm/vcpu.h | 2 +- > 3 files changed, 69 insertions(+), 68 deletions(-) > > diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c > index 4af70b0..38b8956 100644 > --- a/xen/arch/x86/hvm/emulate.c > +++ b/xen/arch/x86/hvm/emulate.c > @@ -60,6 +60,7 @@ static int hvmemul_do_io( > .type = is_mmio ? IOREQ_TYPE_COPY : IOREQ_TYPE_PIO, > .addr = addr, > .size = size, > + .count = *reps, > .dir = dir, > .df = df, > .data = data, > @@ -131,15 +132,6 @@ static int hvmemul_do_io( > HVMIO_dispatched : HVMIO_awaiting_completion; > vio->io_size = size; > > - /* > - * When retrying a repeated string instruction, force exit to guest after > - * completion of the retried iteration to allow handling of interrupts. > - */ > - if ( vio->mmio_retrying ) > - *reps = 1; > - > - p.count = *reps; > - > if ( dir == IOREQ_WRITE ) > { > if ( !data_is_addr ) > @@ -153,17 +145,9 @@ static int hvmemul_do_io( > switch ( rc ) > { > case X86EMUL_OKAY: > - case X86EMUL_RETRY: > - *reps = p.count; > p.state = STATE_IORESP_READY; > - if ( !vio->mmio_retry ) > - { > - hvm_io_assist(&p); > - vio->io_state = HVMIO_none; > - } > - else > - /* Defer hvm_io_assist() invocation to hvm_do_resume(). */ > - vio->io_state = HVMIO_handle_mmio_awaiting_completion; > + hvm_io_assist(&p); > + vio->io_state = HVMIO_none; > break; > case X86EMUL_UNHANDLEABLE: > { > @@ -294,17 +278,67 @@ int hvmemul_do_io_addr( > bool_t is_mmio, paddr_t addr, unsigned long *reps, > unsigned int size, uint8_t dir, bool_t df, paddr_t ram_gpa) > { > - struct page_info *ram_page; > + struct vcpu *v = current; > + unsigned long ram_gmfn = paddr_to_pfn(ram_gpa); > + struct page_info *ram_page[2]; > + int nr_pages = 0; > + unsigned long count; > int rc; > > - rc = hvmemul_acquire_page(paddr_to_pfn(ram_gpa), &ram_page); > + > + rc = hvmemul_acquire_page(ram_gmfn, &ram_page[nr_pages]); > if ( rc != X86EMUL_OKAY ) > - return rc; > + goto out; > + > + nr_pages++; > + > + /* Detemine how many reps will fit within this page */ > + for ( count = 0; count < *reps; count++ ) > + { > + paddr_t start, end; > + > + if ( df ) > + { > + start = ram_gpa - count * size; > + end = ram_gpa + size - 1; > + } > + else > + { > + start = ram_gpa; > + end = ram_gpa + (count + 1) * size - 1; > + } > + > + if ( paddr_to_pfn(start) != ram_gmfn || > + paddr_to_pfn(end) != ram_gmfn ) > + break; > + } > + > + if ( count == 0 ) > + { > + /* > + * This access must span two pages, so grab a reference to > + * the next page and do a single rep. > + */ > + rc = hvmemul_acquire_page(df ? ram_gmfn - 1 : ram_gmfn + 1, > + &ram_page[nr_pages]); > + if ( rc != X86EMUL_OKAY ) > + goto out; > > - rc = hvmemul_do_io(is_mmio, addr, reps, size, dir, df, 1, > + nr_pages++; > + count = 1; > + } > + > + rc = hvmemul_do_io(is_mmio, addr, &count, size, dir, df, 1, > (uint64_t)ram_gpa); > + if ( rc == X86EMUL_OKAY ) > + { > + v->arch.hvm_vcpu.hvm_io.mmio_retry = (count < *reps); > + *reps = count; > + } > > - hvmemul_release_page(ram_page); > + out: > + while ( --nr_pages >= 0 ) > + hvmemul_release_page(ram_page[nr_pages]); > > return rc; > } > @@ -1554,7 +1588,6 @@ static int _hvm_emulate_one(struct > hvm_emulate_ctxt *hvmemul_ctxt, > } > > hvmemul_ctxt->exn_pending = 0; > - vio->mmio_retrying = vio->mmio_retry; > vio->mmio_retry = 0; > > if ( cpu_has_vmx ) > diff --git a/xen/arch/x86/hvm/intercept.c b/xen/arch/x86/hvm/intercept.c > index b8bcd0b..c0c9254 100644 > --- a/xen/arch/x86/hvm/intercept.c > +++ b/xen/arch/x86/hvm/intercept.c > @@ -155,7 +155,6 @@ static const struct hvm_io_ops portio_ops = { > static int process_io_intercept(struct vcpu *v, ioreq_t *p, > struct hvm_io_handler *handler) > { > - struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io; > const struct hvm_io_ops *ops = handler->ops; > int rc = X86EMUL_OKAY, i, step = p->df ? -p->size : p->size; > uint64_t data; > @@ -165,23 +164,12 @@ static int process_io_intercept(struct vcpu *v, > ioreq_t *p, > { > for ( i = 0; i < p->count; i++ ) > { > - if ( vio->mmio_retrying ) > - { > - if ( vio->mmio_large_read_bytes != p->size ) > - return X86EMUL_UNHANDLEABLE; > - memcpy(&data, vio->mmio_large_read, p->size); > - vio->mmio_large_read_bytes = 0; > - vio->mmio_retrying = 0; > - } > - else > - { > - addr = (p->type == IOREQ_TYPE_COPY) ? > - p->addr + step * i : > - p->addr; > - rc = ops->read(handler, v, addr, p->size, &data); > - if ( rc != X86EMUL_OKAY ) > - break; > - } > + addr = (p->type == IOREQ_TYPE_COPY) ? > + p->addr + step * i : > + p->addr; > + rc = ops->read(handler, v, addr, p->size, &data); > + if ( rc != X86EMUL_OKAY ) > + break; > > if ( p->data_is_ptr ) > { > @@ -190,14 +178,12 @@ static int process_io_intercept(struct vcpu *v, > ioreq_t *p, > { > case HVMCOPY_okay: > break; > - case HVMCOPY_gfn_paged_out: > - case HVMCOPY_gfn_shared: > - rc = X86EMUL_RETRY; > - break; > case HVMCOPY_bad_gfn_to_mfn: > /* Drop the write as real hardware would. */ > continue; > case HVMCOPY_bad_gva_to_gfn: > + case HVMCOPY_gfn_paged_out: > + case HVMCOPY_gfn_shared: > ASSERT(0); > /* fall through */ > default: > @@ -210,13 +196,6 @@ static int process_io_intercept(struct vcpu *v, > ioreq_t *p, > else > p->data = data; > } > - > - if ( rc == X86EMUL_RETRY ) > - { > - vio->mmio_retry = 1; > - vio->mmio_large_read_bytes = p->size; > - memcpy(vio->mmio_large_read, &data, p->size); > - } > } > else /* p->dir == IOREQ_WRITE */ > { > @@ -229,14 +208,12 @@ static int process_io_intercept(struct vcpu *v, > ioreq_t *p, > { > case HVMCOPY_okay: > break; > - case HVMCOPY_gfn_paged_out: > - case HVMCOPY_gfn_shared: > - rc = X86EMUL_RETRY; > - break; > case HVMCOPY_bad_gfn_to_mfn: > data = ~0; > break; > case HVMCOPY_bad_gva_to_gfn: > + case HVMCOPY_gfn_paged_out: > + case HVMCOPY_gfn_shared: > ASSERT(0); > /* fall through */ > default: > @@ -256,15 +233,6 @@ static int process_io_intercept(struct vcpu *v, > ioreq_t *p, > if ( rc != X86EMUL_OKAY ) > break; > } > - > - if ( rc == X86EMUL_RETRY ) > - vio->mmio_retry = 1; > - } > - > - if ( i != 0 ) > - { > - p->count = i; > - rc = X86EMUL_OKAY; > } > > return rc; > diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm- > x86/hvm/vcpu.h > index dd08416..97d78bd 100644 > --- a/xen/include/asm-x86/hvm/vcpu.h > +++ b/xen/include/asm-x86/hvm/vcpu.h > @@ -74,7 +74,7 @@ struct hvm_vcpu_io { > * For string instruction emulation we need to be able to signal a > * necessary retry through other than function return codes. > */ > - bool_t mmio_retry, mmio_retrying; > + bool_t mmio_retry; > > unsigned long msix_unmask_address; > > -- > 1.7.10.4 _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |