[PATCH v8 5/5] xen/riscv: rework the A/D bits comment in p2m_set_pte_flags()

[Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>]

The comment in p2m_set_pte_flags() quotes the spec at length to describe
the two A/D bits schemes, and its TODO mixes two things: why A/D are preset
today (OpenSBI delegates the resulting page faults to a lower privilege
mode, so Xen would take them with no handler) and what remains to be done.

Summarize the two schemes as a short list. The reason for presetting A/D is
not future work, so move it out of the TODO, next to the schemes. The TODO
now keeps only the later work, handling these page faults instead of
presetting A/D, which only concerns Svade as A/D page faults cannot happen
otherwise.

Also note that presetting the A/D bits is harmless when the device
tree advertises both Svade and Svadu and the platform uses hardware
A/D updating.

No functional change.

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Baptiste Le Duc <baptiste.le-duc@xxxxxxxxxx>
Reviewed-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
---
Changes since v7:
- collect R-by.
- move the patch last, as the previous patch now records Svade whenever
  the CPU faults on clear A/D bits, instead of riscv_fill_hwcap()
  panicking when the device tree advertises neither Svade nor Svadu.
---
Changes since v6:
- drop the change to preset A/D unless only Svadu is advertised, as
  riscv_fill_hwcap() now panics if the device tree advertises neither
  Svade nor Svadu. The patch only reworks the comment.
- update the patch's title accordingly.
---
Changes since v5:
- new patch, split out of "xen/riscv: always preset A/D bits in G-stage
  PTEs".
---
 xen/arch/riscv/p2m.c | 45 +++++++++++++++++++--------------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/xen/arch/riscv/p2m.c b/xen/arch/riscv/p2m.c
index 3d195294d5..a2d81a9c8a 100644
--- a/xen/arch/riscv/p2m.c
+++ b/xen/arch/riscv/p2m.c
@@ -591,35 +591,28 @@ static void p2m_set_pte_flags(pte_t *e, p2m_type_t t)
     e->pte |= PTE_USER;
 
     /*
-     * Two schemes to manage the A and D bits are defined:
-     *   • The Svade extension: when a virtual page is accessed and the A bit
-     *     is clear, or is written and the D bit is clear, a page-fault
-     *     exception is raised.
-     *   • When the Svade extension is not implemented, the following scheme
-     *     applies.
-     *     When a virtual page is accessed and the A bit is clear, the PTE is
-     *     updated to set the A bit. When the virtual page is written and the
-     *     D bit is clear, the PTE is updated to set the D bit. When G-stage
-     *     address translation is in use and is not Bare, the G-stage virtual
-     *     pages may be accessed or written by implicit accesses to VS-level
-     *     memory management data structures, such as page tables.
-     * Thereby to avoid a page-fault in case of Svade is available, it is
-     * necessary to set A and D bits.
+     * A RISC-V implementation can either:
+     * 1) Update the 'A' and 'D' PTE bits in hardware.
+     * 2) Generate a page fault when 'A' and/or 'D' is clear, so that
+     *    software can set them.
      *
-     * TODO: For now, it’s fine to simply set the A/D bits, since OpenSBI
-     *       delegates page faults to a lower privilege mode and so OpenSBI
-     *       isn't expect to handle page-faults occured in lower modes.
-     *       By setting the A/D bits here, page faults that would otherwise
-     *       be generated due to unset A/D bits will not occur in Xen.
+     * (1) needs nothing more as it is handled by hardware. For (2), set 'A'
+     * and 'D' up front: OpenSBI delegates such page faults to a lower
+     * privilege mode, so they would be raised in Xen, which has no handler
+     * for them.
      *
-     *       Currently, Xen on RISC-V does not make use of the information
-     *       that could be obtained from handling such page faults, which
-     *       could otherwise be useful for several use cases such as demand
-     *       paging, cache-flushing optimizations, memory access tracking,etc.
+     * If both Svade and Svadu are advertised and the CPU updates A/D in
+     * hardware, presetting them is harmless.
      *
-     *       To support the more general case and the optimizations mentioned
-     *       above, it would be better to stop setting the A/D bits here and
-     *       instead handle page faults that occur due to unset A/D bits.
+     * TODO: Xen on RISC-V currently makes no use of the information that
+     *       handling such page faults could provide, which would be useful
+     *       for several use cases such as demand paging, cache-flushing
+     *       optimizations, memory access tracking, etc.
+     *
+     *       To support the more general case and the optimizations
+     *       mentioned above, it would be better to stop setting the A/D
+     *       bits here and instead handle the page faults that occur due to
+     *       unset A/D bits.
      */
     if ( riscv_isa_extension_available(NULL, RISCV_ISA_EXT_svade) )
         e->pte |= PTE_ACCESSED | PTE_DIRTY;

-- 
2.56.0