[PATCH 7/7] gnttab: unreachable code when GNTTAB_MAX_VERSION < 2

[Jan Beulich <jbeulich@xxxxxxxx>]

I'm surprised Eclair doesn't spot the large chunks of unreachable code on
Arm, i.e. violations of Misra C:2012 rule 2.1.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
This pretty certainly isn't dealing with everything. For example, with
another helper the gt_version field could likely also become conditional.
With some more effort the nr_status_frames field similarly could become
conditional.

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -72,8 +72,10 @@ struct grant_table {
     /* Number of grant status frames shared with guest (for version 2) */
     unsigned int          nr_status_frames;
 
+#if GNTTAB_MAX_VERSION >= 2
     /* Number of version 2 operations in progress. */
     atomic_t              nr_v2_ops;
+#endif /* GNTTAB_MAX_VERSION >= 2 */
 
     /*
      * Number of available maptrack entries.  For cleanup purposes it is
@@ -182,12 +184,10 @@ static int cf_check parse_gnttab_max_map
                               opt_max_maptrack_frames_val);
 }
 
-#ifndef GNTTAB_MAX_VERSION
-#define GNTTAB_MAX_VERSION 2
-#endif
-
+#if GNTTAB_MAX_VERSION >= 2
 unsigned int __read_mostly opt_gnttab_max_version = GNTTAB_MAX_VERSION;
 static bool __read_mostly opt_transitive_grants = true;
+#endif
 #ifdef CONFIG_PV
 static bool __ro_after_init opt_grant_transfer = true;
 #else
@@ -196,17 +196,24 @@ static bool __ro_after_init opt_grant_tr
 
 static int __init cf_check parse_gnttab(const char *s)
 {
-    const char *ss, *e;
-    int val, rc = 0;
+    const char *ss;
+    int rc = 0;
 
     do {
+        int val;
+
         ss = strchr(s, ',');
         if ( !ss )
             ss = strchr(s, '\0');
 
-        if ( !strncmp(s, "max-ver:", 8) ||
-             !strncmp(s, "max_ver:", 8) ) /* Alias for original XSA-226 patch 
*/
+        if ( false )
+            (void)val; /* Nothing. */
+#ifndef opt_gnttab_max_version
+        else if ( !strncmp(s, "max-ver:", 8) ||
+                  /* Alias for original XSA-226 patch */
+                  !strncmp(s, "max_ver:", 8) )
         {
+            const char *e;
             long ver = simple_strtol(s + 8, &e, 10);
 
             if ( e == ss && ver >= 1 && ver <= 2 )
@@ -216,6 +223,7 @@ static int __init cf_check parse_gnttab(
         }
         else if ( (val = parse_boolean("transitive", s, ss)) >= 0 )
             opt_transitive_grants = val;
+#endif
 #ifndef opt_grant_transfer
         else if ( (val = parse_boolean("transfer", s, ss)) >= 0 )
             opt_grant_transfer = val;
@@ -330,10 +338,12 @@ shared_entry_header(struct grant_table *
         block_speculation();
         return (grant_entry_header_t*)&shared_entry_v1(t, ref);
 
+#if GNTTAB_MAX_VERSION >= 2
     case 2:
         /* Returned values should be independent of speculative execution */
         block_speculation();
         return &shared_entry_v2(t, ref).hdr;
+#endif
     }
 
     ASSERT_UNREACHABLE();
@@ -378,6 +388,9 @@ struct active_grant_entry {
 })
 
     domid_t       domid;  /* Domain being granted access.             */
+
+#if GNTTAB_MAX_VERSION >= 2
+
     domid_t       src_domid; /* Original domain granting access.      */
     unsigned int  start:15; /* For sub-page grants, the start offset
                                in the page.                           */
@@ -385,6 +398,9 @@ struct active_grant_entry {
     unsigned int  length:16; /* For sub-page grants, the length of the
                                 grant.                                */
     grant_ref_t   trans_gref;
+
+#endif /* GNTTAB_MAX_VERSION < 2 */
+
     mfn_t         mfn;    /* Machine frame being granted.             */
 #ifndef NDEBUG
     gfn_t         gfn;    /* Guest's idea of the frame being granted. */
@@ -405,6 +421,15 @@ static inline void act_set_gfn(struct ac
 #endif
 }
 
+static bool act_is_sub_page(const struct active_grant_entry *act)
+{
+#if GNTTAB_MAX_VERSION >= 2
+    return act->is_sub_page;
+#else
+    return false;
+#endif
+}
+
 static DEFINE_PERCPU_RWLOCK_GLOBAL(grant_rwlock);
 
 static always_inline void grant_read_lock(struct grant_table *gt)
@@ -724,6 +749,7 @@ static unsigned int nr_grant_entries(str
         block_speculation();
         return f2e(nr_grant_frames(gt), 1);
 
+#if GNTTAB_MAX_VERSION >= 2
     case 2:
         BUILD_BUG_ON(f2e(INITIAL_NR_GRANT_FRAMES, 2) <
                      GNTTAB_NR_RESERVED_ENTRIES);
@@ -731,6 +757,7 @@ static unsigned int nr_grant_entries(str
         /* Make sure we return a value independently of speculative execution 
*/
         block_speculation();
         return f2e(nr_grant_frames(gt), 2);
+#endif /* GNTTAB_MAX_VERSION >= 2 */
 #undef f2e
     }
 
@@ -919,7 +946,7 @@ static int _set_status(const grant_entry
                        domid_t ldomid)
 {
 
-    if ( evaluate_nospec(rgt_version == 1) )
+    if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(rgt_version == 1) )
         return _set_status_v1(shah, rd, act, readonly, mapflag, ldomid);
     else
         return _set_status_v2(shah, status, rd, act, readonly, mapflag, 
ldomid);
@@ -1089,17 +1116,18 @@ map_grant_ref(
     if ( act->pin &&
          ((act->domid != ld->domain_id) ||
           (act->pin & GNTPIN_incr2oflow_mask(pin_incr)) ||
-          (act->is_sub_page)) )
+          act_is_sub_page(act)) )
     {
         gdprintk(XENLOG_WARNING,
                  "Bad domain (%d != %d), or risk of counter overflow %08x, or 
subpage %d\n",
-                 act->domid, ld->domain_id, act->pin, act->is_sub_page);
+                 act->domid, ld->domain_id, act->pin, act_is_sub_page(act));
         rc = GNTST_general_error;
         goto act_release_out;
     }
 
     /* Make sure we do not access memory speculatively */
-    status = evaluate_nospec(rgt->gt_version == 1) ? &shah->flags
+    status = GNTTAB_MAX_VERSION < 2 ||
+             evaluate_nospec(rgt->gt_version == 1) ? &shah->flags
                                                    : &status_entry(rgt, ref);
 
     if ( !act->pin ||
@@ -1113,9 +1141,10 @@ map_grant_ref(
 
         if ( !act->pin )
         {
-            unsigned long gfn = evaluate_nospec(rgt->gt_version == 1) ?
-                                shared_entry_v1(rgt, ref).frame :
-                                shared_entry_v2(rgt, ref).full_page.frame;
+            unsigned long gfn = GNTTAB_MAX_VERSION < 2 ||
+                                evaluate_nospec(rgt->gt_version == 1)
+                                ? shared_entry_v1(rgt, ref).frame
+                                : shared_entry_v2(rgt, ref).full_page.frame;
 
             rc = get_paged_frame(gfn, &mfn, &pg,
                                  op->flags & GNTMAP_readonly, rd);
@@ -1124,11 +1153,13 @@ map_grant_ref(
             act_set_gfn(act, _gfn(gfn));
             act->domid = ld->domain_id;
             act->mfn = mfn;
+#if GNTTAB_MAX_VERSION >= 2
             act->start = 0;
             act->length = PAGE_SIZE;
             act->is_sub_page = false;
             act->src_domid = rd->domain_id;
             act->trans_gref = ref;
+#endif /* GNTTAB_MAX_VERSION >= 2 */
         }
     }
 
@@ -1350,7 +1381,8 @@ map_grant_ref(
 
     grant_read_lock(rgt);
 
-    if ( unlikely(evaluate_nospec((rgt->gt_version == 1) !=
+    if ( GNTTAB_MAX_VERSION >= 2 &&
+         unlikely(evaluate_nospec((rgt->gt_version == 1) !=
                                   (status == &shah->flags))) )
     {
         /*
@@ -1625,7 +1657,7 @@ unmap_common_complete(struct gnttab_unma
 
     act = active_entry_acquire(rgt, op->ref);
 
-    if ( evaluate_nospec(rgt->gt_version == 1) )
+    if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(rgt->gt_version == 1) )
         status = &shared_entry_v1(rgt, op->ref).flags;
     else if ( evaluate_nospec(op->ref < nr_grant_entries(rgt)) )
         status = &status_entry(rgt, op->ref);
@@ -1947,7 +1979,7 @@ gnttab_grow_table(struct domain *d, unsi
     }
 
     /* Status pages - version 2 */
-    if ( evaluate_nospec(gt->gt_version > 1) )
+    if ( GNTTAB_MAX_VERSION >= 2 && evaluate_nospec(gt->gt_version > 1) )
     {
         if ( gnttab_populate_status_frames(d, gt, req_nr_frames) )
             goto shared_alloc_failed;
@@ -2122,7 +2154,7 @@ gnttab_setup_table(
     }
 
     if ( (op.nr_frames > nr_grant_frames(gt) ||
-          ((gt->gt_version > 1) &&
+          ((GNTTAB_MAX_VERSION >= 2) && (gt->gt_version > 1) &&
            (grant_to_status_frames(op.nr_frames) > nr_status_frames(gt)))) &&
          gnttab_grow_table(d, op.nr_frames) )
     {
@@ -2472,7 +2504,8 @@ gnttab_transfer(
         grant_read_lock(e->grant_table);
         act = active_entry_acquire(e->grant_table, gop.ref);
 
-        if ( unlikely(evaluate_nospec(e->grant_table->gt_version != ver)) )
+        if ( GNTTAB_MAX_VERSION >= 2 &&
+             unlikely(evaluate_nospec(e->grant_table->gt_version != ver)) )
         {
             rc = -EILSEQ;
             goto release;
@@ -2538,12 +2571,13 @@ release_grant_for_copy(
     act = active_entry_acquire(rgt, gref);
     mfn = act->mfn;
 
-    if ( evaluate_nospec(rgt->gt_version == 1) )
+    if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(rgt->gt_version == 1) )
     {
         status = &shared_entry_v1(rgt, gref).flags;
         td = rd;
         trans_gref = gref;
     }
+#if GNTTAB_MAX_VERSION >= 2
     else
     {
         if ( evaluate_nospec(gref < nr_grant_entries(rgt)) )
@@ -2552,6 +2586,7 @@ release_grant_for_copy(
              ? rd : knownalive_domain_from_domid(act->src_domid);
         trans_gref = act->trans_gref;
     }
+#endif /* GNTTAB_MAX_VERSION >= 2 */
 
     if ( readonly )
     {
@@ -2566,13 +2601,15 @@ release_grant_for_copy(
 
     reduce_status_for_pin(rd, act, status, readonly);
 
+#if GNTTAB_MAX_VERSION >= 2
     if ( !act->pin && act->is_sub_page )
         atomic_dec(&rgt->nr_v2_ops);
+#endif
 
     active_entry_release(act);
     grant_read_unlock(rgt);
 
-    if ( td != rd )
+    if ( GNTTAB_MAX_VERSION >= 2 && td != rd )
     {
         /*
          * Recursive call, but it is bounded (acquire permits only a single
@@ -2593,8 +2630,11 @@ release_grant_for_copy(
 static int
 acquire_grant_for_copy(
     struct domain *rd, grant_ref_t gref, domid_t ldom, bool readonly,
-    mfn_t *mfn, struct page_info **page, uint16_t *page_off,
-    uint16_t *length, bool allow_transitive)
+    mfn_t *mfn, struct page_info **page
+#if GNTTAB_MAX_VERSION >= 2
+    , uint16_t *page_off, uint16_t *length, bool allow_transitive
+#endif
+    )
 {
     struct grant_table *rgt = rd->grant_table;
     grant_entry_v2_t *sha2;
@@ -2637,7 +2677,7 @@ acquire_grant_for_copy(
         goto unlock_out;
     }
 
-    if ( evaluate_nospec(rgt->gt_version == 1) )
+    if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(rgt->gt_version == 1) )
     {
         sha2 = NULL;
         status = &shah->flags;
@@ -2652,6 +2692,7 @@ acquire_grant_for_copy(
     barrier();
 
     old_pin = act->pin;
+#if GNTTAB_MAX_VERSION >= 2
     if ( sha2 && (shflags & GTF_type_mask) == GTF_transitive )
     {
         domid_t trans_domid;
@@ -2786,8 +2827,10 @@ acquire_grant_for_copy(
         else
             atomic_dec(&rgt->nr_v2_ops);
     }
-    else if ( !old_pin ||
-              (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
+    else
+#endif /* GNTTAB_MAX_VERSION >= 2 */
+    if ( !old_pin ||
+         (!readonly && !(old_pin & (GNTPIN_devw_mask|GNTPIN_hstw_mask))) )
     {
         unsigned long gfn;
 
@@ -2828,23 +2871,29 @@ acquire_grant_for_copy(
         if ( !act->pin )
         {
             act->domid = ldom;
+            act->mfn = grant_mfn;
+
+#if GNTTAB_MAX_VERSION >= 2
             act->is_sub_page = is_sub_page;
             act->start = trans_page_off;
             act->length = trans_length;
             act->src_domid = rd->domain_id;
             act->trans_gref = gref;
-            act->mfn = grant_mfn;
 
             if ( is_sub_page )
                 atomic_inc(&rgt->nr_v2_ops);
+#endif /* GNTTAB_MAX_VERSION >= 2 */
         }
         else if ( !mfn_eq(act->mfn, grant_mfn) ||
+#if GNTTAB_MAX_VERSION >= 2
                   act->src_domid != rd->domain_id ||
                   act->trans_gref != gref ||
                   (act->is_sub_page &&
                    (!is_sub_page ||
                     act->start != trans_page_off ||
-                    act->length != trans_length)) )
+                    act->length != trans_length)) ||
+#endif /* GNTTAB_MAX_VERSION >= 2 */
+                  false )
         {
             put_page(*page);
             *page = NULL;
@@ -2881,8 +2930,10 @@ acquire_grant_for_copy(
 
     act->pin += pin_incr;
 
+#if GNTTAB_MAX_VERSION >= 2
     *page_off = act->start;
     *length = act->length;
+#endif
     *mfn = act->mfn;
 
     active_entry_release(act);
@@ -3014,13 +3065,20 @@ static int gnttab_copy_claim_buf(const s
         rc = acquire_grant_for_copy(buf->domain, ptr->u.ref,
                                     current->domain->domain_id,
                                     buf->read_only,
-                                    &buf->mfn, &buf->page,
-                                    &buf->ptr.offset, &buf->len,
-                                    opt_transitive_grants);
+                                    &buf->mfn, &buf->page
+#if GNTTAB_MAX_VERSION >= 2
+                                    , &buf->ptr.offset, &buf->len,
+                                    opt_transitive_grants
+#endif
+                                    );
         if ( rc != GNTST_okay )
             goto out;
         buf->ptr.u.ref = ptr->u.ref;
         buf->have_grant = 1;
+#if GNTTAB_MAX_VERSION < 2
+        buf->ptr.offset = 0;
+        buf->len = PAGE_SIZE;
+#endif
     }
     else
     {
@@ -3241,6 +3299,12 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
     if ( gt->gt_version == op.version )
         goto out_unlock;
 
+    if ( GNTTAB_MAX_VERSION < 2 )
+    {
+        res = -EOPNOTSUPP;
+        goto out_unlock;
+    }
+
     /*
      * Make sure that the grant table isn't currently in use when we
      * change the version number, except for the first 8 entries which
@@ -3270,6 +3334,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
         break;
 
     case 2:
+#if GNTTAB_MAX_VERSION >= 2
         if ( atomic_read(&gt->nr_v2_ops) )
         {
             gdprintk(XENLOG_WARNING,
@@ -3278,6 +3343,7 @@ gnttab_set_version(XEN_GUEST_HANDLE_PARA
             res = -EAGAIN;
             goto out_unlock;
         }
+#endif /* GNTTAB_MAX_VERSION >= 2 */
 
         for ( i = 0; i < GNTTAB_NR_RESERVED_ENTRIES; i++ )
         {
@@ -3527,7 +3593,7 @@ swap_grant_ref(grant_ref_t ref_a, grant_
         goto out;
     }
 
-    if ( evaluate_nospec(gt->gt_version == 1) )
+    if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(gt->gt_version == 1) )
     {
         grant_entry_v1_t shared;
 
@@ -3945,7 +4011,7 @@ int gnttab_release_mappings(struct domai
 
         act = active_entry_acquire(rgt, ref);
         sha = shared_entry_header(rgt, ref);
-        if ( rgt->gt_version == 1 )
+        if ( GNTTAB_MAX_VERSION < 2 || rgt->gt_version == 1 )
             status = &sha->flags;
         else
             status = &status_entry(rgt, ref);
@@ -4120,7 +4186,7 @@ int mem_sharing_gref_to_gfn(struct grant
         rc = -EINVAL;
     else if ( ref >= nr_grant_entries(gt) )
         rc = -ENOENT;
-    else if ( evaluate_nospec(gt->gt_version == 1) )
+    else if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(gt->gt_version == 1) )
     {
         const grant_entry_v1_t *sha1 = &shared_entry_v1(gt, ref);
 
@@ -4144,7 +4210,7 @@ int mem_sharing_gref_to_gfn(struct grant
         rc = -ENXIO;
     else if ( !rc && status )
     {
-        if ( evaluate_nospec(gt->gt_version == 1) )
+        if ( GNTTAB_MAX_VERSION < 2 || evaluate_nospec(gt->gt_version == 1) )
             *status = flags;
         else
             *status = status_entry(gt, ref);
@@ -4275,7 +4341,7 @@ int gnttab_acquire_resource(
         break;
 
     case XENMEM_resource_grant_table_id_status:
-        if ( gt->gt_version != 2 )
+        if ( GNTTAB_MAX_VERSION < 2 || gt->gt_version != 2 )
             break;
 
         /* Check that void ** is a suitable representation for gt->status. */
@@ -4327,7 +4393,8 @@ int gnttab_map_frame_begin(
 
     grant_write_lock(gt);
 
-    if ( evaluate_nospec(gt->gt_version == 2) && (idx & 
XENMAPIDX_grant_table_status) )
+    if ( GNTTAB_MAX_VERSION >= 2 && evaluate_nospec(gt->gt_version == 2) &&
+         (idx & XENMAPIDX_grant_table_status) )
     {
         idx &= ~XENMAPIDX_grant_table_status;
         rc = gnttab_get_status_frame_mfn(d, idx, mfn);
@@ -4395,7 +4462,7 @@ static void gnttab_usage_print(struct do
 
         sha = shared_entry_header(gt, ref);
 
-        if ( gt->gt_version == 1 )
+        if ( GNTTAB_MAX_VERSION < 2 || gt->gt_version == 1 )
         {
             status = sha->flags;
             frame = shared_entry_v1(gt, ref).frame;
--- a/xen/include/xen/grant_table.h
+++ b/xen/include/xen/grant_table.h
@@ -39,7 +39,16 @@ void gnttab_seed_entry(const struct doma
 
 #ifdef CONFIG_GRANT_TABLE
 
+#ifndef GNTTAB_MAX_VERSION
+#define GNTTAB_MAX_VERSION 2
+#endif
+
+#if GNTTAB_MAX_VERSION >= 2
 extern unsigned int opt_gnttab_max_version;
+#else
+# define opt_gnttab_max_version GNTTAB_MAX_VERSION
+#endif
+
 extern unsigned int opt_max_grant_frames;
 
 /* Create/destroy per-domain grant table context. */