Re: [Xen-devel] [PATCH v6 02/16] x86/hvm: remove multiple open coded 'chunking' loops

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.07.15 at 17:57, <Paul.Durrant@xxxxxxxxxx> wrote:
>>  -----Original Message-----
>> From: xen-devel-bounces@xxxxxxxxxxxxx [mailto:xen-devel-
>> bounces@xxxxxxxxxxxxx] On Behalf Of Jan Beulich
>> Sent: 08 July 2015 16:53
>> To: Andrew Cooper; Paul Durrant
>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; Keir (Xen.org)
>> Subject: Re: [Xen-devel] [PATCH v6 02/16] x86/hvm: remove multiple open
>> coded 'chunking' loops
>> 
>> >>> On 03.07.15 at 18:25, <paul.durrant@xxxxxxxxxx> 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);
>> 
>> Does this really matter for the code below?
>> 
>> > +        chunk = min_t(unsigned int, size, PAGE_SIZE);
>> 
>> Iirc Andrew had asked for this, but I still don't see why: "size" is the
>> width of an instruction operand, and hence won't even come close
>> to PAGE_SIZE.
>> 
> 
> I'll let Andrew answer that one. I've already ping-ponged once on this.
> 
> I guess it makes it safer if someone were to try to use this function for 
> another purpose in future?

I can't see such future use, and hence can't see why we should make
this slower than it needs to be.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel