[PATCH] lib: sbi: Fix hw a/d updating defaults

[PATCH] lib: sbi: Fix hw a/d updating defaults

[Andrew Jones]

The Svade dt-binding description states that Svadu should only
be enabled at boot time when only Svadu is present in the DT.
Ensure that's the case. Also, when only Svadu is supported,
disable FWFT.PTE_AD_HW_UPDATING, as we need both to support
toggling.

Signed-off-by: Andrew Jones <andrew.jones@oss.qualcomm.com>
---
 lib/sbi/sbi_fwft.c | 10 +++++++++-
 lib/sbi/sbi_hart.c | 18 ++++++++++--------
 2 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
index 373140b746bd..574b875e408d 100644
--- a/lib/sbi/sbi_fwft.c
+++ b/lib/sbi/sbi_fwft.c
@@ -160,8 +160,16 @@ static int fwft_get_double_trap(struct fwft_config *conf, unsigned long *value)
 
 static int fwft_adue_supported(struct fwft_config *conf)
 {
+	/*
+	 * FWFT.PTE_AD_HW_UPDATING is only supported when both Svade and Svadu
+	 * are supported. We need both in order to support toggling and to
+	 * ensure the reset value of zero is valid (it wouldn't be when only
+	 * Svadu is supported).
+	 */
 	if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
-				    SBI_HART_EXT_SVADU))
+				    SBI_HART_EXT_SVADU) ||
+	    !sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
+				    SBI_HART_EXT_SVADE))
 		return SBI_ENOTSUPP;
 
 	return SBI_OK;
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 99e13990aa60..0df11d0d1029 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -137,6 +137,9 @@ static void mstatus_init(struct sbi_scratch *scratch)
 	if (sbi_hart_priv_version(scratch) >= SBI_HART_PRIV_VER_1_12) {
 		menvcfg_val = csr_read64(CSR_MENVCFG);
 
+		/* Disable HW A/D updating by default */
+		menvcfg_val &= ~ENVCFG_ADUE;
+
 		/* Disable double trap by default */
 		menvcfg_val &= ~ENVCFG_DTE;
 
@@ -158,18 +161,17 @@ static void mstatus_init(struct sbi_scratch *scratch)
 #endif
 		__set_menvcfg_ext(SBI_HART_EXT_SSTC, ENVCFG_STCE)
 		__set_menvcfg_ext(SBI_HART_EXT_SMCDELEG, ENVCFG_CDE);
-		__set_menvcfg_ext(SBI_HART_EXT_SVADU, ENVCFG_ADUE);
-
-#undef __set_menvcfg_ext
 
 		/*
-		 * When both Svade and Svadu are present in DT, the default scheme for managing
-		 * the PTE A/D bits should use Svade. Check Svadu before Svade extension to ensure
-		 * that the ADUE bit is cleared when the Svade support are specified.
+		 * Assume only Svadu is supported when it is the only extension
+		 * present in the ISA string. Svade is assumed when neither are
+		 * present. When both are present we must default to Svade (see
+		 * the zero reset value of FWFT.PTE_AD_HW_UPDATING).
 		 */
+		if (!sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
+			__set_menvcfg_ext(SBI_HART_EXT_SVADU, ENVCFG_ADUE);
 
-		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
-			menvcfg_val &= ~ENVCFG_ADUE;
+#undef __set_menvcfg_ext
 
 		csr_write64(CSR_MENVCFG, menvcfg_val);
 
-- 
2.43.0


-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] lib: sbi: Fix hw a/d updating defaults

[Radim Krčmář]

2026-04-01T17:08:45-05:00, Andrew Jones <andrew.jones@oss.qualcomm.com>:
> The Svade dt-binding description states that Svadu should only
> be enabled at boot time when only Svadu is present in the DT.
> Ensure that's the case. Also, when only Svadu is supported,
> disable FWFT.PTE_AD_HW_UPDATING, as we need both to support
> toggling.

Does Svadu without Svade mean that both menvcfg.ADUE or henvcfg.ADUE are
read-only 1 (the ISA is vague enough to permit this), or is it just an
awkward way to configure the default, so HS mode must assume that
henvcfg.ADUE=0 will be Svade?

Svade is mandatory in RVA profiles, so the corner-case case it not
anything I expect to see...

Reviewed-by: Radim Krčmář <radim.krcmar@oss.qualcomm.com>

Thanks.

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] lib: sbi: Fix hw a/d updating defaults

[Andrew Jones]

On Thu, Apr 02, 2026 at 04:56:43PM +0200, Radim Krčmář wrote:
> 2026-04-01T17:08:45-05:00, Andrew Jones <andrew.jones@oss.qualcomm.com>:
> > The Svade dt-binding description states that Svadu should only
> > be enabled at boot time when only Svadu is present in the DT.
> > Ensure that's the case. Also, when only Svadu is supported,
> > disable FWFT.PTE_AD_HW_UPDATING, as we need both to support
> > toggling.
> 
> Does Svadu without Svade mean that both menvcfg.ADUE or henvcfg.ADUE are
> read-only 1 (the ISA is vague enough to permit this), or is it just an
> awkward way to configure the default, so HS mode must assume that
> henvcfg.ADUE=0 will be Svade?

I couldn't find anything in the spec that explicitly states the ADUE bit
will behave has read-only one when only Svadu is supported. If the spec
at least claimed that the ADUE field was WARL then we might infer that,
but it doesn't even say that.

> 
> Svade is mandatory in RVA profiles, so the corner-case case it not
> anything I expect to see...

Yes, I expect if Svadu-only harts are built that they likely will make
ADUE read-only 1, so I don't expect to see problems with just
unconditionally setting the bit to 1, as we do here (and should set
henvcfg.ADUE in KVM as well).

> 
> Reviewed-by: Radim Krčmář <radim.krcmar@oss.qualcomm.com>

Thanks!

drew

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi

Re: [PATCH] lib: sbi: Fix hw a/d updating defaults

[Radim Krčmář]

2026-04-02T11:24:20-05:00, Andrew Jones <andrew.jones@oss.qualcomm.com>:
> On Thu, Apr 02, 2026 at 04:56:43PM +0200, Radim Krčmář wrote:
>> 2026-04-01T17:08:45-05:00, Andrew Jones <andrew.jones@oss.qualcomm.com>:
>> > The Svade dt-binding description states that Svadu should only
>> > be enabled at boot time when only Svadu is present in the DT.
>> > Ensure that's the case. Also, when only Svadu is supported,
>> > disable FWFT.PTE_AD_HW_UPDATING, as we need both to support
>> > toggling.
>> 
>> Does Svadu without Svade mean that both menvcfg.ADUE or henvcfg.ADUE are
>> read-only 1 (the ISA is vague enough to permit this), or is it just an
>> awkward way to configure the default, so HS mode must assume that
>> henvcfg.ADUE=0 will be Svade?
>
> I couldn't find anything in the spec that explicitly states the ADUE bit
> will behave has read-only one when only Svadu is supported. If the spec
> at least claimed that the ADUE field was WARL then we might infer that,
> but it doesn't even say that.

Yeah, it was a long shot and Greg just clarified in an unrelated thread
that the values are read-write unless explicitly stated otherwise, so
Svadu implies Svade, and we're misusing the extension string to express
a default configuration parameter.

>> Svade is mandatory in RVA profiles, so the corner-case case it not
>> anything I expect to see...
>
> Yes, I expect if Svadu-only harts are built that they likely will make
> ADUE read-only 1, so I don't expect to see problems with just
> unconditionally setting the bit to 1, as we do here (and should set
> henvcfg.ADUE in KVM as well).

Right, we'd only have an issue if software assumed that Svadu without
Svade means that it doesn't have to set the bit to 1 to avoid Svade.

-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi