From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v2 04/17] x86/hvm: unify dpci portio intercept with standard portio intercept Date: Wed, 17 Jun 2015 15:36:45 +0100 Message-ID: <5581A21D02000078000863C1@mail.emea.novell.com> References: <1434037381-10917-1-git-send-email-paul.durrant@citrix.com> <1434037381-10917-5-git-send-email-paul.durrant@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z5ESQ-0007Kg-KS for xen-devel@lists.xenproject.org; Wed, 17 Jun 2015 14:36:50 +0000 In-Reply-To: <1434037381-10917-5-git-send-email-paul.durrant@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Paul Durrant Cc: Andrew Cooper , Keir Fraser , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org >>> On 11.06.15 at 17:42, wrote: > --- a/xen/arch/x86/hvm/io.c > +++ b/xen/arch/x86/hvm/io.c > @@ -208,185 +208,113 @@ void hvm_io_assist(ioreq_t *p) > } > } > > -static int dpci_ioport_read(uint32_t mport, ioreq_t *p) > +static bool_t dpci_portio_accept(struct hvm_io_handler *io_handler, > + struct vcpu *v, > + uint64_t addr, > + uint32_t size) > { > - struct hvm_vcpu_io *vio = ¤t->arch.hvm_vcpu.hvm_io; > - int rc = X86EMUL_OKAY, i, step = p->df ? -p->size : p->size; > - uint32_t data = 0; > - > - 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 switch ( p->size ) > - { > - case 1: > - data = inb(mport); > - break; > - case 2: > - data = inw(mport); > - break; > - case 4: > - data = inl(mport); > - break; > - default: > - BUG(); > - } > + struct hvm_iommu *hd = domain_hvm_iommu(v->domain); > + struct hvm_vcpu_io *vio = &v->arch.hvm_vcpu.hvm_io; > + struct g2m_ioport *g2m_ioport; > + uint32_t start, end; > + uint32_t gport = addr, mport; > > - if ( p->data_is_ptr ) > - { > - switch ( hvm_copy_to_guest_phys(p->data + step * i, > - &data, p->size) ) > - { > - 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: > - ASSERT(0); > - /* fall through */ > - default: > - rc = X86EMUL_UNHANDLEABLE; > - break; > - } > - if ( rc != X86EMUL_OKAY) > - break; > - } > - else > - p->data = data; > - } > > - if ( rc == X86EMUL_RETRY ) > + list_for_each_entry( g2m_ioport, &hd->arch.g2m_ioport_list, list ) > { > - vio->mmio_retry = 1; > - vio->mmio_large_read_bytes = p->size; > - memcpy(vio->mmio_large_read, &data, p->size); > + start = g2m_ioport->gport; > + end = start + g2m_ioport->np; > + if ( (gport >= start) && (gport < end) ) > + goto found; > } > > - if ( i != 0 ) > + return 0; > + > + found: > + mport = (gport - start) + g2m_ioport->mport; > + > + if ( !ioports_access_permitted(current->domain, mport, Isn't v == current here (which of course should be made obvious again by naming it curr)? And then - is this check needed here at all? I realize you just move it, but the permission check should be (and is being) done in the XEN_DOMCTL_ioport_mapping handler. > +void register_dpci_portio_handler(struct domain *d) > +{ > + struct hvm_io_handler *handler = &d->arch.hvm_domain.dpci_handler; > > - switch ( p->dir ) > - { > - case IOREQ_READ: > - rc = dpci_ioport_read(mport, p); > - break; > - case IOREQ_WRITE: > - rc = dpci_ioport_write(mport, p); > - break; > - default: > - gdprintk(XENLOG_ERR, "Error: couldn't handle p->dir = %d", p->dir); > - rc = X86EMUL_UNHANDLEABLE; > - } > + handler->type = IOREQ_TYPE_PIO; > + handler->ops = &dpci_portio_ops; > > - return rc; > + hvm_register_io_handler(d, handler, 1); Again registering a pointer into struct domain, with invariant initialization. Jan