From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v4 07/17] x86/hvm: add length to mmio check op Date: Wed, 24 Jun 2015 15:01:17 +0100 Message-ID: <558AD44D0200007800088F9D@mail.emea.novell.com> References: <1435145089-21999-1-git-send-email-paul.durrant@citrix.com> <1435145089-21999-8-git-send-email-paul.durrant@citrix.com> <558AC7E80200007800088E7E@mail.emea.novell.com> <9AAE0902D5BC7E449B7C8E4E778ABCD02596B9DD@AMSPEX01CL02.citrite.net> <558ACBD40200007800088EE6@mail.emea.novell.com> <9AAE0902D5BC7E449B7C8E4E778ABCD02596BAA2@AMSPEX01CL02.citrite.net> 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 1Z7lEx-0007o5-R2 for xen-devel@lists.xenproject.org; Wed, 24 Jun 2015 14:01:23 +0000 In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD02596BAA2@AMSPEX01CL02.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 24.06.15 at 15:34, wrote: >> -----Original Message----- >> From: Jan Beulich [mailto:JBeulich@suse.com] >> Sent: 24 June 2015 14:25 >> To: Paul Durrant >> Cc: Andrew Cooper; xen-devel@lists.xenproject.org; Keir (Xen.org) >> Subject: RE: [Xen-devel] [PATCH v4 07/17] x86/hvm: add length to mmio >> check op >> >> >>> On 24.06.15 at 15:14, wrote: >> >> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel- >> >> bounces@lists.xen.org] On Behalf Of Jan Beulich >> >> Sent: 24 June 2015 14:08 >> >> >>> On 24.06.15 at 13:24, wrote: >> >> > --- a/xen/include/asm-x86/hvm/io.h >> >> > +++ b/xen/include/asm-x86/hvm/io.h >> >> > @@ -35,7 +35,9 @@ typedef int (*hvm_mmio_write_t)(struct vcpu *v, >> >> > unsigned long addr, >> >> > unsigned long length, >> >> > unsigned long val); >> >> > -typedef int (*hvm_mmio_check_t)(struct vcpu *v, unsigned long >> addr); >> >> > +typedef int (*hvm_mmio_check_t)(struct vcpu *v, >> >> > + unsigned long addr, >> >> > + unsigned long length); >> >> >> >> I don't think this really needs to be "long"? >> >> >> > >> > For consistency with the mmio read and write function types I went with >> > 'long'. Is there any harm in that? >> >> Generally generates worse code (due to the need for the REX64 >> prefix on all involved instructions). Perhaps the other ones don't >> need sizes/lengths passed as longs either? >> > > I'm happy to do it that way round if you don't mind the extra diffs. I'll do > it as a separate patch just before this one, to ease review. Thanks. There's actually a good example of why they shouldn't be unsigned long in patch 9 I just looked at: +static int stdvga_mem_write(struct vcpu *v, unsigned long addr, + unsigned long size, unsigned long data) { + ioreq_t p = { .type = IOREQ_TYPE_COPY, + .addr = addr, + .size = size, This clearly would truncate size if it ever exceeded a uint32_t (and hence an unsigned int on x86). Jan