* [PATCH] x86/HVM: refuse doing string operations in certain situations
@ 2013-09-20 13:08 Jan Beulich
2013-09-20 15:30 ` Keir Fraser
2013-09-20 20:05 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2013-09-20 13:08 UTC (permalink / raw)
To: xen-devel; +Cc: Tim Deegan, Keir Fraser
[-- Attachment #1: Type: text/plain, Size: 2192 bytes --]
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);
[-- Attachment #2: x86-HVM-rep-movs-MMIO.patch --]
[-- Type: text/plain, Size: 2251 bytes --]
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);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
2013-09-20 13:08 [PATCH] x86/HVM: refuse doing string operations in certain situations Jan Beulich
@ 2013-09-20 15:30 ` Keir Fraser
2013-09-20 20:05 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 4+ messages in thread
From: Keir Fraser @ 2013-09-20 15:30 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Tim Deegan
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);
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
2013-09-20 13:08 [PATCH] x86/HVM: refuse doing string operations in certain situations Jan Beulich
2013-09-20 15:30 ` Keir Fraser
@ 2013-09-20 20:05 ` Konrad Rzeszutek Wilk
2013-09-23 6:44 ` Jan Beulich
1 sibling, 1 reply; 4+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-09-20 20:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Tim Deegan
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@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/HVM: refuse doing string operations in certain situations
2013-09-20 20:05 ` Konrad Rzeszutek Wilk
@ 2013-09-23 6:44 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2013-09-23 6:44 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk; +Cc: xen-devel, Keir Fraser, Tim Deegan
>>> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-09-23 6:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-20 13:08 [PATCH] x86/HVM: refuse doing string operations in certain situations Jan Beulich
2013-09-20 15:30 ` Keir Fraser
2013-09-20 20:05 ` Konrad Rzeszutek Wilk
2013-09-23 6:44 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).