Re: [PATCH v2 03/16] xen/riscv: introduce support of Svpbmt extension

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.05.2025 18:51, Oleksii Kurochko wrote:
> Svpbmt extension is necessary for chaning the memory type for a page contains
> a combination of attributes that indicate the cacheability, idempotency,
> and ordering properties for access to that page.

The title suggest use of the extension is optional.

> --- a/xen/arch/riscv/Kconfig
> +++ b/xen/arch/riscv/Kconfig
> @@ -10,11 +10,25 @@ config RISCV
>  config RISCV_64
>       def_bool y
>       select 64BIT
> +     select HAS_SVPBMT

Such redundant ...

>  config ARCH_DEFCONFIG
>       string
>       default "arch/riscv/configs/tiny64_defconfig"
>  
> +config HAS_SVPBMT
> +     bool
> +     depends on RISCV_64

... dependencies are frowned upon, afaik. And it's pretty certainly not
needed here.

> +     help
> +       This config enables usage of Svpbmt ISA-extension ( Supervisor-mode:
> +       page-based memory types).
> +
> +       The memory type for a page contains a combination of attributes
> +       that indicate the cacheability, idempotency, and ordering
> +       properties for access to that page.
> +
> +       The Svpbmt extension is only available on 64-bit cpus.

I don't mind the help text, but for a prompt-less option it's of little
use (beyond what a comment could also achieve).

> --- a/xen/arch/riscv/include/asm/page.h
> +++ b/xen/arch/riscv/include/asm/page.h
> @@ -46,6 +46,8 @@
>  #define PAGE_HYPERVISOR_RX          (PTE_VALID | PTE_READABLE | 
> PTE_EXECUTABLE)
>  
>  #define PAGE_HYPERVISOR             PAGE_HYPERVISOR_RW
> +#define PAGE_HYPERVISOR_NOCACHE     (PAGE_HYPERVISOR_RW | PTE_PMBT_IO)
> +#define PAGE_HYPERVISOR_WC          (PAGE_HYPERVISOR_RW | PTE_PMBT_NOCACHE)

Hmm, odd - NOCACHE doesn't really mean "no cache" then? I think this
would require a comment then.

> @@ -56,8 +58,21 @@
>  #define PTE_SMALL       BIT(10, UL)
>  #define PTE_POPULATE    BIT(11, UL)
>  
> +/*
> + * [62:61] Svpbmt Memory Type definitions:
> + *
> + *  00 - PMA    Normal Cacheable, No change to implied PMA memory type
> + *  01 - NC     Non-cacheable, idempotent, weakly-ordered Main Memory
> + *  10 - IO     Non-cacheable, non-idempotent, strongly-ordered I/O memory
> + *  11 - Rsvd   Reserved for future standard use
> + */
> +#define PTE_PMBT_NOCACHE    BIT(61, UL)
> +#define PTE_PMBT_IO         BIT(62, UL)

Unlike PTE_SMALL and PTE_POPULATE these are arch-defined; I think they
want to move up to where the other arch-defined bits are, thus also
maping them appear before their first use.

Jan