Messages in this thread | | | From | Linus Torvalds <> | Date | Thu, 18 Aug 2022 20:08:08 -0700 | Subject | Re: [PATCH 1/1] x86/mm: Use proper mask when setting PUD mapping |
| |
On Thu, Aug 18, 2022 at 7:30 PM Aaron Lu <aaron.lu@intel.com> wrote: > > - prot = __pgprot(pgprot_val(prot) | __PAGE_KERNEL_LARGE); > + prot = __pgprot_mask(pgprot_val(prot) | __PAGE_KERNEL_LARGE);
The patch looks "ObviouslyCorrect(tm)" to me, but I have to admit that I absolutely hate how we use the pte helpers for all the levels.
It gets even worse when we do that
set_pte_init((pte_t *)pud, pfn_pte((paddr & PUD_MASK) >> PAGE_SHIFT, prot), init);
on the next lines, and I don't understand why this doesn't use "set_pud_init()" here.
It's probably something obvious, like "using set_pud_init() would mean that we'd have to cast the *second* argument instead, because we don't have a pfd_pud() function".
But it all makes me go a bit eww, and also makes me suspect I am missing something else too, and that my "this looks ObviouslyCorrect(tm)" is thus worthless.
Also, I don't understand why we use that __PAGE_KERNEL_LARGE at all. We already have a valid set of protection bits, that had gotten properly masked previously.
Isn't the only bit we actually want to set "_PAGE_PSE"?
IOW, I get the feeling that that patch should instead just be
- prot = __pgprot(pgprot_val(prot) | __PAGE_KERNEL_LARGE); + prot = __pgprot(pgprot_val(prot) | _PAGE_PSE);
and we should never need to mask anything off at all with __pgprot_mask() - the bug was really that we set way too many bits.
But again, I *also* have the feeling that I'm missing something important.
Ingo, Thomas, any others who know this code better than me by now - comments?
Linus
| |