Messages in this thread | | | Date | Tue, 10 May 2022 20:50:29 +0900 | From | Hyeonggon Yoo <> | Subject | Re: [PATCH] x86/mm/cpa: set PAGE_KERNEL in __set_pages_p() |
| |
On Tue, May 10, 2022 at 12:47:11AM +0000, Edgecombe, Rick P wrote: > On Fri, 2022-05-06 at 14:19 +0900, Hyeonggon Yoo wrote: > > __set_pages_np() not only clears _PAGE_PRESENT and _PAGE_RW, but also > > clears _PAGE_GLOBAL to avoid confusing _PAGE_GLOBAL as _PAGE_PROTNONE > > when the PTE is not present. > > > > Common usage for __set_pages_p() is to call it after > > __set_pages_np(). > > Therefore calling __set_pages_p() after __set_pages_np() clears > > _PAGE_GLOBAL, making it unable to globally shared in TLB. > > > > As they are called by set_direct_map_{invalid,default}_noflush(), > > pages in direct map cannot be globally shared in TLB after being used > > by > > vmalloc, secretmem, and hibernation. > > > > So set PAGE_KERNEL isntead of __pgprot(_PAGE_PRESENT | _PAGE_RW) in > > __set_pages_p(). > > Nice find. I think we can't always set PAGE_KERNEL also because of the > PTI code. It sometimes wants the direct map to be non global. >
Thanks for review!
IIUC __pgprot_mask() already clears _PAGE_GLOBAL from PAGE_KERNEL when PTI is used.
#define __pgprot_mask(x) __pgprot((x) & __default_kernel_pte_mask) #define PAGE_KERNEL __pgprot_mask(__PAGE_KERNEL | _ENC)
But yeah, it seems PAGE_KERNEL is too much for this.
> Maybe something like this? > (_PAGE_PRESENT | _PAGE_RW | _PAGE_GLOBAL) & __default_kernel_pte_mask >
What about __pgprot_mask(_PAGE_PRESENT | _PAGE_RW | _PAGE_GLOBAL)?
> That would add back in a little of the "default global" behavior that > was removed in d1440b2, but I think it should be ok because it is > limited to the direct map.
> Otherwise, I wonder if the existing global > clearing logic is really needed.
I think it's still needed. pte_present() returning true due to _PAGE_PROTNONE after __set_pages_np() simply doesn't make sense. No?
-- Thanks, Hyeonggon
| |