Re: [PATCH 1/2] powerpc/64s/radix: do not flush TLB when relaxing access

[Nicholas Piggin <npiggin@gmail.com>]

On Wed, 9 May 2018 17:07:47 +1000
Balbir Singh <bsingharora@gmail.com> wrote:

> On Wed, May 9, 2018 at 4:51 PM, Nicholas Piggin <npiggin@gmail.com> wrote:
> > Radix flushes the TLB when updating ptes to increase permissiveness
> > of protection (increase access authority). Book3S does not require
> > TLB flushing in this case, and it is not done on hash. This patch
> > avoids the flush for radix.
> >
> > From Power ISA v3.0B, p.1090:
> >
> >     Setting a Reference or Change Bit or Upgrading Access Authority
> >     (PTE Subject to Atomic Hardware Updates)
> >
> >     If the only change being made to a valid PTE that is subject to
> >     atomic hardware updates is to set the Reference or Change bit to 1
> >     or to add access authorities, a simpler sequence suffices because
> >     the translation hardware will refetch the PTE if an access is
> >     attempted for which the only problems were reference and/or change
> >     bits needing to be set or insufficient access authority.
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  arch/powerpc/mm/pgtable-book3s64.c | 1 -
> >  arch/powerpc/mm/pgtable.c          | 3 ++-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/mm/pgtable-book3s64.c b/arch/powerpc/mm/pgtable-book3s64.c
> > index 518518fb7c45..6e991eaccab4 100644
> > --- a/arch/powerpc/mm/pgtable-book3s64.c
> > +++ b/arch/powerpc/mm/pgtable-book3s64.c
> > @@ -40,7 +40,6 @@ int pmdp_set_access_flags(struct vm_area_struct *vma, unsigned long address,
> >         if (changed) {
> >                 __ptep_set_access_flags(vma->vm_mm, pmdp_ptep(pmdp),
> >                                         pmd_pte(entry), address);
> > -               flush_pmd_tlb_range(vma, address, address + HPAGE_PMD_SIZE);  
> 
> The comment states that this can be used for missing execution
> permissions as well. I am not convinced we can skip a flush in those
> cases

Why not? Execute is part of the access authority. And they're already no
ops on hash. What am I missing?

Thanks,
Nick