Re: [PATCH v4 8/8] x86/vm_event: Add HVM debug exception vm_events

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Tamas K Lengyel <tamas@tklengyel.com>, Jan Beulich <JBeulich@suse.com>
Cc: Kevin Tian <kevin.tian@intel.com>, Wei Liu <wei.liu2@citrix.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v4 8/8] x86/vm_event: Add HVM debug exception vm_events
Date: Wed, 1 Jun 2016 23:17:43 +0100	[thread overview]
Message-ID: <05a98321-f49d-6709-b6ff-5237ca17ca1e@citrix.com> (raw)
In-Reply-To: <CABfawhkG_DZCNFB4bZq4+M1KmSixgkVWm53V5ROZZOg7c=6_nw@mail.gmail.com>

On 01/06/2016 22:46, Tamas K Lengyel wrote:
> On Tue, May 31, 2016 at 1:59 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>> On 30.05.16 at 22:13, <tamas@tklengyel.com> wrote:
>>> On Mon, May 30, 2016 at 8:16 AM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>>> On 30.05.16 at 00:37, <tamas@tklengyel.com> wrote:
>>>>> @@ -3393,8 +3409,9 @@ void vmx_vmexit_handler(struct cpu_user_regs *regs)
>>>>>              }
>>>>>              else {
>>>>>                  int handled =
>>>>> -                    hvm_monitor_breakpoint(regs->eip,
>>>>> -                                           HVM_MONITOR_SOFTWARE_BREAKPOINT);
>>>>> +                        hvm_monitor_debug(regs->eip,
>>>>> +                                          HVM_MONITOR_SOFTWARE_BREAKPOINT,
>>>>> +                                          X86_EVENTTYPE_SW_EXCEPTION, 1);
>>>> Please let's not add further mistakes like this, assuming INT3 can't
>>>> have any prefixes. It can, even if they're useless.
>>> You mean the instruction length is not necessarily 1? Ultimately it
>>> doesn't seem to matter because reinjecting it with xc_hvm_inject_trap
>>> ignores this field. Instruction length is only required to be properly
>>> set AFAICT for a subset of debug exceptions during reinjection.
>> As you suggest later in your reply, if the insn length really doesn't
>> matter, this should be made recognizable here. Either by a suitably
>> named manifest constant (which could then even evaluate to zero),
>> or by a comment (personally I'd prefer the former, but I'm not
>> maintainer of this code).
>>
>> Jan
>
> Running Andrew's framework with xen-access monitoring breakpoints results in
>
> xen-access:
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
>
> xl dmesg:
> (d28) --- Xen Test Framework ---
> (d28) Environment: HVM 64bit (Long mode 4 levels)
> (d28) Trap emulation
> (d28) Warning: FEP support not detected - some tests will be skipped
> (d28) Test cpl0: all perms ok
> (d28)   Testing int3
> (XEN) d28v0 VMRESUME error: 0x7
> (XEN) domain_crash_sync called from vmcs.c:1599
> (XEN) Domain 28 (vcpu#0) crashed on cpu#7:
> (XEN) ----[ Xen-4.6.1  x86_64  debug=n  Not tainted ]----
> (XEN) CPU:    7
> (XEN) RIP:    0008:[<00000000001032d1>]
> (XEN) RFLAGS: 0000000000000046   CONTEXT: hvm guest (d28v0)
> (XEN) rax: 00000000001032d2   rbx: 00000000001102b0   rcx: 0000000000000000
> (XEN) rdx: 0000000000104af0   rsi: 0000000000000000   rdi: 0000000000000000
> (XEN) rbp: 0000000000000001   rsp: 0000000000114f98   r8:  000000000000000f
> (XEN) r9:  00000000000000ad   r10: 000000000000000f   r11: 0000000000000004
> (XEN) r12: 0000000000000003   r13: 0000000000000000   r14: 0000000000000000
> (XEN) r15: 0000000000000000   cr0: 0000000080000011   cr4: 0000000000000020
> (XEN) cr3: 000000000010b000   cr2: 0000000000000000
> (XEN) ds: 0033   es: 0033   fs: 0033   gs: 0033   ss: 0000   cs: 0008
>
> This is likely because xen-access sets the instruction length to 0
> during reinjection. If I change that to 1 the tests still fail but
> without crashing the domain, output:
>
> xen-access:
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e2, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e2, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032d1, gfn=103 (vcpu 0)
> Got event from Xen
> Breakpoint: rip=00000000001032e1, gfn=103 (vcpu 0)
>
> xl dmesg:
> (d30) Environment: HVM 64bit (Long mode 4 levels)
> (d30) Trap emulation
> (d30) Warning: FEP support not detected - some tests will be skipped
> (d30) Test cpl0: all perms ok
> (d30)   Testing int3
> (d30) Fail redundant: Expected 1 exception (vec 3 at 00000000001032e3), got 2
> (d30)  exlog[00] 0008:00000000001032e2 vec 3[0000]
> (d30)  exlog[01] 0008:00000000001032e3 vec 3[0000]
> (d30)   Testing int $3
> (d30)   Testing icebp
> (d30)   Testing int $1
> (d30)   Testing into
> (d30) Test cpl0: p=0
> (d30)   Testing int3
> (d30)   Testing int $3
> (d30)   Testing icebp
> (d30)   Testing int $1
> (d30)   Testing into
> (d30) Test cpl3: all perms ok
> (d30)   Testing int3
> (d30) Fail redundant: Expected 1 exception (vec 3 at 00000000001032e3), got 2
> (d30)  exlog[00] 0023:00000000001032e2 vec 3[0000]
> (d30)  exlog[01] 0023:00000000001032e3 vec 3[0000]
> (d30)   Testing int $3
> (d30)   Testing icebp
> (d30)   Testing int $1
> (d30)   Testing into
> (d30) Test cpl3: p=0
> (d30)   Testing int3
> (d30)   Testing int $3
> (d30)   Testing icebp
> (d30)   Testing int $1
> (d30)   Testing into
> (d30) Test cpl3: dpl=0
> (d30)   Testing int3
> (d30)   Testing int $3
> (d30)   Testing icebp
> (d30)   Testing int $1
> (d30)   Testing into
> (d30) Test result: FAILURE
>
> So we _should be_ sending the instruction length information along for
> this type of vm_events and it is in fact buggy right now.

On a related note, do emulated instruction get appropriately sent for
introspection?

You can check this by booting a debug hypervisor with "hvm_fep" on the
command line, which will double the number of tests run by this suite.

If they are sent for emulation, you would expect to see some "Fail force
redundant:" issues as well.  I can't think of any codepath offhand which
links the emulation results up to the current introspection paths.

~Andrew

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