Re: [PATCH v1 4/6] xen/riscv: introduce cache management operations (CMO)

[Jan Beulich <jbeulich@xxxxxxxx>]

On 27.11.2024 13:50, Oleksii Kurochko wrote:
> --- a/xen/arch/riscv/Kconfig
> +++ b/xen/arch/riscv/Kconfig
> @@ -14,6 +14,9 @@ config ARCH_DEFCONFIG
>       string
>       default "arch/riscv/configs/tiny64_defconfig"
>  
> +config HAS_CMO # Cache Management Operations
> +     bool

Hmm, and nothing ever sets this, and hence ...

> @@ -148,9 +149,24 @@ static inline bool pte_is_mapping(pte_t p)
>      return (p.pte & PTE_VALID) && (p.pte & PTE_ACCESS_MASK);
>  }
>  
> +#ifndef HAS_CMO
> +static inline int clean_and_invalidate_dcache_va_range(const void *p, 
> unsigned long size)
> +{
> +    return -EOPNOTSUPP;
> +}
> +
> +static inline int clean_dcache_va_range(const void *p, unsigned long size)
> +{
> +    return -EOPNOTSUPP;
> +}
> +#else
> +int clean_and_invalidate_dcache_va_range(const void *p, unsigned long size);
> +int clean_dcache_va_range(const void *p, unsigned long size);
> +#endif

... all you really provide are stubs and declarations, but no
definition anywhere?

Plus of course this gets us into feature detection territory again: If
RISC-V provided a way to detect presence / absence of certain extensions,
this really shouldn't be a compile time setting, but be determined
dynamically.

>  static inline void invalidate_icache(void)
>  {
> -    BUG_ON("unimplemented");
> +    asm volatile ( "fence.i" ::: "memory" );
>  }

That's a separate extension, Zifencei, which I don't think you can just
assume to be present?

Jan