Jan Beulich
2013-Sep-20 13:08 UTC
[PATCH] 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@suse.com>
--- 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@lists.xen.org
http://lists.xen.org/xen-devel
Keir Fraser
2013-Sep-20 15:30 UTC
Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
On 20/09/2013 14:08, "Jan Beulich" <JBeulich@suse.com> 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 > > Signed-off-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>> --- 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); > > >
Konrad Rzeszutek Wilk
2013-Sep-20 20:05 UTC
Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
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 MMIOCould you explain this in a bit more details? Why is acceleration in those cases bad? Thanks.> > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > --- 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@suse.com> > > --- 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@lists.xen.org > http://lists.xen.org/xen-devel
Jan Beulich
2013-Sep-23 06:44 UTC
Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
>>> On 20.09.13 at 22:05, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > 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?- we currently can''t have more than one request issued to qemu for a single guest side instruction (there''s only one ioreq_t instance per vCPU) - hvm_copy_{to,from}_guest_phys() don''t honor the original access size Jan