On Mon, 2022-10-03 at 16:25 -0700, Nadav Amit wrote:
> On Oct 3, 2022, at 4:20 PM, Nadav Amit <nadav.amit@xxxxxxxxx> wrote:
>
> > On Oct 3, 2022, at 4:17 PM, Nadav Amit <nadav.amit@xxxxxxxxx>
> > wrote:
> >
> > > On Oct 3, 2022, at 3:28 PM, Edgecombe, Rick P <
> > > rick.p.edgecombe@xxxxxxxxx> wrote:
> > >
> > > > On Mon, 2022-10-03 at 11:11 -0700, Nadav Amit wrote:
> > > > > Did you have a look at ptep_set_access_flags() and friends
> > > > > and
> > > > > checked they
> > > > > do not need to be changed too?
> > > >
> > > > ptep_set_access_flags() doesn't actually set any additional
> > > > dirty bits
> > > > on x86, so I think it's ok.
> > >
> > > Are you sure about that? (lost my confidence today so I am
> > > hesitant).
> > >
> > > Looking on insert_pfn(), I see:
> > >
> > > entry = maybe_mkwrite(pte_mkdirty(entry),
> > > vma);
> > > if (ptep_set_access_flags(vma, addr, pte,
> > > entry, 1)) ...
> > >
> > > This appears to set the dirty bit while potentially leaving the
> > > write-bit
> > > clear. This is the scenario you want to avoid, no?
> >
> > No. I am not paying attention. Ignore.
>
> Sorry for the spam. Just this “dirty” argument is confusing. This
> indeed
> seems like a flow that can set the dirty bit. I think.
I think the HW dirty bit will not be set here. How it works is,
pte_mkdirty() will not actually set the HW dirty bit, but instead the
software COW bit. Here is the relevant snippet:
static inline pte_t pte_mkdirty(pte_t pte)
{
pteval_t dirty = _PAGE_DIRTY;
/* Avoid creating Dirty=1,Write=0 PTEs */
if (cpu_feature_enabled(X86_FEATURE_SHSTK) && !pte_write(pte))
dirty = _PAGE_COW;
return pte_set_flags(pte, dirty | _PAGE_SOFT_DIRTY);
}
So for a !VM_WRITE vma, you end up with Write=0,Cow=1 PTE passed
into ptep_set_access_flags(). Does it make sense?
