Re: [PATCH v2 18/39] xen/riscv: add guest page fault handling stub

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 9/7/26 5:57 PM, Baptiste Le Duc wrote:
> > Add a handler for guest page faults and hook it into the trap path,
> > providing the trap-side entry point which will later feed the MMIO
> > dispatch.
> >
> > This will be used, for example, to trap accesses to APLIC registers so
> > that a guest can initialize and drive an emulated interrupt controller.
> >
> > Two of the situations handled here are already decided, as neither can
> > ever be turned into an emulated access:
> >
> >   - A fault reported with a pseudoinstruction in htinst was taken on an
> >     implicit access made for VS-stage address translation, so htval holds
> >     the address of a VS-stage PTE rather than of anything the guest asked
> >     for, and the guest physical address behind the original access is not
> >     known. This is orthogonal to the cause and can accompany any of the
> >     three, which is why it is checked first. scause keeps reporting the
> >     type of the original access, and on bare hardware a PTE which cannot
> >     be read raises an access fault of exactly that type, so reflect one
> >     back to the guest.
> >
> >   - A fetch fault means the guest tried to execute from a guest physical
> >     address which is unmapped or which G-stage does not allow to be
> >     executed. On bare hardware a fetch from physical memory which does
> >     not exist, or which may not be executed, raises an instruction access
> >     fault, so reflect one back too.
> >
> > Explicit loads and stores are where MMIO emulation will hook in.
> >
> > Neither of the two paths above consults the p2m first, and neither will
> > the MMIO one: RISC-V has no populate-on-demand, no paging and no
> > mem_access, so every guest mapping is established eagerly and a G-stage
> > fault never denotes a mapping Xen could install to let the faulting
> > access complete.
> >
> > Both of the helpers this leans on, resolve_faulting_gpa() and
> > trap_redirect(), are BUG_ON() placeholders for now, so each of the three
> > causes currently takes the host down rather than the domain. That is no
> > worse than before this patch, where the same causes fell through to
> > do_unexpected_trap() and die(). Implementing the helpers is left to
> > later patches.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> I think the commit message it not very clear as it conflates two
> different countable things (the 3 fault causes reported via scause vs.
> the pseudoinstruction condition, which is orthogonal and can accompany
> any of them), which makes "two situations decided" or "any of the three"
> hard to follow on first read.
>
> Here is a proposition with a split to distinguish fetch-fault and
> pseudoinstruction cases into explicit bullets with their outcome stated
> ("Decided here"), and added spec-mentioned conditions about the
> pseudoinstruction's existence conditions:
> ```
>      Add a handler for guest page faults and hook it into the trap path,
>      providing the trap-side entry point which will later feed the MMIO
>      dispatch.
>
>      This will be used, for example, to trap accesses to APLIC registers so
>      that a guest can initialize and drive an emulated interrupt controller.
>
>      A G-stage (stage-2) fault has one of three causes, reported via scause:
>
>          - Fetch fault: guest tried to execute from a guest-physical address
>          that is unmapped or that G-stage marks non-executable. Never
>          emulatable (nothing to emulate a fetch into). On real hardware
>          this raises an instruction access fault, so Xen reflects the same
>          fault back to the guest. Decided here.
>
>          - Load fault / Store fault: left undecided by this patch, this is
>          where MMIO emulation will hook in later.
>
>      Any of these three faults can instead be reported via a pseudoinstruction
>      in htinst, when both:
>
>          (a) the fault occurred on an implicit access Xen made to walk a
>              VS-stage page table, and
>          (b) htval holds a nonzero value: the guest-physical address of that
>              VS-stage PTE, not of the guest's original access.
>
>      However, none of these paths consult the p2m first, and the future MMIO 
> path
>      won't either: RISC-V has no populate-on-demand, no paging, and no
>      mem_access, so every guest mapping is established eagerly. A G-stage
>      fault therefore never indicates a mapping Xen could lazily resolve to
>      let the access complete.
>
>      Both helpers this handler relies on, resolve_faulting_gpa() and
>      trap_redirect(), are BUG_ON() placeholders for now, so all three causes
>      currently take the host down instead of just the guest. This is no worse
>      than before this patch, where these traps fell through to
>      do_unexpected_trap() and die().
> ```
I will apply your suggestion.

Thanks.

~ Oleksii