>>> On 29.01.19 at 15:43, <nmant...@amazon.de> wrote:
> @@ -963,6 +965,9 @@ map_grant_ref(
> PIN_FAIL(unlock_out, GNTST_bad_gntref, "Bad ref %#x for d%d\n",
> op->ref, rgt->domain->domain_id);
>
> + /* Make sure the above check is not bypassed speculatively */
> + op->ref = array_index_nospec(op->ref, nr_grant_entries(rgt));
> +
> act = active_entry_acquire(rgt, op->ref);
> shah = shared_entry_header(rgt, op->ref);
> status = rgt->gt_version == 1 ? &shah->flags : &status_entry(rgt,
> op->ref);
Just FTR - this is a case where the change, according to prior
discussion, is pretty unlikely to help at all. The compiler will have
a hard time realizing that it could keep the result in a register past
the active_entry_acquire() invocation, as that - due to the spin
lock acquired there - acts as a compiler barrier. And looking at
generated code (gcc 8.2) confirms that there's a reload from the
stack.
> @@ -2026,6 +2031,9 @@ gnttab_prepare_for_transfer(
> goto fail;
> }
>
> + /* Make sure the above check is not bypassed speculatively */
> + ref = array_index_nospec(ref, nr_grant_entries(rgt));
> +
> sha = shared_entry_header(rgt, ref);
>
> scombo.word = *(u32 *)&sha->flags;
> @@ -2223,7 +2231,8 @@ gnttab_transfer(
> okay = gnttab_prepare_for_transfer(e, d, gop.ref);
> spin_lock(&e->page_alloc_lock);
>
> - if ( unlikely(!okay) || unlikely(e->is_dying) )
> + /* Make sure this check is not bypassed speculatively */
> + if ( evaluate_nospec(unlikely(!okay) || unlikely(e->is_dying)) )
I'm still not really happy about this. The comment isn't helpful in
connecting the use of evaluate_nospec() to the problem site
(in the earlier hunk, which I've left in context), and I still don't
understand why the e->is_dying is getting wrapped as well.
Plus it occurs to me now that you're liable to render unlikely()
ineffective here. So how about
if ( unlikely(evaluate_nospec(!okay)) || unlikely(e->is_dying) )
?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
