Re: [Xen-devel] [PATCH] x86/HVM: refuse doing string operations in certain situations

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Fri, Sep 20, 2013 at 02:08:45PM +0100, Jan Beulich wrote:
> We shouldn't do any acceleration for
> - "rep movs" when either side is passed through MMIO or when both sides
>   are handled by qemu
> - "rep ins" and "rep outs" when the memory operand is any kind of MMIO

Could you explain this in a bit more details?

Why is acceleration in those cases bad?

Thanks.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -686,6 +686,7 @@ static int hvmemul_rep_ins(
>      unsigned long addr;
>      uint32_t pfec = PFEC_page_present | PFEC_write_access;
>      paddr_t gpa;
> +    p2m_type_t p2mt;
>      int rc;
>  
>      rc = hvmemul_virtual_to_linear(
> @@ -702,6 +703,10 @@ static int hvmemul_rep_ins(
>      if ( rc != X86EMUL_OKAY )
>          return rc;
>  
> +    (void) get_gfn_query_unlocked(current->domain, gpa >> PAGE_SHIFT, &p2mt);
> +    if ( p2mt == p2m_mmio_direct || p2mt == p2m_mmio_dm )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      return hvmemul_do_pio(src_port, reps, bytes_per_rep, gpa, IOREQ_READ,
>                            !!(ctxt->regs->eflags & X86_EFLAGS_DF), NULL);
>  }
> @@ -719,6 +724,7 @@ static int hvmemul_rep_outs(
>      unsigned long addr;
>      uint32_t pfec = PFEC_page_present;
>      paddr_t gpa;
> +    p2m_type_t p2mt;
>      int rc;
>  
>      rc = hvmemul_virtual_to_linear(
> @@ -735,6 +741,10 @@ static int hvmemul_rep_outs(
>      if ( rc != X86EMUL_OKAY )
>          return rc;
>  
> +    (void) get_gfn_query_unlocked(current->domain, gpa >> PAGE_SHIFT, &p2mt);
> +    if ( p2mt == p2m_mmio_direct || p2mt == p2m_mmio_dm )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      return hvmemul_do_pio(dst_port, reps, bytes_per_rep, gpa, IOREQ_WRITE,
>                            !!(ctxt->regs->eflags & X86_EFLAGS_DF), NULL);
>  }
> @@ -787,6 +797,10 @@ static int hvmemul_rep_movs(
>      (void) get_gfn_query_unlocked(current->domain, sgpa >> PAGE_SHIFT, 
> &sp2mt);
>      (void) get_gfn_query_unlocked(current->domain, dgpa >> PAGE_SHIFT, 
> &dp2mt);
>  
> +    if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
> +         (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      if ( sp2mt == p2m_mmio_dm )
>          return hvmemul_do_mmio(
>              sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);
> 
> 
> 

> x86/HVM: refuse doing string operations in certain situations
> 
> We shouldn't do any acceleration for
> - "rep movs" when either side is passed through MMIO or when both sides
>   are handled by qemu
> - "rep ins" and "rep outs" when the memory operand is any kind of MMIO
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/hvm/emulate.c
> +++ b/xen/arch/x86/hvm/emulate.c
> @@ -686,6 +686,7 @@ static int hvmemul_rep_ins(
>      unsigned long addr;
>      uint32_t pfec = PFEC_page_present | PFEC_write_access;
>      paddr_t gpa;
> +    p2m_type_t p2mt;
>      int rc;
>  
>      rc = hvmemul_virtual_to_linear(
> @@ -702,6 +703,10 @@ static int hvmemul_rep_ins(
>      if ( rc != X86EMUL_OKAY )
>          return rc;
>  
> +    (void) get_gfn_query_unlocked(current->domain, gpa >> PAGE_SHIFT, &p2mt);
> +    if ( p2mt == p2m_mmio_direct || p2mt == p2m_mmio_dm )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      return hvmemul_do_pio(src_port, reps, bytes_per_rep, gpa, IOREQ_READ,
>                            !!(ctxt->regs->eflags & X86_EFLAGS_DF), NULL);
>  }
> @@ -719,6 +724,7 @@ static int hvmemul_rep_outs(
>      unsigned long addr;
>      uint32_t pfec = PFEC_page_present;
>      paddr_t gpa;
> +    p2m_type_t p2mt;
>      int rc;
>  
>      rc = hvmemul_virtual_to_linear(
> @@ -735,6 +741,10 @@ static int hvmemul_rep_outs(
>      if ( rc != X86EMUL_OKAY )
>          return rc;
>  
> +    (void) get_gfn_query_unlocked(current->domain, gpa >> PAGE_SHIFT, &p2mt);
> +    if ( p2mt == p2m_mmio_direct || p2mt == p2m_mmio_dm )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      return hvmemul_do_pio(dst_port, reps, bytes_per_rep, gpa, IOREQ_WRITE,
>                            !!(ctxt->regs->eflags & X86_EFLAGS_DF), NULL);
>  }
> @@ -787,6 +797,10 @@ static int hvmemul_rep_movs(
>      (void) get_gfn_query_unlocked(current->domain, sgpa >> PAGE_SHIFT, 
> &sp2mt);
>      (void) get_gfn_query_unlocked(current->domain, dgpa >> PAGE_SHIFT, 
> &dp2mt);
>  
> +    if ( sp2mt == p2m_mmio_direct || dp2mt == p2m_mmio_direct ||
> +         (sp2mt == p2m_mmio_dm && dp2mt == p2m_mmio_dm) )
> +        return X86EMUL_UNHANDLEABLE;
> +
>      if ( sp2mt == p2m_mmio_dm )
>          return hvmemul_do_mmio(
>              sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);

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


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