[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] xen/grant: Purge PIN_FAIL()
commit 1512a68721620338daf477bf717d23befbb4faf5 Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> AuthorDate: Tue Jun 13 17:25:42 2023 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Thu Jun 15 10:51:57 2023 +0100 xen/grant: Purge PIN_FAIL() The name PIN_FAIL() is poor; it's not used only for pinning failures. More importantly, it interferes with code legibility by hiding control flow. Expand and drop it. * Drop redundant "rc = rc" assignment * Rework gnttab_copy_buf() to be simpler by dropping the rc variable As a side effect, this fixes several violations of MISRA rule 2.1 (dead code - the while() following a goto). No functional change. Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> --- xen/common/grant_table.c | 154 ++++++++++++++++++++++++++++++++++------------- 1 file changed, 111 insertions(+), 43 deletions(-) diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index d87e58a53d..89b7811c51 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -270,13 +270,6 @@ struct gnttab_unmap_common { #define GNTTAB_UNMAP_BATCH_SIZE 32 -#define PIN_FAIL(_lbl, _rc, _f, _a...) \ - do { \ - gdprintk(XENLOG_WARNING, _f, ## _a ); \ - rc = (_rc); \ - goto _lbl; \ - } while ( 0 ) - /* * Tracks a mapping of another domain's grant reference. Each domain has a * table of these, indexes into which are returned as a 'mapping handle'. @@ -785,9 +778,13 @@ static int _set_status_v1(const grant_entry_header_t *shah, /* If not already pinned, check the grant domid and type. */ if ( !act->pin && (((scombo.flags & mask) != GTF_permit_access) || (scombo.domid != ldomid)) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad flags (%x) or dom (%d); expected d%d\n", scombo.flags, scombo.domid, ldomid); + rc = GNTST_general_error; + goto done; + } new = scombo; new.flags |= GTF_reading; @@ -796,8 +793,12 @@ static int _set_status_v1(const grant_entry_header_t *shah, { new.flags |= GTF_writing; if ( unlikely(scombo.flags & GTF_readonly) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Attempt to write-pin a r/o grant entry\n"); + rc = GNTST_general_error; + goto done; + } } prev.raw = guest_cmpxchg(rd, raw_shah, scombo.raw, new.raw); @@ -805,8 +806,11 @@ static int _set_status_v1(const grant_entry_header_t *shah, break; if ( retries++ == 4 ) - PIN_FAIL(done, GNTST_general_error, - "Shared grant entry is unstable\n"); + { + gdprintk(XENLOG_WARNING, "Shared grant entry is unstable\n"); + rc = GNTST_general_error; + goto done; + } scombo = prev; } @@ -840,9 +844,13 @@ static int _set_status_v2(const grant_entry_header_t *shah, ((((scombo.flags & mask) != GTF_permit_access) && (mapflag || ((scombo.flags & mask) != GTF_transitive))) || (scombo.domid != ldomid)) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad flags (%x) or dom (%d); expected d%d, flags %x\n", scombo.flags, scombo.domid, ldomid, mask); + rc = GNTST_general_error; + goto done; + } if ( readonly ) { @@ -851,8 +859,12 @@ static int _set_status_v2(const grant_entry_header_t *shah, else { if ( unlikely(scombo.flags & GTF_readonly) ) - PIN_FAIL(done, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Attempt to write-pin a r/o grant entry\n"); + rc = GNTST_general_error; + goto done; + } *status |= GTF_reading | GTF_writing; } @@ -870,9 +882,11 @@ static int _set_status_v2(const grant_entry_header_t *shah, (!readonly && (scombo.flags & GTF_readonly)) ) { gnttab_clear_flags(rd, GTF_writing | GTF_reading, status); - PIN_FAIL(done, GNTST_general_error, + gdprintk(XENLOG_WARNING, "Unstable flags (%x) or dom (%d); expected d%d (r/w: %d)\n", scombo.flags, scombo.domid, ldomid, !readonly); + rc = GNTST_general_error; + goto done; } } else @@ -880,8 +894,9 @@ static int _set_status_v2(const grant_entry_header_t *shah, if ( unlikely(scombo.flags & GTF_readonly) ) { gnttab_clear_flags(rd, GTF_writing, status); - PIN_FAIL(done, GNTST_general_error, - "Unstable grant readonly flag\n"); + gdprintk(XENLOG_WARNING, "Unstable grant readonly flag\n"); + rc = GNTST_general_error; + goto done; } } @@ -1050,8 +1065,12 @@ map_grant_ref( /* Bounds check on the grant ref */ ref = op->ref; if ( unlikely(ref >= nr_grant_entries(rgt))) - PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n", + { + gdprintk(XENLOG_WARNING, "Bad ref %#x for d%d\n", ref, rgt->domain->domain_id); + rc = GNTST_bad_gntref; + goto unlock_out; + } /* This call also ensures the above check cannot be passed speculatively */ shah = shared_entry_header(rgt, ref); @@ -1062,9 +1081,13 @@ map_grant_ref( ((act->domid != ld->domain_id) || (act->pin & GNTPIN_incr2oflow_mask(pin_incr)) || (act->is_sub_page)) ) - PIN_FAIL(act_release_out, GNTST_general_error, + { + 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); + 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 @@ -1465,9 +1488,13 @@ unmap_common( if ( op->dev_bus_addr && (flags & GNTMAP_device_map) && unlikely(op->dev_bus_addr != mfn_to_maddr(act->mfn)) ) - PIN_FAIL(act_release_out, GNTST_bad_dev_addr, + { + gdprintk(XENLOG_WARNING, "Bus address doesn't match gntref (%"PRIx64" != %"PRIpaddr")\n", op->dev_bus_addr, mfn_to_maddr(act->mfn)); + rc = GNTST_bad_dev_addr; + goto act_release_out; + } if ( op->host_addr && (flags & GNTMAP_host_map) ) { @@ -2560,8 +2587,11 @@ acquire_grant_for_copy( grant_read_lock(rgt); if ( unlikely(gref >= nr_grant_entries(rgt)) ) - PIN_FAIL(gt_unlock_out, GNTST_bad_gntref, - "Bad grant reference %#x\n", gref); + { + gdprintk(XENLOG_WARNING, "Bad grant reference %#x\n", gref); + rc = GNTST_bad_gntref; + goto gt_unlock_out; + } /* This call also ensures the above check cannot be passed speculatively */ shah = shared_entry_header(rgt, gref); @@ -2571,9 +2601,13 @@ acquire_grant_for_copy( if ( act->pin && ((act->domid != ldom) || (act->pin & GNTPIN_incr2oflow_mask(pin_incr))) ) - PIN_FAIL(unlock_out, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "Bad domain (%d != %d), or risk of counter overflow %08x\n", act->domid, ldom, act->pin); + rc = GNTST_general_error; + goto unlock_out; + } if ( evaluate_nospec(rgt->gt_version == 1) ) { @@ -2596,16 +2630,24 @@ acquire_grant_for_copy( goto unlock_out; if ( !allow_transitive ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grant when transitivity not allowed\n"); + rc = GNTST_general_error; + goto unlock_out_clear; + } trans_domid = sha2->transitive.trans_domid; trans_gref = sha2->transitive.gref; barrier(); /* Stop the compiler from re-loading trans_domid from shared memory */ if ( trans_domid == rd->domain_id ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grants cannot be self-referential\n"); + rc = GNTST_general_error; + goto unlock_out_clear; + } /* * We allow the trans_domid == ldom case, which corresponds to a @@ -2618,9 +2660,13 @@ acquire_grant_for_copy( /* We need to leave the rrd locked during the grant copy. */ td = rcu_lock_domain_by_id(trans_domid); if ( td == NULL ) - PIN_FAIL(unlock_out_clear, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "transitive grant referenced bad domain %d\n", trans_domid); + rc = GNTST_general_error; + goto unlock_out_clear; + } /* * acquire_grant_for_copy() will take the lock on the remote table, @@ -2928,8 +2974,11 @@ static int gnttab_copy_claim_buf(const struct gnttab_copy *op, rc = get_paged_frame(ptr->u.gmfn, &buf->mfn, &buf->page, buf->read_only, buf->domain); if ( rc != GNTST_okay ) - PIN_FAIL(out, rc, + { + gdprintk(XENLOG_WARNING, "source frame %"PRI_xen_pfn" invalid\n", ptr->u.gmfn); + goto out; + } buf->ptr.u.gmfn = ptr->u.gmfn; buf->ptr.offset = 0; @@ -2972,25 +3021,29 @@ static int gnttab_copy_buf(const struct gnttab_copy *op, struct gnttab_copy_buf *dest, const struct gnttab_copy_buf *src) { - int rc; - if ( ((op->source.offset + op->len) > PAGE_SIZE) || ((op->dest.offset + op->len) > PAGE_SIZE) ) - PIN_FAIL(out, GNTST_bad_copy_arg, "copy beyond page area\n"); + { + gdprintk(XENLOG_WARNING, "copy beyond page area\n"); + return GNTST_bad_copy_arg; + } if ( op->source.offset < src->ptr.offset || op->source.offset + op->len > src->ptr.offset + src->len ) - PIN_FAIL(out, GNTST_general_error, + { + gdprintk(XENLOG_WARNING, "copy source out of bounds: %d < %d || %d > %d\n", - op->source.offset, src->ptr.offset, - op->len, src->len); + op->source.offset, src->ptr.offset, op->len, src->len); + return GNTST_general_error; + } if ( op->dest.offset < dest->ptr.offset || op->dest.offset + op->len > dest->ptr.offset + dest->len ) - PIN_FAIL(out, GNTST_general_error, - "copy dest out of bounds: %d < %d || %d > %d\n", - op->dest.offset, dest->ptr.offset, - op->len, dest->len); + { + gdprintk(XENLOG_WARNING, "copy dest out of bounds: %d < %d || %d > %d\n", + op->dest.offset, dest->ptr.offset, op->len, dest->len); + return GNTST_general_error; + } /* Make sure the above checks are not bypassed speculatively */ block_speculation(); @@ -2998,9 +3051,8 @@ static int gnttab_copy_buf(const struct gnttab_copy *op, memcpy(dest->virt + op->dest.offset, src->virt + op->source.offset, op->len); gnttab_mark_dirty(dest->domain, dest->mfn); - rc = GNTST_okay; - out: - return rc; + + return GNTST_okay; } static int gnttab_copy_one(const struct gnttab_copy *op, @@ -3373,9 +3425,17 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b) /* Bounds check on the grant refs */ if ( unlikely(ref_a >= nr_grant_entries(d->grant_table))) - PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-a %#x\n", ref_a); + { + gdprintk(XENLOG_WARNING, "Bad ref-a %#x\n", ref_a); + rc = GNTST_bad_gntref; + goto out; + } if ( unlikely(ref_b >= nr_grant_entries(d->grant_table))) - PIN_FAIL(out, GNTST_bad_gntref, "Bad ref-b %#x\n", ref_b); + { + gdprintk(XENLOG_WARNING, "Bad ref-b %#x\n", ref_b); + rc = GNTST_bad_gntref; + goto out; + } /* Make sure the above checks are not bypassed speculatively */ block_speculation(); @@ -3386,11 +3446,19 @@ swap_grant_ref(grant_ref_t ref_a, grant_ref_t ref_b) act_a = active_entry_acquire(gt, ref_a); if ( act_a->pin ) - PIN_FAIL(out, GNTST_eagain, "ref a %#x busy\n", ref_a); + { + gdprintk(XENLOG_WARNING, "ref a %#x busy\n", ref_a); + rc = GNTST_eagain; + goto out; + } act_b = active_entry_acquire(gt, ref_b); if ( act_b->pin ) - PIN_FAIL(out, GNTST_eagain, "ref b %#x busy\n", ref_b); + { + gdprintk(XENLOG_WARNING, "ref b %#x busy\n", ref_b); + rc = GNTST_eagain; + goto out; + } if ( evaluate_nospec(gt->gt_version == 1) ) { -- generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |