Re: [Xen-devel] [PATCH v2 2/2] x86/hvm/emulate: make sure rep I/O emulation does not cross GFN boundaries

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.08.18 at 16:48, <paul.durrant@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -184,6 +184,25 @@ static int hvmemul_do_io(
>          hvmtrace_io_assist(&p);
>      }
>  
> +    /*
> +     * Make sure that we truncate rep MMIO at any GFN boundary. This is
> +     * necessary to ensure that the correct device model is targetted
> +     * or that we correctly handle a rep op spanning MMIO and RAM.
> +     */
> +    if ( unlikely(p.count > 1) && p.type == IOREQ_TYPE_COPY )
> +    {
> +        unsigned long off = p.addr & ~PAGE_MASK;
> +
> +        if ( PAGE_SIZE - off < p.size ) /* single rep spans GFN */
> +            p.count = 1;
> +        else
> +            p.count = min_t(unsigned long,
> +                            p.count,
> +                            ((p.df ? (off + p.size) : (PAGE_SIZE - off)) /
> +                             p.size));
> +    }
> +    ASSERT(p.count);

I've applied the patch with the earlier suggested changes, but I wonder
if the placement especially wrt the visible in context hvmtrace_io_assist()
isn't sub-optimal.

Considering the other splitting done elsewhere I also wonder whether
some of that can't then go away, or whether the specific code path
where you've found the splitting to be missing wouldn't better be fixed
instead. It certainly feels wrong for splitting to happen in multiple
places for (I think) a single request.

I'll defer the decision whether to backport this until we've settled both
of the above.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel