[PATCH 7/7] gnttab: unreachable code when GNTTAB_MAX_VERSION < 2
Jan Beulich <[email protected]> Tue, 28 Jul 2026 15:53:06 +0200
| Newsgroups | gmane.comp.emulators.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(>->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. */