On Thursday, 8 June 2023 00:47:36 CEST Edgecombe, Rick P wrote: > 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. Yes, it is, sorry for confusion. I should have said: we may still want to keep _PAGE_PSE explicitly added to _HPAGE_CHK_MASK to have the reason for including that bit documented. Thanks, Janusz > > > > > > > > > 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... >