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:58:27 +0100 Message-ID: <5581A733020000780008642C@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> <5581A21D02000078000863C1@mail.emea.novell.com> <9AAE0902D5BC7E449B7C8E4E778ABCD0259535FB@AMSPEX01CL01.citrite.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z5EnO-0003Jv-Uy for xen-devel@lists.xenproject.org; Wed, 17 Jun 2015 14:58:31 +0000 In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD0259535FB@AMSPEX01CL01.citrite.net> 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(Xen.org)" , "xen-devel@lists.xenproject.org" List-Id: xen-devel@lists.xenproject.org >>> On 17.06.15 at 16:46, wrote: >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: 17 June 2015 15:37 >> >>> On 11.06.15 at 17:42, wrote: >> > + 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. >> > > Ok. I'll happily drop the check. Just please make sure you mention this in the description. >> > +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. > > Agreed, but I don't see any other way of unifying this with the main process > loop. Do you have an alternative suggestion? Since the list elements are all part of struct domain, why can't this be an array instead (presumably yielding cheaper iteration over it at once)? Jan