diff mbox series

[3/5] x86/p2m: suppress audit_p2m hook when possible

Message ID 722cf75e-da6a-49c5-472a-898796c9030e@suse.com (mailing list archive)
State Superseded
Headers show
Series x86/p2m: hook adjustments | expand

Commit Message

Jan Beulich Oct. 28, 2020, 9:23 a.m. UTC 
When P2M_AUDIT is false, it's unused, so instead of having a dangling
NULL pointer sit there, omit the field altogether.

Instead of adding "#if P2M_AUDIT && defined(CONFIG_HVM)" in even more
places, fold the latter part right into the definition of P2M_AUDIT.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

Comments

Roger Pau Monné Nov. 10, 2020, 11:30 a.m. UTC | #1 
On Wed, Oct 28, 2020 at 10:23:42AM +0100, Jan Beulich wrote:
> When P2M_AUDIT is false, it's unused, so instead of having a dangling
> NULL pointer sit there, omit the field altogether.
> 
> Instead of adding "#if P2M_AUDIT && defined(CONFIG_HVM)" in even more
> places, fold the latter part right into the definition of P2M_AUDIT.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
Jan Beulich Nov. 10, 2020, 1:21 p.m. UTC | #2 
On 10.11.2020 12:30, Roger Pau Monné wrote:
> On Wed, Oct 28, 2020 at 10:23:42AM +0100, Jan Beulich wrote:
>> When P2M_AUDIT is false, it's unused, so instead of having a dangling
>> NULL pointer sit there, omit the field altogether.
>>
>> Instead of adding "#if P2M_AUDIT && defined(CONFIG_HVM)" in even more
>> places, fold the latter part right into the definition of P2M_AUDIT.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks. Since I see you keep replying to v1, are you aware of
there being v2? For this particular patch there actually is a
clang related fix in v2, which may be of interest to you.

Jan
Roger Pau Monné Nov. 10, 2020, 2:01 p.m. UTC | #3 
On Tue, Nov 10, 2020 at 02:21:43PM +0100, Jan Beulich wrote:
> On 10.11.2020 12:30, Roger Pau Monné wrote:
> > On Wed, Oct 28, 2020 at 10:23:42AM +0100, Jan Beulich wrote:
> >> When P2M_AUDIT is false, it's unused, so instead of having a dangling
> >> NULL pointer sit there, omit the field altogether.
> >>
> >> Instead of adding "#if P2M_AUDIT && defined(CONFIG_HVM)" in even more
> >> places, fold the latter part right into the definition of P2M_AUDIT.
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Acked-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Thanks. Since I see you keep replying to v1, are you aware of
> there being v2? For this particular patch there actually is a
> clang related fix in v2, which may be of interest to you.

Urg, no sorry. I was just going over my mail queue and didn't realize
there was a v2. Will take a look.

Roger.
diff mbox series

Patch

--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1012,7 +1012,7 @@  long arch_do_domctl(
         break;
 #endif
 
-#if P2M_AUDIT && defined(CONFIG_HVM)
+#if P2M_AUDIT
     case XEN_DOMCTL_audit_p2m:
         if ( d == currd )
             ret = -EPERM;
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -1260,7 +1260,9 @@  int ept_p2m_init(struct p2m_domain *p2m)
     p2m->change_entry_type_global = ept_change_entry_type_global;
     p2m->change_entry_type_range = ept_change_entry_type_range;
     p2m->memory_type_changed = ept_memory_type_changed;
+#if P2M_AUDIT
     p2m->audit_p2m = NULL;
+#endif
     p2m->tlb_flush = ept_tlb_flush;
 
     /* Set the memory type used when accessing EPT paging structures. */
--- a/xen/arch/x86/mm/p2m-pt.c
+++ b/xen/arch/x86/mm/p2m-pt.c
@@ -971,8 +971,8 @@  static int p2m_pt_change_entry_type_rang
     return err;
 }
 
-#if P2M_AUDIT && defined(CONFIG_HVM)
-long p2m_pt_audit_p2m(struct p2m_domain *p2m)
+#if P2M_AUDIT
+static long p2m_pt_audit_p2m(struct p2m_domain *p2m)
 {
     unsigned long entry_count = 0, pmbad = 0;
     unsigned long mfn, gfn, m2pfn;
@@ -1120,8 +1120,6 @@  long p2m_pt_audit_p2m(struct p2m_domain
 
     return pmbad;
 }
-#else
-# define p2m_pt_audit_p2m NULL
 #endif /* P2M_AUDIT */
 
 /* Set up the p2m function pointers for pagetable format */
@@ -1141,8 +1139,6 @@  void p2m_pt_init(struct p2m_domain *p2m)
 
 #if P2M_AUDIT
     p2m->audit_p2m = p2m_pt_audit_p2m;
-#else
-    p2m->audit_p2m = NULL;
 #endif
 }
 
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -2435,7 +2435,7 @@  int p2m_altp2m_propagate_change(struct d
 
 /*** Audit ***/
 
-#if P2M_AUDIT && defined(CONFIG_HVM)
+#if P2M_AUDIT
 void audit_p2m(struct domain *d,
                uint64_t *orphans,
                 uint64_t *m2p_bad,
--- a/xen/include/asm-x86/p2m.h
+++ b/xen/include/asm-x86/p2m.h
@@ -31,6 +31,14 @@ 
 #include <asm/mem_sharing.h>
 #include <asm/page.h>    /* for pagetable_t */
 
+/* Debugging and auditing of the P2M code? */
+#ifndef NDEBUG
+#define P2M_AUDIT     defined(CONFIG_HVM)
+#else
+#define P2M_AUDIT     0
+#endif
+#define P2M_DEBUGGING 0
+
 extern bool_t opt_hap_1gb, opt_hap_2mb;
 
 /*
@@ -268,7 +276,9 @@  struct p2m_domain {
     int                (*write_p2m_entry)(struct p2m_domain *p2m,
                                           unsigned long gfn, l1_pgentry_t *p,
                                           l1_pgentry_t new, unsigned int level);
+#if P2M_AUDIT
     long               (*audit_p2m)(struct p2m_domain *p2m);
+#endif
 
     /*
      * P2M updates may require TLBs to be flushed (invalidated).
@@ -758,14 +768,6 @@  extern void p2m_pt_init(struct p2m_domai
 void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn, mfn_t *mfn,
                      p2m_query_t q, uint32_t *pfec);
 
-/* Debugging and auditing of the P2M code? */
-#ifndef NDEBUG
-#define P2M_AUDIT     1
-#else
-#define P2M_AUDIT     0
-#endif
-#define P2M_DEBUGGING 0
-
 #if P2M_AUDIT
 extern void audit_p2m(struct domain *d,
                       uint64_t *orphans,