On 2/6/19 16:25, Jan Beulich wrote: >>>> On 29.01.19 at 15:43, <[email protected]> wrote: >> @@ -33,10 +34,10 @@ unsigned long __read_mostly >> pdx_group_valid[BITS_TO_LONGS( >> >> bool __mfn_valid(unsigned long mfn) >> { >> - return likely(mfn < max_page) && >> - likely(!(mfn & pfn_hole_mask)) && >> - likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT, >> - pdx_group_valid)); >> + return evaluate_nospec(likely(mfn < max_page) && >> + likely(!(mfn & pfn_hole_mask)) && >> + likely(test_bit(pfn_to_pdx(mfn) / >> PDX_GROUP_COUNT, >> + pdx_group_valid))); > Other than in the questionable grant table case, here I agree that > you want to wrap the entire construct. This has an unwanted effect > though: The test_bit() may still be speculated into with an out-of- > bounds mfn. (As mentioned elsewhere, operations on bit arrays are > an open issue altogether.) I therefore think you want to split this into > two: > > bool __mfn_valid(unsigned long mfn) > { > return likely(evaluate_nospec(mfn < max_page)) && > evaluate_nospec(likely(!(mfn & pfn_hole_mask)) && > likely(test_bit(pfn_to_pdx(mfn) / PDX_GROUP_COUNT, > pdx_group_valid))); > }
I can split the code. However, I wonder whether the test_bit accesses should be protected separately, or actually as part of the test_bit method themselves. Do you have any plans to do that already, because in that case I would not have to modify the code. Best, Norbert Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich Ust-ID: DE 289 237 879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
