Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Jiuyang Liu <liu@jiuyang.me>
To:     Jisheng Zhang <Jisheng.Zhang@synaptics.com>,
        Alex Ghiti <alex@ghiti.fr>
Cc:     Palmer Dabbelt <palmer@dabbelt.com>,
        Andrew Waterman <waterman@eecs.berkeley.edu>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Andrew Morton <akpm@linux-foundation.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] implement flush_cache_vmap for RISC-V
Date:   Wed, 14 Apr 2021 09:16:34 +0000
Message-ID: <9908880.BHKHtspdbC@nuc>
In-Reply-To: <18d198ac-7bc0-934d-e1e9-eca01b790d61@ghiti.fr>
References: <20210412000531.12249-1-liu@jiuyang.me> <20210412150817.33f49be8@xhacker.debian> <18d198ac-7bc0-934d-e1e9-eca01b790d61@ghiti.fr>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
Precedence: bulk

On Wednesday, April 14, 2021 7:03:13 AM UTC you wrote:
> Hi,
>=20
> Le 4/12/21 =E0 3:08 AM, Jisheng Zhang a =E9crit :
> > Hi Jiuyang,
> >=20
> > On Mon, 12 Apr 2021 00:05:30 +0000 Jiuyang Liu <liu@jiuyang.me> wrote:
> >> This patch implements flush_cache_vmap for RISC-V, since it modifies P=
TE.
> >> Without this patch, SFENCE.VMA won't be added to related codes, which
> >> might introduce a bug in the out-of-order micro-architecture
> >> implementations.
> >>=20
> >> Signed-off-by: Jiuyang Liu <liu@jiuyang.me>
> >> Reviewed-by: Alexandre Ghiti <alex@ghiti.fr>
> >> Reviewed-by: Palmer Dabbelt <palmer@dabbelt.com>
> >=20
> > IIRC, Palmer hasn't given this Reviewed-by tag.
> >=20
> >> ---
> >=20
> > Could you plz add version and changes? IIRC, this is the v3.
> >=20
> >>   arch/riscv/include/asm/cacheflush.h | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>=20
> >> diff --git a/arch/riscv/include/asm/cacheflush.h
> >> b/arch/riscv/include/asm/cacheflush.h index 23ff70350992..3fd528badc35
> >> 100644
> >> --- a/arch/riscv/include/asm/cacheflush.h
> >> +++ b/arch/riscv/include/asm/cacheflush.h
> >> @@ -30,6 +30,12 @@ static inline void flush_dcache_page(struct page
> >> *page)
> >>=20
> >>   #define flush_icache_user_page(vma, pg, addr, len) \
> >>  =20
> >>          flush_icache_mm(vma->vm_mm, 0)
> >>=20
> >> +/*
> >> + * flush_cache_vmap is invoked after map_kernel_range() has installed
> >> the page + * table entries, which modifies PTE, SFENCE.VMA should be
> >> inserted.>=20
> > Just my humble opinion, flush_cache_vmap() may not be necessary.
> > vmalloc_fault can take care of this, and finally sfence.vma is inserted
> > in related path.
> I believe Palmer and Jisheng are right, my initial proposal to implement
> flush_cache_vmap is wrong.
>=20
> But then, Jiuyang should not have noticed any problem here, so what's
> wrong? @Jiuyang: Does implementing flush_cache_vmap fix your issue?
Yes, I just checked this today, it did fix our issue.
But I'm happy to rework on this, thanks for your help! This week I will try=
 to=20
trace wave of our core, and give a more detail email then.
> And regarding flush_cache_vunmap, from Jisheng call stack, it seems also
> not necessary.
>=20
> @Jiuyang: Can you tell us more about what you noticed?
>=20
> > Regards
> >=20
> >> + */
> >> +#define flush_cache_vmap(start, end) flush_tlb_all()
> >> +
> >>=20
> >>   #ifndef CONFIG_SMP
> >>  =20
> >>   #define flush_icache_all() local_flush_icache_all()
> >>=20
> >> --
> >> 2.31.1
> >=20
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Thanks,
Jiuyang