[kvm-unit-tests PATCH v2 02/11] riscv: sbi: Add fwft pte_hw_ad_updating test

[Clément Léger]

On 27/02/2025 15:19, Andrew Jones wrote:
> Add SBI FWFT tests for the PTE_HW_AD_UPDATING feature.
> 
> Signed-off-by: Andrew Jones <andrew.jones at linux.dev>
> ---
>  riscv/sbi-fwft.c | 170 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 170 insertions(+)
> 
> diff --git a/riscv/sbi-fwft.c b/riscv/sbi-fwft.c
> index f3408d8de081..ca4d49b5db85 100644
> --- a/riscv/sbi-fwft.c
> +++ b/riscv/sbi-fwft.c
> @@ -5,9 +5,12 @@
>   * Copyright (C) 2024, Rivos Inc., Clément Léger <cleger at rivosinc.com>
>   */
>  #include <libcflat.h>
> +#include <alloc.h>
>  #include <stdlib.h>
>  
>  #include <asm/csr.h>
> +#include <asm/io.h>
> +#include <asm/mmu.h>
>  #include <asm/processor.h>
>  #include <asm/ptrace.h>
>  #include <asm/sbi.h>
> @@ -37,6 +40,21 @@ static struct sbiret fwft_get(uint32_t feature)
>  	return fwft_get_raw(feature);
>  }
>  
> +static struct sbiret fwft_set_and_check_raw(const char *str, unsigned long feature,
> +					    unsigned long value, unsigned long flags)
> +{
> +	struct sbiret ret;
> +
> +	ret = fwft_set_raw(feature, value, flags);
> +	if (!sbiret_report_error(&ret, SBI_SUCCESS, "set to %ld%s", value, str))
> +		return ret;
> +
> +	ret = fwft_get_raw(feature);
> +	sbiret_report(&ret, SBI_SUCCESS, value, "get %ld after set%s", value, str);
> +
> +	return ret;
> +}
> +
>  static void fwft_check_reserved(unsigned long id)
>  {
>  	struct sbiret ret;
> @@ -160,6 +178,157 @@ static void fwft_check_misaligned_exc_deleg(void)
>  	report_prefix_pop();
>  }
>  
> +static bool adue_triggered_read, adue_triggered_write;
> +
> +static void adue_set_ad(unsigned long addr, pteval_t prot)
> +{
> +	pte_t *ptep = get_pte(current_pgtable(), addr);
> +	*ptep = __pte(pte_val(*ptep) | prot);
> +	local_flush_tlb_page(addr);
> +}
> +
> +static void adue_read_handler(struct pt_regs *regs)
> +{
> +	adue_triggered_read = true;
> +	adue_set_ad(regs->badaddr, _PAGE_ACCESSED);
> +}
> +
> +static void adue_write_handler(struct pt_regs *regs)
> +{
> +	adue_triggered_write = true;
> +	adue_set_ad(regs->badaddr, _PAGE_ACCESSED | _PAGE_DIRTY);
> +}
> +
> +static bool adue_check_pte(pteval_t pte, bool write)
> +{
> +	return (pte & (_PAGE_ACCESSED | _PAGE_DIRTY)) == (_PAGE_ACCESSED | (write ? _PAGE_DIRTY : 0));
> +}
> +
> +static void adue_check(bool hw_updating_enabled, bool write)
> +{
> +	unsigned long *ptr = malloc(sizeof(unsigned long));
> +	pte_t *ptep = get_pte(current_pgtable(), (uintptr_t)ptr);
> +	bool *triggered;
> +	const char *op;
> +
> +	WRITE_ONCE(adue_triggered_read, false);
> +	WRITE_ONCE(adue_triggered_write, false);
> +
> +	*ptep = __pte(pte_val(*ptep) & ~(_PAGE_ACCESSED | _PAGE_DIRTY));
> +	local_flush_tlb_page((uintptr_t)ptr);
> +
> +	if (write) {
> +		op = "write";
> +		triggered = &adue_triggered_write;
> +		writel(0xdeadbeef, ptr);
> +	} else {
> +		op = "read";
> +		triggered = &adue_triggered_read;
> +		readl(ptr);
> +	}
> +
> +	report(hw_updating_enabled != *triggered &&
> +	       adue_check_pte(pte_val(*ptep), write), "hw updating %s %s",
> +	       hw_updating_enabled ? "enabled" : "disabled", op);
> +
> +	free(ptr);
> +}
> +
> +static bool adue_toggle_and_check_raw(const char *str, unsigned long feature, unsigned long value,
> +				      unsigned long flags)
> +{
> +	struct sbiret ret = fwft_set_and_check_raw(str, feature, value, flags);
> +
> +	if (!ret.error) {
> +		adue_check(value, false);
> +		adue_check(value, true);
> +		return true;
> +	}
> +
> +	return false;
> +}
> +
> +static bool adue_toggle_and_check(const char *str, unsigned long value, unsigned long flags)
> +{
> +	return adue_toggle_and_check_raw(str, SBI_FWFT_PTE_AD_HW_UPDATING, value, flags);
> +}
> +
> +static void fwft_check_pte_ad_hw_updating(void)
> +{
> +	struct sbiret ret;
> +	bool enabled;
> +
> +	report_prefix_push("pte_ad_hw_updating");
> +
> +	ret = fwft_get(SBI_FWFT_PTE_AD_HW_UPDATING);
> +	if (ret.error == SBI_ERR_NOT_SUPPORTED) {
> +		report_skip("not supported by platform");
> +		return;
> +	} else if (!sbiret_report_error(&ret, SBI_SUCCESS, "get")) {
> +		/* Not much we can do without a working get... */
> +		return;
> +	}
> +
> +	report(ret.value == 0 || ret.value == 1, "first get value is 0/1");
> +
> +	enabled = ret.value;
> +	report_kfail(true, !enabled, "resets to 0");
> +
> +	install_exception_handler(EXC_LOAD_PAGE_FAULT, adue_read_handler);
> +	install_exception_handler(EXC_STORE_PAGE_FAULT, adue_write_handler);
> +
> +	adue_check(enabled, false);
> +	adue_check(enabled, true);
> +
> +	if (!adue_toggle_and_check("", !enabled, 0))
> +		goto adue_inval_tests;
> +	else
> +		enabled = !enabled;
> +
> +	if (!adue_toggle_and_check(" again", !enabled, 0))
> +		goto adue_inval_tests;
> +	else
> +		enabled = !enabled;
> +
> +#if __riscv_xlen > 32
> +	if (!adue_toggle_and_check_raw(" with high feature bits set",
> +				       BIT(32) | SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0))
> +		goto adue_inval_tests;
> +	else
> +		enabled = !enabled;
> +#endif
> +
> +adue_inval_tests:
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, 2, 0);
> +	sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to 2");
> +
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 2);
> +	sbiret_report_error(&ret, SBI_ERR_INVALID_PARAM, "set to %d with flags=2", !enabled);
> +
> +	if (!adue_toggle_and_check(" with lock", !enabled, 1))
> +		goto adue_done;
> +	else
> +		enabled = !enabled;
> +
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 0);
> +	sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", !enabled);
> +
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, !enabled, 1);
> +	sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", !enabled);> +
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 0);
> +	sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d without lock", enabled);
> +
> +	ret = fwft_set(SBI_FWFT_PTE_AD_HW_UPDATING, enabled, 1);
> +	sbiret_report_error(&ret, SBI_ERR_DENIED_LOCKED, "set locked to %d with lock", enabled);

Hi Andrew,

As I said, this could probably be factorized in some function common to
all features since it will (probably) be done by other tests as well.
But that can be done at a later time as well.

> +
> +adue_done:
> +	install_exception_handler(EXC_LOAD_PAGE_FAULT, NULL);
> +	install_exception_handler(EXC_STORE_PAGE_FAULT, NULL);
> +
> +	report_prefix_pop();
> +}
> +
>  void check_fwft(void)
>  {
>  	report_prefix_push("fwft");
> @@ -172,6 +341,7 @@ void check_fwft(void)
>  
>  	fwft_check_base();
>  	fwft_check_misaligned_exc_deleg();
> +	fwft_check_pte_ad_hw_updating();
>  
>  	report_prefix_pop();
>  }

With or without my previous comment fixed:

Reviewed-by: Clément Léger <cleger at rivosinc.com>

Thanks,

Clément