On 11/10/18 4:31 PM, Dan Williams wrote:
If it indeed can run late in boot or after boot, then it sure looks buggy. Either the __flush_tlb_all() should be removed or it should be replaced with flush_tlb_kernel_range(). It’s unclear to me why a flush is needed at all, but if it’s needed, surely all CPUs need flushing.
Yeah, I don't think __flush_tlb_all() is needed at kernel_physical_mapping_init() time, and at kernel_physical_mapping_remove() time we do a full flush_tlb_all().
It doesn't look strictly necessary to me. I _think_ we're only ever populating previously non-present entries, and those never need TLB flushes. I didn't look too deeply, so I'd appreciate anyone else double-checking me on this.
The __flush_tlb_all() actually appears to predate git and it was originally entirely intended for early-boot-only. It probably lasted this long because it looks really important. :)
It was even next to where we set MMU features in CR4, which is *really* early in boot:
asm volatile("movq %%cr4,%0" : "=r" (mmu_cr4_features));__flush_tlb_all();
I also totally agree with Andy that if it were needed on the local CPU, this code would be buggy because it doesn't initiate any *remote* TLB flushes.
So, let's remove it, but also add some comments about not being allowed to *change* page table entries, only populate them. We could even add some warnings to keep this enforced.