Yo, On Thu, Jan 23, 2025 at 03:46:36PM +0100, Oleksii Kurochko wrote: > Supported ISA extensions are specified in the device tree within the CPU > node, using two properties: `riscv,isa-extensions` and `riscv,isa`. > > Currently, Xen does not support the `riscv,isa-extensions` property, as > all available device tree source (DTS) files in the Linux kernel (v6.12-rc3) > and DTBs generated by QEMU use only the `riscv,isa` property.
That's not true? The riscv,isa-extensions property went into linux in
late 2023 (6.7 or so) and QEMU in v9.0.0 about a year ago, so all source
files in linux and blobs generated by QEMU should have both. OpenSBI &
U-Boot support both also. Might not affect your decision about what to
support here - but the rationale provided for implementing the deprecated
(per the binding at least) property isn't accurate.
> Therefore, only `riscv,isa` parsing is implemented.
>
> The `riscv,isa` property is parsed for each CPU, and the common extensions
> are stored in the `host_riscv_isa` bitmap.
> This bitmap is then used by `riscv_isa_extension_available()` to check
> if a specific extension is supported.
>
> The current implementation is based on Linux kernel v6.12-rc3
> implementation with the following changes:
> - Drop unconditional setting of {RISCV_ISA_EXT_ZICSR,
> RISCV_ISA_EXT_ZIFENCEI, RISCV_ISA_EXT_ZICNTR, RISCV_ISA_EXT_ZIHPM} as they
> are now part of the riscv,isa string.
Hmm, not sure I follow your logic here. Linux unconditionally sets these
extensions because the binding was written when these these were part of
the base instruction set*. That they can be put into the "riscv,isa"
string is actually the reason that the code setting them unconditionally
in linux exists! Linux cannot tell the difference between an "old" dtb
containing "rv64ima" meaning that Zicsr is present & a "new" one containing
"rv64ima" meaning that it is not! For backwards compatibility reasons,
linux is then forced to set its internal flags for Zicsr et al when it sees
"i" in riscv,isa. If you were to use the "new" definition of "i", and use
that to construct a dtb to pass to a linux guest, things would blow up.
This is the whole reason that riscv,isa is marked deprecated in the binding
and replaced by riscv,isa-extensions - there are no concrete definitions
for what extensions mean using "riscv,isa".
I suppose you're free to decide to interpret the property in Xen
differently to linux - particularly because the hypervisor extension
requirement means that you're only going to run on hardware produced after
the aforementioned extensions were split out of "i" - but if you are
doing that, the rationale for a differing interpretation should be correct
IMO.
Perhaps the wording of my comment in linux was not "strong" enough, and
the "can be set" should be changed to "must be set"?
/*
* These ones were as they were part of the base ISA when the
* port & dt-bindings were upstreamed, and so can be set
* unconditionally where `i` is in riscv,isa on DT systems.
*/
if (acpi_disabled) {
set_bit(RISCV_ISA_EXT_ZICSR, source_isa);
set_bit(RISCV_ISA_EXT_ZIFENCEI, source_isa);
set_bit(RISCV_ISA_EXT_ZICNTR, source_isa);
set_bit(RISCV_ISA_EXT_ZIHPM, source_isa);
}
* IIRC only 2 of them were part of i, the other two were assumed to be present
by Linux etc and only became independent extensions later on.
> - Remove saving of the ISA for each CPU, only the common available ISA is
> saved.
> - Remove ACPI-related code as ACPI is not supported by Xen.
> - Drop handling of elf_hwcap, since Xen does not provide hwcap to
> userspace.
> - Replace of_cpu_device_node_get() API, which is not available in Xen,
> with a combination of dt_for_each_child_node(), dt_device_type_is_equal(),
> and dt_get_cpuid_from_node() to retrieve cpuid and riscv,isa in
> riscv_fill_hwcap_from_isa_string().
> - Rename arguments of __RISCV_ISA_EXT_DATA() from _name to ext_name, and
> _id to ext_id for clarity.
> - Replace instances of __RISCV_ISA_EXT_DATA with RISCV_ISA_EXT_DATA.
> - Replace instances of __riscv_isa_extension_available with
> riscv_isa_extension_available for consistency. Also, update the type of
> `bit` argument of riscv_isa_extension_available().
> - Redefine RISCV_ISA_EXT_DATA() to work only with ext_name and ext_id,
> as other fields are not used in Xen currently.
> - Add check of first 4 letters of riscv,isa string to
> riscv_isa_parse_string() as Xen doesn't do this check before so it is
> necessary to check correctness of riscv,isa string. ( it should start with
> rv{32,64} with taking into account upper and lower case of "rv").
> - Drop an argument of riscv_fill_hwcap() and
> riscv_fill_hwcap_from_isa_string()
> as it isn't used, at the moment.
> - Update the comment message about QEMU workaround.
Does Xen (for riscv) even work with QEMU 7.1?
Cheers,
Conor.
signature.asc
Description: PGP signature
