On Mon, 2019-02-11 at 14:59 -0800, Andy Lutomirski wrote:

On Mon, Feb 11, 2019 at 11:09 AM Borislav Petkov [off-list ref] wrote:

quoted

On Mon, Jan 28, 2019 at 04:34:15PM -0800, Rick Edgecombe wrote:

quoted

This adds two new functions set_alias_default_noflush and

s/This adds/Add/

quoted

set_alias_nv_noflush for setting the alias mapping for the page to its

Please end function names with parentheses, below too.

quoted

default valid permissions and to an invalid state that cannot be cached in
a TLB, respectively. These functions to not flush the TLB.

s/to/do/

Also, pls put that description as comments over the functions in the
code. Otherwise that "nv" as part of the name doesn't really explain
what it does.

Actually, you could just as well call the function

set_alias_invalid_noflush()

All the other words are written in full, no need to have "nv" there.

Why are you calling this an "alias"?  You're modifying the direct map.
Your patches are thinking of the direct map as an alias of the vmap
mapping, but that does seem a bit backwards.  How about
set_direct_map_invalid_noflush(), etc?

I picked it up from some of the names in arch/x86/mm/pageattr.c:
CPA_NO_CHECK_ALIAS, set_memory_np_noalias(), etc. In that file the directmap
address seems to be the "alias". For 32 bit with highmem though, this would also
set permissions for a kmap mapping as well (if one existed), since that address
will be returned from page_address().

Yea, in vmalloc, vm_unmap_aliases talks about the vmap address "alias". So I
guess calling it "alias" is ambiguous. But does set_direct_map_invalid_noflush
make sense in the highmem case?

I couldn't think of any names that I loved, which is why I ran the
set_alias_*_noflush names by people in an earlier version, although looking back
only Ard chimed in on that. "set_direct_map_invalid_noflush" is fine with me if
nobody objects.

Thanks,

Rick