Re: [PATCH] riscv: Add sfence.vma after page table changed

From: Will Deacon <willdeacon@google.com>
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: mark.rutland@arm.com, Marc Zyngier <maz@kernel.org>,
	aou@eecs.berkeley.edu, syven.wang@sifive.com,
	catalin.marinas@arm.com, Paul Walmsley <paul.walmsley@sifive.com>,
	linux-kernel@vger.kernel.org, anup@brainfault.org,
	greentime.hu@sifive.com, linux-riscv@lists.infradead.org
Subject: Re: [PATCH] riscv: Add sfence.vma after page table changed
Date: Wed, 5 Aug 2020 09:12:50 +0100	[thread overview]
Message-ID: <20200805081250.GA3064472@google.com> (raw)
In-Reply-To: <mhng-1f7adcd7-551c-4b8a-80f5-e1230b335c6a@palmerdabbelt-glaptop1>

On Tue, Aug 04, 2020 at 07:03:06PM -0700, Palmer Dabbelt wrote:
> > On Tue, Aug 4, 2020 at 8:32 AM Greentime Hu <greentime.hu@sifive.com> wrote:
> > > diff --git a/arch/riscv/mm/init.c b/arch/riscv/mm/init.c
> > > index f4adb3684f3d..29b0f7108054 100644
> > > --- a/arch/riscv/mm/init.c
> > > +++ b/arch/riscv/mm/init.c
> > > @@ -202,12 +202,11 @@ void __set_fixmap(enum fixed_addresses idx, phys_addr_t phys, pgprot_t prot)
> > > 
> > >         ptep = &fixmap_pte[pte_index(addr)];
> > > 
> > > -       if (pgprot_val(prot)) {
> > > +       if (pgprot_val(prot))
> > >                 set_pte(ptep, pfn_pte(phys >> PAGE_SHIFT, prot));
> > > -       } else {
> > > +       else
> > >                 pte_clear(&init_mm, addr, ptep);
> > > -               local_flush_tlb_page(addr);
> > > -       }
> > > +       local_flush_tlb_page(addr);
> > >  }
> 
> arm64 appears to be upgrading all set_pte()s on valid kernel mappings to
> include the fence.  It looks like the message from 7f0b1bf04511 ("arm64: Fix
> barriers used for page table modifications") is out of date, as I can't find
> create_mapping() any more.  If that was some generic kernel thing then we
> should probably upgrade ours as well, but if it was arch/arm64/ code then this
> approach seems fine as __set_fixmap() isn't on the hot path -- I guess this is
> fine either way, but there may be other issues that the arm64 approach fixes.
> 
> Do you guys happen to remember what was going on here?

Basically, the architecture says we need these fences in order to guarantee
that the page-table walker sees the updated entry. Since we couldn't
tolerate spurious faults on kernel mappings, we had to add them (for
userspace we don't bother because in reality we don't tend to take the
spurious fault, and it's harmless if we do).

But having said all that, check out 42f91093b043 ("arm64: mm: Ignore spurious
translation faults taken from the kernel").

Will

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv