Re: [Xen-devel] [PATCH RESEND v1 6/7] x86: Implement Intel Processor Trace MSRs read/write

["Kang, Luwei" <luwei.kang@xxxxxxxxx>]

> >>> On 04.05.18 at 05:53, <luwei.kang@xxxxxxxxx> wrote:
> >> >      Thanks for your clarification. Please correct me if I have
> >> > something wrong. Guest may execute an instruction and this
> >> > instruction may produce an PT packet save in PT output buffer. An
> >> > EPT violation will be generated if the address of this PT buffer
> >> > don't have EPT page table mapping, but this EPT violations
> >> > shouldn't be handled by
> >> > x86_emulate() because it no relate with the execute of this instruction.
> >>
> >> Plus - and that's very important - the EPT violation may be reported
> >> on some
> > later insn.
> >
> > You mean the "later instruction" is some new instruction in future hardware?
> 
> No, I mean an instruction following later in the instruction stream.
> 
> >> >      In that case, can we build the EPT map when set the output
> >> > buffer address (IA32_RTIT_OUTPUT_BASE) and crash the guest if still
> >> > happened EPT violation with Intel PT output buffer read/write exit
> >> > qualification. Or add an exit qualification check before instruction 
> >> > emulation?
> >>
> >> Imo you should add an exit qualification check in any case. Depending
> >> what
> > else you do, finding the new bit set may imply crashing
> >> the domain or doing something more sophisticated.
> >
> > Do you mean add this check at the beginning of any specific "exit_reason"
> > handler in vmx_vmexit_handler() function?
> 
> That depends. Surely not for every exit reason, but only those for which this 
> new bit is valid (iirc exit qualifications differ per exit
> reason anyway, so you can't unilaterally check the same bit everywhere). And 
> even for those where the bit is valid, I'm not sure you
> can decide in the exit handler alone what to do if the bit is set. It may be 
> necessary to propagate the flag down the call tree.
> 
> > Another question is where to build this EPT mapping? Setting
> > IA32_RTIT_OUTPUT_BASE or handled by EPT violation?
> 
> I have no idea - that's more a question for you to answer yourself.
> 

I make a draft patch for VM exit caused by Intel PT output (comments as 
annotation). It looks have little thing to deal with.
There have four case which may cause VM-exit by PT output (spec 5.2.2.1).
1. EPT violations. This because there don't have EPT mapping from GPA to HPA, I 
think this can be handled as usual. About MMIO, I think maybe we can make guest 
OS crash.
2. EPT misconfigurations. We may misconfigure EPT table for log the dirty page 
and MMIO access (Please correct me if have something wrong). I can't find there 
need to have some special need to be handled.
3. PML log-full. PT output may cause vm-exit for PML  log-full when trace 
record to a new page. But it looks don't need additional handle as well.
4. APIC access. Currently, I have no idea about how this relate with PT buffer 
write. When PT buffer is full, an PMI interrupt would be injected to this VM, 
but still have no direct relationship.

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index c23983c..fbf272e 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1873,6 +1873,18 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long 
gla,
          (npfec.write_access &&
           (p2m_is_discard_write(p2mt) || (p2mt == p2m_ioreq_server))) )
     {
+        /*
+         * Don't need emulate and make guest crash when write to
+         * mmio address or a ram address without write permission.
+         * In fact, output buffer can set to be MMIO address (35.2.6.1),
+         * it need the support of a hardware PCI card which use for
+         * collect trace information. I am afraid it initialized to
+         * a valid general MMIO address which is used by a pass through
+         * device.
+         */
+        if ( npfec.pt_output )
+            goto out_put_gfn;
+
         if ( !handle_mmio_with_translation(gla, gpa >> PAGE_SHIFT, npfec) )
             hvm_inject_hw_exception(TRAP_gp_fault, 0);
         rc = 1;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 021cf33..ba4f979 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3249,6 +3249,7 @@ static void ept_handle_violation(ept_qual_t q, paddr_t 
gpa)
         .write_access = q.write,
         .insn_fetch = q.fetch,
         .present = q.eff_read || q.eff_write || q.eff_exec,
+        .pt_output = q.pt_output,
     };

     if ( tb_init_done )
diff --git a/xen/include/asm-x86/hvm/vmx/vmx.h 
b/xen/include/asm-x86/hvm/vmx/vmx.h
index 89619e4..70b6c5f 100644
--- a/xen/include/asm-x86/hvm/vmx/vmx.h
+++ b/xen/include/asm-x86/hvm/vmx/vmx.h
@@ -620,11 +620,13 @@ void vmx_pi_hooks_deassign(struct domain *d);
 typedef union ept_qual {
     unsigned long raw;
     struct {
-        bool read:1, write:1, fetch:1,
+        unsigned long read:1, write:1, fetch:1,
             eff_read:1, eff_write:1, eff_exec:1, /* eff_user_exec */:1,
             gla_valid:1,
-            gla_fault:1; /* Valid iff gla_valid. */
-        unsigned long /* pad */:55;
+            gla_fault:1, /* Valid iff gla_valid. */
+            :7,
+            pt_output:1; /* VM exit caused by Intel PT output */
+        unsigned long /* pad */:47;
     };
 } __transparent__ ept_qual_t;

diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index e928551..221ffc3 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -228,6 +228,7 @@ struct npfec {
     unsigned int present:1;
     unsigned int gla_valid:1;
     unsigned int kind:2;  /* npfec_kind_t */
+    unsigned int ipt_output:1;
 };

 /* memflags: */






> Jan
> 


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