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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

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@gmail.com>
>>
> 
> 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