Re: [PATCH v1 14/17] xen/riscv: add guest page fault handling stub

On 8/19/26 11:52 AM, Jan Beulich wrote:
> On 19.08.2026 10:59, Oleksii Kurochko wrote:
>> On 8/18/26 6:04 PM, Oleksii Kurochko wrote:
>>> On 8/18/26 10:29 AM, Jan Beulich wrote:
>>>> On 17.08.2026 18:10, Oleksii Kurochko wrote:
>>>>> On 8/12/26 5:48 PM, Jan Beulich wrote:
>>>>>> On 20.07.2026 18:02, Oleksii Kurochko wrote:
>>>>>>> +{
>>>>>>> +    /*
>>>>>>> +     * According to RISC-V spec:
>>>>>>> +     *  18.2.8. Hypervisor Trap Value Register (htval)
>>>>>>> +     *   ...
>>>>>>> +     *   A guest physical address written to htval is shifted
>>>>>>> right by 2 bits
>>>>>>> +     *   to accommodate addresses wider than the current XLEN.
>>>>>>> +     *   ...
>>>>>>> +     *   If the least-significant two bits of a faulting guest
>>>>>>> physical address
>>>>>>> +     *   are needed, these bits are ordinarily the same as the
>>>>>>> +     *   least-significant two bits of the faulting virtual
>>>>>>> address in stval.
>>>>>>> +     *   For faults due to implicit memory accesses for VS-stage
>>>>>>> address
>>>>>>> +     *   translation, the least-significant two bits are instead
>>>>>>> zeros. These
>>>>>>> +     *   cases can be distinguished using the value provided in
>>>>>>> register htinst.
>>>>>>> +     */
>>>>>>> +    return (csr_read(CSR_HTVAL) << 2) | (csr_read(CSR_STVAL) & 0x3);
>>>>>>
>>>>>> Well, okay, but instead of not losing the bottom two bits you're now
>>>>>> losing
>>>>>> the top two ones.
>>>>>
>>>>> Oh, right, I will add a cast ((uint64_t)csr_read(CSR_HTVAL) << 2) | ...
>>>>>
>>>>> It will cover all the cases RV32 which has 34-bit guest address and it
>>>>> will be enough for RV64 where GPA is 59bit (the highest possible for
>>>>> Sv59).
>>>>
>>>> Only if the function return type then also changes.
>>>>
>>>>>> Also the spec reads as if htval only _may_ hold the original address
>>>>>> of the
>>>>>> faulting access. What if htval ends up 0?
>>>>>
>>>>> good point. then we have to emulate fault instruction and get an address
>>>>> from an instruction. I think that for now it will be enough just to
>>>>> support platforms which always write GPA to HTVAL.
>>>>>
>>>>> If I understand correctly if htval is supported by platform then htval
>>>>> will be always filled for guest page fault. To verify if HTVAL is
>>>>> supported we could do:
>>>>>
>>>>> 'Unless it has reason to assume otherwise (such as a platform standard),
>>>>> software that writes a value to htval should read back from htval to
>>>>> confirm the stored value.'
>>>>
>>>> How does this matter here? It's one thing for htval to be capable of
>>>> holding (all?) non-zero values, and another that it would always be
>>>> written. If the platform doesn't indicate the behavior, I fear you have
>>>> to assume that you may (perhaps even randomly) observe 0.
>>>
>>> So to be very sure we could check for two extensions: Sstval and Shtval.
>>> They will guarantee that under any circumstances it will be filled.
>>>
>>> Also, as an option we could check that htinst value isn't zero as
>>> according to the spec:
>>>
>>> For guest-page faults, the trap instruction register is written with a
>>> special pseudoinstruction value if:
>>> (a) the fault is caused by an implicit memory access for VS-stage
>>> address translation, and (b) a nonzero
>>> value (the faulting guest physical address) is written to mtval2 or htval.
>>>
>>> So if htinst != 0 then htval is filled with GPA and a nonzero guest
>>> physical address written to mtval2/htval shall correspond to the exact
>>> virtual address written to mtval/stval.
>>
>> I've re-read SPEC again and it looks like htinst != 0 doesn't guarantee
>> that htval and stval will contain necessary for me here faulty
>> instruction. What I wrote above guarantee that if a fault during
>> VS-stage translation failed then it htval will contatain GPA of PTE with
>> which was an issue.
>>
>> So I have to blindly believe that HTVAL and STVAL will always contain
>> necessary for me data as KVM and other hypervisor does or introduce here
>> software guest page table walker if Sstvala and Shtvala aren't provided
>> by platform.
> 
> Why "blindly believe"? Checking for the necessary extension(s) should be
> an option. Adding fallback code for when an extension isn't available can
> come later, can't it?

It can be an option. It is what I planned to do.

~ Oleksii