Re: [PATCH v5 11/39] x86/mm: Update pte_modify for _PAGE_COW

[Edgecombe, Rick P]

On Fri, 2023-02-17 at 17:11 +0100, Borislav Petkov wrote:
> 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.
> 
> 
Fair enough, I'll adjust it. Thanks!