[MODERATED] Re: [PATCH 8/8] L1TFv3 3

[Andi Kleen <ak@linux.intel.com>]

On Fri, May 04, 2018 at 03:15:34PM -0700, speck for Dave Hansen wrote:
> > +	/* If it's real memory always allow */
> > +	if (pfn_valid(pfn))
> > +		return true;
> > +	if ((pfn & (1ULL << (boot_cpu_data.x86_phys_bits - 1 - PAGE_SHIFT))) &&
> > +	    !capable(CAP_SYS_ADMIN))
> > +		return false;
> 
> The CAP_SYS_ADMIN kinda surprised me here.  Was there a reason we don't
> just do this up front in the function?  Seems like we'd always allow pfn
> modification if CAP_SYS_ADMIN.

In most cases it's useless because the earlier checks reject it.
I tried to order it by likelyhood that it rejects.

> Otherwise it looks OK to me.  The need for arch_has_pfn_modify_check()
> wasn't obvious until the end, but it all came together.  This seems like
> a pretty reasonable compromise for something we think is really rare.
> 
> Although, I do wonder: should we put a WARN_ONCE() or something else in
> here to help us see if *anybody* does this?  Or even a VM counter?  Or

If it's really a problem it would show up in perf.

> do we just wait for the bug reports from somebody doing frequent 1-page
> PROT_NONEs in a 100GB VMA?  :)

I doubt it is actually noticeable slower than something open coded. 
Even though there are some callbacks modern CPUs are really good at
running them very very fast fast

-Andi