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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

> +        rc = hvmemul_linear_to_phys(gla, &gpa, chunk, &one_rep, pfec,
> +                                    hvmemul_ctxt);

The fact that &one_rep is being passed here also makes clear that
we can't be dealing with an <operand-width> * <reps> situation.
And if we were, "size" couldn't validly be "unsigned int".

Jan


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