Re: [Xen-devel] [PATCH v2] hvm/svm: Implement Debug events

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Tue, Mar 20, 2018 at 11:07 AM, Alexandru Stefan ISAILA
<aisaila@xxxxxxxxxxxxxxx> wrote:
> On Ma, 2018-03-20 at 10:49 -0600, Tamas K Lengyel wrote:
>> On Tue, Mar 20, 2018 at 3:40 AM, Alexandru Isaila
>> <aisaila@xxxxxxxxxxxxxxx> wrote:
>> >
>> > At this moment the Debug events for the AMD architecture are not
>> > forwarded to the monitor layer.
>> >
>> > This patch adds the Debug event to the common capabilities, adds
>> > the VMEXIT_ICEBP then forwards the event to the monitor layer.
>> >
>> > Chapter 2: SVM Processor and Platform Extensions: "Note: A vector 1
>> > exception generated by the single byte INT1
>> > instruction (also known as ICEBP) does not trigger the #DB
>> > intercept. Software should use the dedicated ICEBP
>> > intercept to intercept ICEBP"
>> >
>> > ---
>> > Changes since V1:
>> >         - Get inst_len from __get_instruction_length()
>> >         - Updated __get_instruction_length() for the INSTR_ICEBP
>> >           instruction
>> >
>> > Signed-off-by: Alexandru Isaila <aisaila@xxxxxxxxxxxxxxx>
>> > ---
>> >  xen/arch/x86/hvm/svm/emulate.c        |  1 +
>> >  xen/arch/x86/hvm/svm/svm.c            | 37
>> > +++++++++++++++++++++++++----------
>> >  xen/arch/x86/hvm/svm/vmcb.c           |  2 +-
>> >  xen/include/asm-x86/hvm/svm/emulate.h |  1 +
>> >  xen/include/asm-x86/monitor.h         |  4 ++--
>> >  5 files changed, 32 insertions(+), 13 deletions(-)
>> >
>> > diff --git a/xen/arch/x86/hvm/svm/emulate.c
>> > b/xen/arch/x86/hvm/svm/emulate.c
>> > index e1a1581..172369e 100644
>> > --- a/xen/arch/x86/hvm/svm/emulate.c
>> > +++ b/xen/arch/x86/hvm/svm/emulate.c
>> > @@ -80,6 +80,7 @@ static const struct {
>> >      [INSTR_RDTSC]   = { X86EMUL_OPC(0x0f, 0x31) },
>> >      [INSTR_RDMSR]   = { X86EMUL_OPC(0x0f, 0x32) },
>> >      [INSTR_CPUID]   = { X86EMUL_OPC(0x0f, 0xa2) },
>> > +    [INSTR_ICEBP]   = { X86EMUL_OPC(   0, 0xf1) },
>> >  };
>> >
>> >  int __get_instruction_length_from_list(struct vcpu *v,
>> > diff --git a/xen/arch/x86/hvm/svm/svm.c
>> > b/xen/arch/x86/hvm/svm/svm.c
>> > index c34f5b5..d4f2290 100644
>> > --- a/xen/arch/x86/hvm/svm/svm.c
>> > +++ b/xen/arch/x86/hvm/svm/svm.c
>> > @@ -1109,7 +1109,8 @@ static void noreturn svm_do_resume(struct
>> > vcpu *v)
>> >  {
>> >      struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
>> >      bool debug_state = (v->domain->debugger_attached ||
>> > -                        v->domain-
>> > >arch.monitor.software_breakpoint_enabled);
>> > +                        v->domain-
>> > >arch.monitor.software_breakpoint_enabled ||
>> > +                        v->domain-
>> > >arch.monitor.debug_exception_enabled);
>> Please explain the rationale why this is included under
>> "debug_exception" and not "software_breakpoint".
>>
>> Thanks,
>> Tamas
>>
> Hi Tamas,
>
> I've included the icebp under the debug_exception only to have
> consistency with the vmx side. I saw this behavior after running
> the swint-emulation from xtf.
>

Sounds good, as long as we are consistent it's good.

Thanks,
Tamas

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel