* [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
@ 2016-11-16 10:51 Andrew Cooper
2016-11-16 10:56 ` Jan Beulich
2016-11-21 10:40 ` Andrew Cooper
0 siblings, 2 replies; 7+ messages in thread
From: Andrew Cooper @ 2016-11-16 10:51 UTC (permalink / raw)
To: Xen-devel
Cc: Andrew Cooper, Boris Ostrovsky, Wei Liu, Suravee Suthikulpanit,
Jan Beulich
vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
back around to 0. Instead, complain if the reported length is greater than 15
and use x86_decode_insn() as a fallback.
While making changes here, fix two whitespace issues with the case labels.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
xen/arch/x86/hvm/svm/emulate.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/emulate.c b/xen/arch/x86/hvm/svm/emulate.c
index 36585b0..ca670bf 100644
--- a/xen/arch/x86/hvm/svm/emulate.c
+++ b/xen/arch/x86/hvm/svm/emulate.c
@@ -30,18 +30,18 @@ static unsigned long svm_nextrip_insn_length(struct vcpu *v)
{
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
- if ( !cpu_has_svm_nrips || (vmcb->nextrip <= vmcb->rip) )
+ if ( !cpu_has_svm_nrips )
return 0;
#ifndef NDEBUG
switch ( vmcb->exitcode )
{
- case VMEXIT_CR0_READ... VMEXIT_DR15_WRITE:
+ case VMEXIT_CR0_READ ... VMEXIT_DR15_WRITE:
/* faults due to instruction intercepts */
/* (exitcodes 84-95) are reserved */
case VMEXIT_IDTR_READ ... VMEXIT_TR_WRITE:
case VMEXIT_RDTSC ... VMEXIT_MSR:
- case VMEXIT_VMRUN ... VMEXIT_XSETBV:
+ case VMEXIT_VMRUN ... VMEXIT_XSETBV:
/* ...and the rest of the #VMEXITs */
case VMEXIT_CR0_SEL_WRITE:
case VMEXIT_EXCEPTION_BP:
@@ -88,7 +88,8 @@ int __get_instruction_length_from_list(struct vcpu *v,
struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
struct hvm_emulate_ctxt ctxt;
struct x86_emulate_state *state;
- unsigned int inst_len, j, modrm_rm, modrm_reg;
+ unsigned long inst_len, j;
+ unsigned int modrm_rm, modrm_reg;
int modrm_mod;
/*
@@ -96,7 +97,9 @@ int __get_instruction_length_from_list(struct vcpu *v,
* hardware.
*/
#ifdef NDEBUG
- if ( (inst_len = svm_nextrip_insn_length(v)) != 0 )
+ if ( (inst_len = svm_nextrip_insn_length(v)) > MAX_INST_LEN )
+ gprintk(XENLOG_WARNING, "NRip reported insn_len %lu\n", insn_len);
+ else if ( insn_len != 0 )
return inst_len;
if ( vmcb->exitcode == VMEXIT_IOIO )
@@ -120,7 +123,7 @@ int __get_instruction_length_from_list(struct vcpu *v,
j = svm_nextrip_insn_length(v);
if ( j && j != inst_len )
{
- gprintk(XENLOG_WARNING, "insn-len[%02x]=%u (exp %u)\n",
+ gprintk(XENLOG_WARNING, "insn-len[%02x]=%lu (exp %lu)\n",
ctxt.ctxt.opcode, inst_len, j);
return j;
}
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-16 10:51 [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0 Andrew Cooper
@ 2016-11-16 10:56 ` Jan Beulich
2016-11-16 14:52 ` Wei Liu
2016-11-21 10:40 ` Andrew Cooper
1 sibling, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2016-11-16 10:56 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Boris Ostrovsky, Wei Liu, SuraveeSuthikulpanit, Xen-devel
>>> On 16.11.16 at 11:51, <andrew.cooper3@citrix.com> wrote:
> vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
> back around to 0. Instead, complain if the reported length is greater than 15
> and use x86_decode_insn() as a fallback.
>
> While making changes here, fix two whitespace issues with the case labels.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-16 10:56 ` Jan Beulich
@ 2016-11-16 14:52 ` Wei Liu
0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2016-11-16 14:52 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Boris Ostrovsky, Wei Liu, SuraveeSuthikulpanit,
Xen-devel
On Wed, Nov 16, 2016 at 03:56:04AM -0700, Jan Beulich wrote:
> >>> On 16.11.16 at 11:51, <andrew.cooper3@citrix.com> wrote:
> > vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
> > back around to 0. Instead, complain if the reported length is greater than 15
> > and use x86_decode_insn() as a fallback.
> >
> > While making changes here, fix two whitespace issues with the case labels.
> >
> > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
Release-acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-16 10:51 [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0 Andrew Cooper
2016-11-16 10:56 ` Jan Beulich
@ 2016-11-21 10:40 ` Andrew Cooper
2016-11-21 13:38 ` Boris Ostrovsky
1 sibling, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2016-11-21 10:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Boris Ostrovsky, Wei Liu, Suravee Suthikulpanit, Jan Beulich
On 16/11/16 10:51, Andrew Cooper wrote:
> vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
> back around to 0. Instead, complain if the reported length is greater than 15
> and use x86_decode_insn() as a fallback.
>
> While making changes here, fix two whitespace issues with the case labels.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> CC: Wei Liu <wei.liu2@citrix.com>
SVM maintainers, ping?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-21 10:40 ` Andrew Cooper
@ 2016-11-21 13:38 ` Boris Ostrovsky
2016-11-21 13:53 ` Andrew Cooper
0 siblings, 1 reply; 7+ messages in thread
From: Boris Ostrovsky @ 2016-11-21 13:38 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Suravee Suthikulpanit, Jan Beulich
On 11/21/2016 05:40 AM, Andrew Cooper wrote:
> On 16/11/16 10:51, Andrew Cooper wrote:
>> vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
>> back around to 0. Instead, complain if the reported length is greater than 15
>> and use x86_decode_insn() as a fallback.
Why do we need to complain? In the case that you are addressing by this
patch wouldn't that be the expected result (length>15)?
-boris
>>
>> While making changes here, fix two whitespace issues with the case labels.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
> SVM maintainers, ping?
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-21 13:38 ` Boris Ostrovsky
@ 2016-11-21 13:53 ` Andrew Cooper
2016-11-21 14:00 ` Boris Ostrovsky
0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2016-11-21 13:53 UTC (permalink / raw)
To: Boris Ostrovsky, Xen-devel; +Cc: Wei Liu, Suravee Suthikulpanit, Jan Beulich
On 21/11/16 13:38, Boris Ostrovsky wrote:
> On 11/21/2016 05:40 AM, Andrew Cooper wrote:
>> On 16/11/16 10:51, Andrew Cooper wrote:
>>> vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
>>> back around to 0. Instead, complain if the reported length is greater than 15
>>> and use x86_decode_insn() as a fallback.
> Why do we need to complain? In the case that you are addressing by this
> patch wouldn't that be the expected result (length>15)?
No. An instruction crossing the boundary looks like:
e.g. nextrip = 0x3, rip = 0xfffffffffffffffe
As this is all evaluated in unsigned long arithmetic, nextrip - rip
evaluates to 5, which is correct.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0
2016-11-21 13:53 ` Andrew Cooper
@ 2016-11-21 14:00 ` Boris Ostrovsky
0 siblings, 0 replies; 7+ messages in thread
From: Boris Ostrovsky @ 2016-11-21 14:00 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Suravee Suthikulpanit, Jan Beulich
On 11/21/2016 08:53 AM, Andrew Cooper wrote:
> On 21/11/16 13:38, Boris Ostrovsky wrote:
>> On 11/21/2016 05:40 AM, Andrew Cooper wrote:
>>> On 16/11/16 10:51, Andrew Cooper wrote:
>>>> vmcb->nextrip can legitimately be less than vmcb->rip when execution wraps
>>>> back around to 0. Instead, complain if the reported length is greater than 15
>>>> and use x86_decode_insn() as a fallback.
>> Why do we need to complain? In the case that you are addressing by this
>> patch wouldn't that be the expected result (length>15)?
> No. An instruction crossing the boundary looks like:
>
> e.g. nextrip = 0x3, rip = 0xfffffffffffffffe
>
> As this is all evaluated in unsigned long arithmetic, nextrip - rip
> evaluates to 5, which is correct.
Oh, right.
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-11-21 14:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-16 10:51 [PATCH for-4.8] x86/svm: Fix svm_nextrip_insn_length() when crossing the virtual boundary to 0 Andrew Cooper
2016-11-16 10:56 ` Jan Beulich
2016-11-16 14:52 ` Wei Liu
2016-11-21 10:40 ` Andrew Cooper
2016-11-21 13:38 ` Boris Ostrovsky
2016-11-21 13:53 ` Andrew Cooper
2016-11-21 14:00 ` Boris Ostrovsky
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).