On 12/9/24 3:38 PM, Jan Beulich wrote:
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?
Yes, this was done intentionally because:
- I don't have hardware with the CMO extension, so I can't test it. ( QEMU
doesn't model cache and so
there is no need for CMO extension emulation IIUC )
- The instructions used for these functions may be hardware-specific and exist
only for particular devices.
It seems useful to have something similar to Linux:
https://elixir.bootlin.com/linux/v6.6.64/source/arch/riscv/include/asm/errata_list.h#L135
<https://elixir.bootlin.com/linux/v6.6.64/source/arch/riscv/include/asm/errata_list.h#L135>
(There are also custom instructions for THEAD above this macro.)
We could use|ALT_CMO_OP(...)| inside|clean_and_invalidate_dcache_va_range()|
and|clean_dcache_va_range()|.
However, I think it would be better to introduce or implement these functions
when|HAS_CMO| is set to|y| someday.
As an alternative, we could implement these functions as|panic("need to be
implemented\n")| in case when HAS_CMO=y.
Another option is to drop|HAS_CMO| entirely for now and keep the current
implementation (|return -EOPNOTSUPP|).
However, with this approach, there's a risk of encountering hard-to-debug
issues on platforms with the CMO extension.
And necessity of implementation of these could be missed because there is no
any notification...
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.
This is the next patch I plan to send after this patch series:
https://gitlab.com/xen-project/people/olkur/xen/-/commit/f81ae67c42854073da5403210c9e31de6b0ee5bd
<https://gitlab.com/xen-project/people/olkur/xen/-/commit/f81ae67c42854073da5403210c9e31de6b0ee5bd>
It "detects" available extensions based on a device tree property. While this
is not the best approach
(the ideal solution would be hardware having a register that lists all
available extensions), it seems to be
the best option available at the moment.
Another option I considered was introducing a new SBI call, delegating the
responsibility to OpenSBI
to provide this information.
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?
Based on the specification:
```
Chapter 34. RV32/64G Instruction Set Listings
One goal of the RISC-V project is that it be used as a stable software
development target. For this
purpose, we define a combination of a base ISA (RV32I or RV64I) plus selected
standard extensions
(IMAFD, Zicsr, Zifencei) as a "general-purpose" ISA, and we use the
abbreviation G for the
IMAFDZicsr_Zifencei combination of instruction-set extensions. This chapter
presents opcode maps
and instruction-set listings for RV32G and RV64G
```
and that G is needed to boot Linux kernel ( and so Xen ) I make an assumption
that Zifencei will be always
present.
And based on Linux code
(https://elixir.bootlin.com/linux/v6.12.4/source/arch/riscv/kernel/cpufeature.c#L676
)
when 'i' is present in riscv,isa property zifencei is present unconditionally.
~ Oleksii
