Re: [PATCH v6 1/4] xen/riscv: probe the PTE A/D bits update scheme at boot

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 10/13/26 5:24 PM, Jan Beulich wrote:
> > > > --- a/xen/arch/riscv/include/asm/cpufeature.h
> > > > +++ b/xen/arch/riscv/include/asm/cpufeature.h
> > > > @@ -42,6 +42,7 @@ enum riscv_isa_ext_id {
> > > >       RISCV_ISA_EXT_sstc,
> > > >       RISCV_ISA_EXT_svade,
> > > >       RISCV_ISA_EXT_svpbmt,
> > > > +    RISCV_ISA_EXT_svadu,
> > > Isn't there some sort of sorting in effect here?
> > No, as it is mentioned in the docstring that:
> >      * New extensions should just be added to the bottom, rather than added
> >      * alphabetically, in order to avoid unnecessary shuffling.
> I see. Yet I wonder: What shuffling would we need to be afraid of? Always
> inserting at the bottom increases the risk of patch conflicts. Furthermore
> single-letter extensions can't be inserted at the bottom anyway.
None, really. The comment was copied from Linux together with the first
version of this code, where the IDs were plain macros with explicit values:

  #define RISCV_ISA_EXT_SSTC      27
  #define RISCV_ISA_EXT_SVINVAL   28
  ...

With such definitions an alphabetical insertion means renumbering every
following line, which is the "shuffling" the comment refers to (see 
Linux commit dac8bf14bb49 "riscv: hwcap: Don't alphabetize ISA extension 
IDs"). When the IDs were converted to enum riscv_isa_ext_id, the comment 
was kept as is, although the reason for it went away: the values are 
assigned by the compiler and nothing depends on them.
So I think the comment should be updated: drop the "add to the bottom"
part (together with the leftover "macros" wording) and say that
multi-letter extensions follow the same order as riscv_isa_ext[], above
which the ordering rules are already described. Something like:

--- a/xen/arch/riscv/include/asm/cpufeature.h
+++ b/xen/arch/riscv/include/asm/cpufeature.h
@@ -8,13 +8,13 @@
 #include <xen/types.h>

 /*
- * These macros represent the logical IDs of each multi-letter RISC-V ISA
- * extension and are used in the ISA bitmap. The logical IDs start from
- * RISCV_ISA_EXT_BASE, which allows the 0-25 range to be reserved for 
single
- * letter extensions and are used in enum riscv_isa_ext_id.
+ * Logical IDs of RISC-V ISA extensions, used as bit positions in the ISA
+ * bitmap.
  *
- * New extensions should just be added to the bottom, rather than added
- * alphabetically, in order to avoid unnecessary shuffling.
+ * Single-letter extensions use IDs below RISCV_ISA_EXT_BASE and are sorted
+ * alphabetically. Multi-letter extensions start from 
RISCV_ISA_EXT_BASE and
+ * follow the same order as riscv_isa_ext[] (see the ordering rules 
described
+ * above it).
  */
 #define RISCV_ISA_EXT_BASE  26

Accordingly, svadu will go between svade and svpbmt, both in the enum 
and in riscv_isa_ext[].
~ Oleksii