[PATCH RFC] x86/PV: avoid HVM-copying alternatives in PV-only code

Jan Beulich posted 1 patch 4 days, 6 hours ago
Failed in applying to current master (apply log)
[PATCH RFC] x86/PV: avoid HVM-copying alternatives in PV-only code
Posted by Jan Beulich 4 days, 6 hours ago
raw_copy_*_guest*() expanding to both a HVM and a PV alternative leaves a
lot of unreachable code, violating Misra C:2012 rule 2.1 (without Eclair
being able to spot this). Introduce a mechanism to avoid that in handling
of PV-only hypercalls (i.e. ones which only PV domains can issue and which
only act on PV domains [or which are not domain related]).

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
In my default test build this reduces overall code size by almost 2k, and
that's with PV_SHIM=n.

RFC: Is something like this going to be acceptable at all?

RFC: The HVM-only enabling is just in case; right now it doesn't look as
     if that would actually be usable anywhere.

Overriding is_hvm_vcpu() (and/or is_hvm_domain()) of course has yet better
effects, which pv/shim.c shows particularly well. Yet Misra won't like us
doing so.

--- a/xen/arch/x86/compat.c
+++ b/xen/arch/x86/compat.c
@@ -5,6 +5,8 @@
  * hypercall after doing necessary argument munging.
  */
 
+#define PV_ONLY_SOURCE 1
+
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <xen/trace.h>
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -1262,6 +1262,8 @@ static void cf_check __maybe_unused x86_
 
 #ifdef CONFIG_PV /* do_mca() hypercall is PV-only */
 
+#define PV_ONLY_SOURCE 1
+
 #if BITS_PER_LONG == 64
 
 /* Two layers of casting to cover Misra C:2012 rule 11.2. */
@@ -1666,6 +1668,8 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_m
     return ret;
 }
 
+#undef PV_ONLY_SOURCE
+
 #endif /* CONFIG_PV */
 
 static int mcinfo_dumped;
--- a/xen/arch/x86/include/asm/guest_access.h
+++ b/xen/arch/x86/include/asm/guest_access.h
@@ -14,11 +14,15 @@
 
 /* Raw access functions: no type checking. */
 #define raw_copy_to_guest(dst, src, len)        \
-    (is_hvm_vcpu(current) ?                     \
+    ((IS_ENABLED(HVM_ONLY_SOURCE) ||            \
+      (!IS_ENABLED(PV_ONLY_SOURCE) &&           \
+       is_hvm_vcpu(current))) ?                 \
      copy_to_user_hvm((dst), (src), (len)) :    \
      copy_to_guest_pv(dst, src, len))
 #define raw_copy_from_guest(dst, src, len)      \
-    (is_hvm_vcpu(current) ?                     \
+    ((IS_ENABLED(HVM_ONLY_SOURCE) ||            \
+      (!IS_ENABLED(PV_ONLY_SOURCE) &&           \
+       is_hvm_vcpu(current))) ?                 \
      copy_from_user_hvm((dst), (src), (len)) :  \
      copy_from_guest_pv(dst, src, len))
 #define raw_clear_guest(dst,  len)              \
@@ -26,11 +30,15 @@
      clear_user_hvm((dst), (len)) :             \
      clear_guest_pv(dst, len))
 #define __raw_copy_to_guest(dst, src, len)      \
-    (is_hvm_vcpu(current) ?                     \
+    ((IS_ENABLED(HVM_ONLY_SOURCE) ||            \
+      (!IS_ENABLED(PV_ONLY_SOURCE) &&           \
+       is_hvm_vcpu(current))) ?                 \
      copy_to_user_hvm((dst), (src), (len)) :    \
      __copy_to_guest_pv(dst, src, len))
 #define __raw_copy_from_guest(dst, src, len)    \
-    (is_hvm_vcpu(current) ?                     \
+    ((IS_ENABLED(HVM_ONLY_SOURCE) ||            \
+      (!IS_ENABLED(PV_ONLY_SOURCE) &&           \
+       is_hvm_vcpu(current))) ?                 \
      copy_from_user_hvm((dst), (src), (len)) :  \
      __copy_from_guest_pv(dst, src, len))
 
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -3407,6 +3407,9 @@ int new_guest_cr3(mfn_t mfn)
 #endif
 
 #ifdef CONFIG_PV
+
+#define PV_ONLY_SOURCE 1
+
 static int vcpumask_to_pcpumask(
     struct domain *d, XEN_GUEST_HANDLE_PARAM(const_void) bmap, cpumask_t *pmask)
 {
@@ -3447,6 +3450,8 @@ static int vcpumask_to_pcpumask(
     }
 }
 
+#undef PV_ONLY_SOURCE
+
 long do_mmuext_op(
     XEN_GUEST_HANDLE_PARAM(mmuext_op_t) uops,
     unsigned int count,
@@ -3983,6 +3988,8 @@ long do_mmuext_op(
     return rc;
 }
 
+#define PV_ONLY_SOURCE 1
+
 long do_mmu_update(
     XEN_GUEST_HANDLE_PARAM(mmu_update_t) ureqs,
     unsigned int count,
@@ -4361,6 +4364,9 @@ long do_mmu_update(
 
     return rc;
 }
+
+#undef PV_ONLY_SOURCE
+
 #endif /* CONFIG_PV */
 
 /*
--- a/xen/arch/x86/pv/callback.c
+++ b/xen/arch/x86/pv/callback.c
@@ -5,6 +5,8 @@
  * hypercall handles and helper functions for guest callback
  */
 
+#define PV_ONLY_SOURCE 1
+
 #include <xen/event.h>
 #include <xen/hypercall.h>
 #include <xen/guest_access.h>
--- a/xen/arch/x86/pv/descriptor-tables.c
+++ b/xen/arch/x86/pv/descriptor-tables.c
@@ -8,6 +8,8 @@
  * Copyright (c) 2004 Christian Limpach
  */
 
+#define PV_ONLY_SOURCE 1
+
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
 
--- a/xen/arch/x86/pv/shim.c
+++ b/xen/arch/x86/pv/shim.c
@@ -6,6 +6,9 @@
  *
  * Copyright (c) 2017 Citrix Systems Ltd.
  */
+
+#define PV_ONLY_SOURCE 1
+
 #include <xen/event.h>
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -6,6 +6,8 @@
  * - Magnus Damm <magnus@valinux.co.jp>
  */
 
+#define PV_ONLY_SOURCE 1 /* Relevant only for x86. */
+
 #include <xen/init.h>
 #include <xen/lib.h>
 #include <xen/acpi.h>