From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Durrant Subject: Re: [PATCH v2 04/17] x86/hvm: unify dpci portio intercept with standard portio intercept Date: Wed, 17 Jun 2015 15:17:20 +0000 Message-ID: <9AAE0902D5BC7E449B7C8E4E778ABCD0259537A9@AMSPEX01CL01.citrite.net> 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> <5581A733020000780008642C@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1Z5F5f-0007Ke-SB for xen-devel@lists.xenproject.org; Wed, 17 Jun 2015 15:17:23 +0000 In-Reply-To: <5581A733020000780008642C@mail.emea.novell.com> Content-Language: en-US List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Andrew Cooper , "Keir (Xen.org)" , "xen-devel@lists.xenproject.org" List-Id: xen-devel@lists.xenproject.org > -----Original Message----- > From: Jan Beulich [mailto:JBeulich@suse.com] > Sent: 17 June 2015 15:58 > To: Paul Durrant > Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org) > Subject: RE: [PATCH v2 04/17] x86/hvm: unify dpci portio intercept with > standard portio intercept > > >>> 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. > Ok. > >> > +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)? > I had envisaged that maybe msixtbl code might be adapted to use the list too (rather than iterating over its own list but I never got as far as that). I'll use an array for now. Paul > Jan