On Fri, Feb 10, 2023 at 05:00:05PM +0000, Edgecombe, Rick P wrote:
> /*
> * Dirty bit is not preserved above so it can be done
> * in a special way for the shadow stack case, where it
> * may need to set _PAGE_SAVED_DIRTY. __pte_mkdirty() will do
> * this in the case of shadow stack.
> */
> if (oldval & _PAGE_DIRTY)
> if (cpu_feature_enabled(X86_FEATURE_USER_SHSTK) &&
> !pte_write(pte_result))
> pte_set_flags(pte_result, _PAGE_SAVED_DIRTY);
> else
> pte_set_flags(pte_result, _PAGE_DIRTY);
> }
>
> return pte_result;
> }
>
> So the later logic of doing the _PAGE_SAVED_DIRTY (_PAGE_COW) part is
> not centralized. It's ok?
I think so.
1. If you have a single pte_mkdirty() and not also a __ helper, then
there's less confusion for callers as to which interface they should be
using
2. The not centralized part is a single conditional so it's not like
you're saving on gazillion code lines
So I'd prefer that.
If we end up needing this in more places then we can carve it out into
a proper helper which is not in a header file such that anyone can use
it but move the whole functionality into cet.c or so where we can
control its visibility to the rest of the kernel.
I'd say.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
