From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v7 01/15] x86/hvm: remove multiple open
 coded 'chunking' loops
Date: Thu, 09 Jul 2015 17:24:13 +0100
Message-ID: <559EBC4D020000780008EFAB@mail.emea.novell.com>
References: <1436447455-11524-1-git-send-email-paul.durrant@citrix.com>
	<1436447455-11524-2-git-send-email-paul.durrant@citrix.com>
	<559EABAF020000780008EEC6@mail.emea.novell.com>
	<9AAE0902D5BC7E449B7C8E4E778ABCD02F4B7014@AMSPEX01CL02.citrite.net>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <9AAE0902D5BC7E449B7C8E4E778ABCD02F4B7014@AMSPEX01CL02.citrite.net>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Paul Durrant <Paul.Durrant@citrix.com>
Cc: "Keir (Xen.org)" <keir@xen.org>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.org

>>> On 09.07.15 at 18:16, <Paul.Durrant@citrix.com> wrote:
>>  -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: 09 July 2015 16:13
>> To: Paul Durrant
>> Cc: xen-devel@lists.xen.org; Keir (Xen.org)
>> Subject: Re: [PATCH v7 01/15] x86/hvm: remove multiple open coded
>> 'chunking' loops
>> 
>> >>> On 09.07.15 at 15:10, <paul.durrant@citrix.com> wrote:
>> > +static int hvmemul_linear_mmio_access(
>> > +    unsigned long gla, unsigned int size, uint8_t dir, uint8_t *buffer,
>> > +    uint32_t pfec, struct hvm_emulate_ctxt *hvmemul_ctxt, bool_t
>> known_gpfn)
>> > +{
>> > +    struct hvm_vcpu_io *vio = &current->arch.hvm_vcpu.hvm_io;
>> > +    unsigned long offset = gla & ~PAGE_MASK;
>> > +    unsigned int chunk;
>> > +    paddr_t gpa;
>> > +    unsigned long one_rep = 1;
>> > +    int rc;
>> > +
>> > +    chunk = min_t(unsigned int, size, PAGE_SIZE - offset);
>> > +
>> > +    if ( known_gpfn )
>> > +        gpa = pfn_to_paddr(vio->mmio_gpfn) | offset;
>> > +    else
>> > +    {
>> > +        rc = hvmemul_linear_to_phys(gla, &gpa, chunk, &one_rep, pfec,
>> > +                                    hvmemul_ctxt);
>> > +        if ( rc != X86EMUL_OKAY )
>> > +            return rc;
>> > +    }
>> > +
>> > +    for ( ;; )
>> > +    {
>> > +        rc = hvmemul_phys_mmio_access(gpa, chunk, dir, buffer);
>> > +        if ( rc != X86EMUL_OKAY )
>> > +            break;
>> > +
>> > +        gla += chunk;
>> > +        buffer += chunk;
>> > +        size -= chunk;
>> > +
>> > +        if ( size == 0 )
>> > +            break;
>> > +
>> > +        ASSERT((gla & ~PAGE_MASK) == 0);
>> 
>> While I don't mean you to re-submit another time, I'd still like to
>> get my question answered: Does this really matter for the code
>> below?
> 
> No, it doesn't, but if it's not true then an incorrect chunk size was chosen 
> above.

I suspected as much. It's then just slightly less odd than having

    x = 0;
    ASSERT(x == 0);

I guess I'll strip the ASSERT() when applying.

Jan