On Fri, Feb 12, 2021 at 01:48:43PM +0100, Jan Beulich wrote: > On 12.02.2021 11:41, Roger Pau Monné wrote: > > On Thu, Jan 14, 2021 at 04:04:57PM +0100, Jan Beulich wrote: > >> @@ -94,6 +106,8 @@ unsigned __copy_from_user_ll(void *to, c > >> return n; > >> } > >> > >> +#if GUARD(1) + 0 > > > > Why do you need the '+ 0' here? I guess it's to prevent the > > preprocessor from complaining when GUARD(1) gets replaced by nothing? > > Yes. "#if" with nothing after it is an error from all I know. > > >> --- a/xen/include/asm-x86/asm-defns.h > >> +++ b/xen/include/asm-x86/asm-defns.h > >> @@ -44,3 +44,16 @@ > >> .macro INDIRECT_JMP arg:req > >> INDIRECT_BRANCH jmp \arg > >> .endm > >> + > >> +.macro guest_access_mask_ptr ptr:req, scratch1:req, scratch2:req > >> +#if defined(CONFIG_SPECULATIVE_HARDEN_GUEST_ACCESS) > >> + mov $(HYPERVISOR_VIRT_END - 1), \scratch1 > >> + mov $~0, \scratch2 > >> + cmp \ptr, \scratch1 > >> + rcr $1, \scratch2 > >> + and \scratch2, \ptr > > > > If my understanding is correct, that's equivalent to: > > > > ptr &= ~0ull >> (ptr < HYPERVISOR_VIRT_END); > > > > It might be helpful to add this as a comment, to clarify the indented > > functionality of the assembly bit. > > > > I wonder if the C code above can generate any jumps? As you pointed > > out, we already use something similar in array_index_mask_nospec and > > that's fine to do in C. > > Note how array_index_mask_nospec() gets away without any use of > relational operators. They're what poses the risk of getting > translated to branches. (Quite likely the compiler wouldn't use > any in the case here, as the code can easily get away without, > but we don't want to chance it. Afaict it would instead use a > 3rd scratch register, so register pressure might still lead to > using a branch instead in some exceptional case.)
I see, it's not easy to build such construct without using any relational operator. Would you mind adding the C equivalent next to assembly chunk? I don't think I have further comments: Reviewed-by: Roger Pau Monné <roger....@citrix.com> Thanks, Roger.
