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:
> When emulating a rep I/O operation it is possible that the ioreq will
> describe a single operation that spans multiple GFNs. This is fine as long
> as all those GFNs fall within an MMIO region covered by a single device
> model, but unfortunately the higher levels of the emulation code do not
> guarantee that. This is something that should almost certainly be fixed,
> but in the meantime this patch makes sure that MMIO is truncated at GFN
> boundaries and hence the appropriate device model is re-evaluated for each
> target GFN.
> 
> NOTE: This patch does not deal with the case of a single MMIO operation
>       spanning a GFN boundary. That is more complex to deal with and is
>       deferred to a subsequent patch.
> 
> Signed-off-by: Paul Durrant <paul.durrant@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with a type change request:

> --- 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 this was "unsigned int", all calculations below could be slightly
cheaper 32-bit ones and ...

> +        if ( PAGE_SIZE - off < p.size ) /* single rep spans GFN */
> +            p.count = 1;
> +        else
> +            p.count = min_t(unsigned long,

... this could be just min(), as long as ...

> +                            p.count,
> +                            ((p.df ? (off + p.size) : (PAGE_SIZE - off)) /

... the 3rd arg of the ?: gets cast to unsigned int. If you agree, I'd
be fine doing the adjustments while committing.

Jan



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