lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 30 May 2024 17:41:14 +0800
From: Yong-Xuan Wang <yongxuan.wang@...ive.com>
To: Alexandre Ghiti <alex@...ti.fr>
Cc: Andrew Jones <ajones@...tanamicro.com>, linux-riscv@...ts.infradead.org, 
	kvm-riscv@...ts.infradead.org, kvm@...r.kernel.org, greentime.hu@...ive.com, 
	vincent.chen@...ive.com, cleger@...osinc.com, Jinyu Tang <tjytimi@....com>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Anup Patel <anup@...infault.org>, 
	Conor Dooley <conor.dooley@...rochip.com>, Mayuresh Chitale <mchitale@...tanamicro.com>, 
	Samuel Holland <samuel.holland@...ive.com>, Samuel Ortiz <sameo@...osinc.com>, 
	Evan Green <evan@...osinc.com>, Xiao Wang <xiao.w.wang@...el.com>, 
	Alexandre Ghiti <alexghiti@...osinc.com>, Andrew Morton <akpm@...ux-foundation.org>, 
	Kemeng Shi <shikemeng@...weicloud.com>, "Mike Rapoport (IBM)" <rppt@...nel.org>, 
	Jisheng Zhang <jszhang@...nel.org>, "Matthew Wilcox (Oracle)" <willy@...radead.org>, 
	Charlie Jenkins <charlie@...osinc.com>, Leonardo Bras <leobras@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH v4 1/5] RISC-V: Detect and Enable Svadu Extension Support

Hi Alexandre,

On Thu, May 30, 2024 at 4:19 PM Alexandre Ghiti <alex@...ti.fr> wrote:
>
> Hi Yong-Xuan,
>
> On 27/05/2024 18:25, Andrew Jones wrote:
> > On Fri, May 24, 2024 at 06:33:01PM GMT, Yong-Xuan Wang wrote:
> >> Svadu is a RISC-V extension for hardware updating of PTE A/D bits.
> >>
> >> In this patch we detect Svadu extension support from DTB and enable it
> >> with SBI FWFT extension. Also we add arch_has_hw_pte_young() to enable
> >> optimization in MGLRU and __wp_page_copy_user() if Svadu extension is
> >> available.
>
>
> So we talked about this yesterday during the linux-riscv patchwork
> meeting. We came to the conclusion that we should not wait for the SBI
> FWFT extension to enable Svadu but instead, it should be enabled by
> default by openSBI if the extension is present in the device tree. This
> is because we did not find any backward compatibility issues, meaning
> that enabling Svadu should not break any S-mode software. This is what
> you did in your previous versions of this patchset so the changes should
> be easy. This behaviour must be added to the dtbinding description of
> the Svadu extension.
>
> Another thing that we discussed yesterday. There exist 2 schemes to
> manage the A/D bits updates, Svade and Svadu. If a platform supports
> both extensions and both are present in the device tree, it is M-mode
> firmware's responsibility to provide a "sane" device tree to the S-mode
> software, meaning the device tree can not contain both extensions. And
> because on such platforms, Svadu is more performant than Svade, Svadu
> should be enabled by the M-mode firmware and only Svadu should be
> present in the device tree.
>
> I hope that clearly explains what we discussed yesterday, let me know if
> you (or anyone else) need more explanations. If no one is opposed to
> this solution, do you think you can implement this behaviour? If not, I
> can deal with it, just let me know.
>
> Thanks
>
>

Sure, I can do it. Just to confirm, which extension should be removed
when both Svade and Svadu are present in the DT?

Regards,
Yong-Xuan

> >>
> >> Co-developed-by: Jinyu Tang <tjytimi@....com>
> >> Signed-off-by: Jinyu Tang <tjytimi@....com>
> >> Signed-off-by: Yong-Xuan Wang <yongxuan.wang@...ive.com>
> >> Reviewed-by: Conor Dooley <conor.dooley@...rochip.com>
> >> Reviewed-by: Andrew Jones <ajones@...tanamicro.com>
> > I think this patch changed too much to keep r-b's. We didn't have the
> > FWFT part before.
> >
> >> ---
> >>   arch/riscv/Kconfig               |  1 +
> >>   arch/riscv/include/asm/csr.h     |  1 +
> >>   arch/riscv/include/asm/hwcap.h   |  1 +
> >>   arch/riscv/include/asm/pgtable.h |  8 +++++++-
> >>   arch/riscv/kernel/cpufeature.c   | 11 +++++++++++
> >>   5 files changed, 21 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index be09c8836d56..30fa558ee284 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -34,6 +34,7 @@ config RISCV
> >>      select ARCH_HAS_PMEM_API
> >>      select ARCH_HAS_PREPARE_SYNC_CORE_CMD
> >>      select ARCH_HAS_PTE_SPECIAL
> >> +    select ARCH_HAS_HW_PTE_YOUNG
> >>      select ARCH_HAS_SET_DIRECT_MAP if MMU
> >>      select ARCH_HAS_SET_MEMORY if MMU
> >>      select ARCH_HAS_STRICT_KERNEL_RWX if MMU && !XIP_KERNEL
> >> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csrh
> >> index 2468c55933cd..2ac270ad4acd 100644
> >> --- a/arch/riscv/include/asm/csr.h
> >> +++ b/arch/riscv/include/asm/csr.h
> >> @@ -194,6 +194,7 @@
> >>   /* xENVCFG flags */
> >>   #define ENVCFG_STCE                        (_AC(1, ULL) << 63)
> >>   #define ENVCFG_PBMTE                       (_AC(1, ULL) << 62)
> >> +#define ENVCFG_ADUE                 (_AC(1, ULL) << 61)
> >>   #define ENVCFG_CBZE                        (_AC(1, UL) << 7)
> >>   #define ENVCFG_CBCFE                       (_AC(1, UL) << 6)
> >>   #define ENVCFG_CBIE_SHIFT          4
> >> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> >> index e17d0078a651..8d539e3f4e11 100644
> >> --- a/arch/riscv/include/asm/hwcap.h
> >> +++ b/arch/riscv/include/asm/hwcap.h
> >> @@ -81,6 +81,7 @@
> >>   #define RISCV_ISA_EXT_ZTSO         72
> >>   #define RISCV_ISA_EXT_ZACAS                73
> >>   #define RISCV_ISA_EXT_XANDESPMU            74
> >> +#define RISCV_ISA_EXT_SVADU         75
> >>
> >>   #define RISCV_ISA_EXT_XLINUXENVCFG 127
> >>
> >> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> >> index 9f8ea0e33eb1..1f1b326ccf63 100644
> >> --- a/arch/riscv/include/asm/pgtable.h
> >> +++ b/arch/riscv/include/asm/pgtable.h
> >> @@ -117,6 +117,7 @@
> >>   #include <asm/tlbflush.h>
> >>   #include <linux/mm_types.h>
> >>   #include <asm/compat.h>
> >> +#include <asm/cpufeature.h>
> >>
> >>   #define __page_val_to_pfn(_val)  (((_val) & _PAGE_PFN_MASK) >> _PAGE_PFN_SHIFT)
> >>
> >> @@ -285,7 +286,6 @@ static inline pte_t pud_pte(pud_t pud)
> >>   }
> >>
> >>   #ifdef CONFIG_RISCV_ISA_SVNAPOT
> >> -#include <asm/cpufeature.h>
> >>
> >>   static __always_inline bool has_svnapot(void)
> >>   {
> >> @@ -621,6 +621,12 @@ static inline pgprot_t pgprot_writecombine(pgprot_t _prot)
> >>      return __pgprot(prot);
> >>   }
> >>
> >> +#define arch_has_hw_pte_young arch_has_hw_pte_young
> >> +static inline bool arch_has_hw_pte_young(void)
> >> +{
> >> +    return riscv_has_extension_unlikely(RISCV_ISA_EXT_SVADU);
> >> +}
> >> +
> >>   /*
> >>    * THP functions
> >>    */
> >> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> >> index 3ed2359eae35..b023908c5932 100644
> >> --- a/arch/riscv/kernel/cpufeature.c
> >> +++ b/arch/riscv/kernel/cpufeature.c
> >> @@ -93,6 +93,16 @@ static bool riscv_isa_extension_check(int id)
> >>                      return false;
> >>              }
> >>              return true;
> >> +    case RISCV_ISA_EXT_SVADU:
> >> +            if (sbi_probe_extension(SBI_EXT_FWFT) > 0) {
> > I think we've decided the appropriate way to prove for SBI extensions is
> > to first ensure the SBI version and then do the probe, like we do for STA
> > in has_pv_steal_clock()
> >
> >> +                    struct sbiret ret;
> >> +
> >> +                    ret = sbi_ecall(SBI_EXT_FWFT, SBI_EXT_FWFT_SET, SBI_FWFT_PTE_AD_HW_UPDATING,
> >> +                                    1, 0, 0, 0, 0);
> >> +
> >> +                    return ret.error == SBI_SUCCESS;
> >> +            }
> >> +            return false;
> >>      case RISCV_ISA_EXT_INVALID:
> >>              return false;
> >>      }
> >> @@ -301,6 +311,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
> >>      __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
> >>      __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
> >>      __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
> >> +    __RISCV_ISA_EXT_SUPERSET(svadu, RISCV_ISA_EXT_SVADU, riscv_xlinuxenvcfg_exts),
> > We do we need XLINUXENVCFG?
> >
> > Thanks,
> > drew
> >
> >>      __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
> >>      __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
> >>      __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
> >> --
> >> 2.17.1
> >>
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ