Re: [Xen-devel] [PATCH v5 06/13] x86: simplify guest_has_trap_callback

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jun 27, 2017 at 12:09:14AM -0600, Jan Beulich wrote:
> >>> Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 06/26/17 6:58 PM >>>
> >On 26/06/17 17:28, Wei Liu wrote:
> >> --- a/xen/arch/x86/cpu/mcheck/vmce.c
> >> +++ b/xen/arch/x86/cpu/mcheck/vmce.c
> >> @@ -359,6 +359,15 @@ static int vmce_load_vcpu_ctxt(struct domain *d, 
> >> hvm_domain_context_t *h)
> >>  HVM_REGISTER_SAVE_RESTORE(VMCE_VCPU, vmce_save_vcpu_ctxt,
> >>                            vmce_load_vcpu_ctxt, 1, HVMSR_PER_VCPU);
> >>  
> >> +static inline bool pv_callback_registered(const struct vcpu *v, uint8_t 
> >> vector)
> >> +{
> >> +#ifdef CONFIG_PV
> >> +    return v->arch.pv_vcpu.trap_ctxt[vector].address;
> >> +#else
> >> +    return false;
> >> +#endif
> >> +}
> >> +
> >
> >Isn't there a header file this would be better living in?  Its certainly
> >not vmce-specific.
> 
> We certainly have the equivalent of this check in assembly code (which
> iirc we mean to convert to C eventually), so pv/traps.h would seem to be
> the right place. The function name would need to change a little though,
> as we're talking about exception callbacks here, not the things we call
> callbacks in the public interface. pv_trap_callback_registered() perhaps?

I agree pv/traps.h is the right place and I've rename it to
pv_trap_callback_registered.

I kept Andrew's Rb because those are just cosmetic changes.

---8<---
From d211f6a7aea2e8497bddf72df084f37588234e2d Mon Sep 17 00:00:00 2001
From: Wei Liu <wei.liu2@xxxxxxxxxx>
Date: Fri, 23 Jun 2017 18:34:41 +0100
Subject: [PATCH] x86: simplify guest_has_trap_callback

Simplify that function, rename it to pv_trap_callback_registered and
move it to pv/traps.h. Adjust vmce.c accordingly.

Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
 xen/arch/x86/cpu/mcheck/vmce.c |  3 ++-
 xen/arch/x86/traps.c           | 18 ------------------
 xen/include/asm-x86/pv/traps.h | 11 +++++++++++
 xen/include/asm-x86/traps.h    |  8 --------
 4 files changed, 13 insertions(+), 27 deletions(-)

diff --git a/xen/arch/x86/cpu/mcheck/vmce.c b/xen/arch/x86/cpu/mcheck/vmce.c
index d591d31600..1356f611ab 100644
--- a/xen/arch/x86/cpu/mcheck/vmce.c
+++ b/xen/arch/x86/cpu/mcheck/vmce.c
@@ -32,6 +32,7 @@
 #include <asm/system.h>
 #include <asm/msr.h>
 #include <asm/p2m.h>
+#include <asm/pv/traps.h>
 
 #include "mce.h"
 #include "x86_mca.h"
@@ -383,7 +384,7 @@ int inject_vmce(struct domain *d, int vcpu)
             continue;
 
         if ( (is_hvm_domain(d) ||
-              guest_has_trap_callback(d, v->vcpu_id, TRAP_machine_check)) &&
+              pv_trap_callback_registered(v, TRAP_machine_check)) &&
              !test_and_set_bool(v->mce_pending) )
         {
             mce_printk(MCE_VERBOSE, "MCE: inject vMCE to %pv\n", v);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index c43f3f1095..f3c5de6f2c 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2008,24 +2008,6 @@ long unregister_guest_nmi_callback(void)
     return 0;
 }
 
-int guest_has_trap_callback(struct domain *d, uint16_t vcpuid, unsigned int 
trap_nr)
-{
-    struct vcpu *v;
-    struct trap_info *t;
-
-    BUG_ON(d == NULL);
-    BUG_ON(vcpuid >= d->max_vcpus);
-
-    /* Sanity check - XXX should be more fine grained. */
-    BUG_ON(trap_nr >= NR_VECTORS);
-
-    v = d->vcpu[vcpuid];
-    t = &v->arch.pv_vcpu.trap_ctxt[trap_nr];
-
-    return (t->address != 0);
-}
-
-
 int send_guest_trap(struct domain *d, uint16_t vcpuid, unsigned int trap_nr)
 {
     struct vcpu *v;
diff --git a/xen/include/asm-x86/pv/traps.h b/xen/include/asm-x86/pv/traps.h
index f9d44ab989..c32c38c372 100644
--- a/xen/include/asm-x86/pv/traps.h
+++ b/xen/include/asm-x86/pv/traps.h
@@ -29,12 +29,23 @@ int pv_emulate_privileged_op(struct cpu_user_regs *regs);
 void pv_emulate_gate_op(struct cpu_user_regs *regs);
 bool pv_emulate_invalid_op(struct cpu_user_regs *regs);
 
+static inline bool pv_trap_callback_registered(const struct vcpu *v,
+                                               uint8_t vector)
+{
+    return v->arch.pv_vcpu.trap_ctxt[vector].address;
+}
+
 #else  /* !CONFIG_PV */
 
 static inline int pv_emulate_privileged_op(struct cpu_user_regs *regs) { 
return 0; }
 static inline void pv_emulate_gate_op(struct cpu_user_regs *regs) {}
 static inline bool pv_emulate_invalid_op(struct cpu_user_regs *regs) { return 
true; }
 
+static inline bool pv_trap_callback_registered(const struct vcpu *v,
+                                               uint8_t vector)
+{
+    return false;
+}
 #endif /* CONFIG_PV */
 
 #endif /* __X86_PV_TRAPS_H__ */
diff --git a/xen/include/asm-x86/traps.h b/xen/include/asm-x86/traps.h
index 893b4dbe77..cc1d7d01d9 100644
--- a/xen/include/asm-x86/traps.h
+++ b/xen/include/asm-x86/traps.h
@@ -29,14 +29,6 @@ struct cpu_user_regs;
 
 void async_exception_cleanup(struct vcpu *);
  
-/**
- * guest_has_trap_callback
- *
- * returns true (non-zero) if guest registered a trap handler
- */
-extern int guest_has_trap_callback(struct domain *d, uint16_t vcpuid,
-                               unsigned int trap_nr);
-
 /**
  * send_guest_trap
  *
-- 
2.11.0


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