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

Jan Beulich <[email protected]>
Newsgroups org.xenproject.lists.xen-devel
Message-ID <[email protected]>
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 <[email protected]>
---
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. */
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.