On Wed, 2023-06-07 at 23:33 +0200, Janusz Krzysztofik wrote: > > So since _PAGE_PSE is actually the same value as _PAGE_PAT, you > > don't > > actually need to have _PAGE_PSE in _HPAGE_CHG_MASK in order to get > > functional correctness. Is that right? > > As soon as we add _PAGE_PAT to _PAGE_CHG_MASK -- yes, that's right. > But we > may still want to add _PAGE_PSE to _HPAGE_CHG_MASK to have the need > for that > bit explicitly documented. _PAGE_PSE is already in _HPAGE_CHG_MASK though, right? I'm confused. > > > > > I think it is still a little hidden (even before this) and I wonder > > about separating out the common bits into, like, > > _COMMON_PAGE_CHG_MASK > > or something. Then setting specific PAGE and HPAGE bits (like > > _PAGE_PAT, _PAGE_PSE and _PAGE_PAT_LARGE) in their specific define. > > Would it be more readable that way? > > Yes, I think that's a good idea, and I can use it in my patch. > > The question if _PAGE_PAT vel _PAGE_PSE added to _PAGE_CHG_MASK is > really > harmless for pte_modify() and its users is still open for me though. When you say "vel", this is similar to the english acronym "AKA" I think? So I think you mean, when you add _PAGE_PAT to _PAGE_CHG_MASK, you are also adding _PAGE_PSE to it. So does that cause any problems? I see, good question... vm_page_prot is used when creating PTEs and huge PMDs, and the setter only uses _PAGE_CHG_MASK, even though it won't actually know where that prot is going to end up. Having _PAGE_PAT/PSE in _PAGE_CHG_MASK certainly doesn't make it easier to think about. One thing it's favor though is vm_page_prot is not applied to page table entries that are pointing to other page table entries (PSE = 0). So you shouldn't accidentally set PSE=1. And _PAGE_PSE shouldn't be being set in there, so you also shouldn't accidentally be setting PAT=1. But yea, I see why you are concerned. I would /guess/ it would be ok functionally. That probably doesn't help much...
