Re: [PATCH] xen/riscv: copy_to_guest/copy_from_guest functionality.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/03/2025 11:59 am, Milan Đokić wrote:
> Hello Andrew,
> On Fri, Feb 28, 2025 at 4:47 PM Andrew Cooper <andrew.cooper3@xxxxxxxxxx> 
> wrote:
>> On 28/02/2025 2:59 pm, Milan Djokic wrote:
>>> From: Slavisa Petrovic <Slavisa.Petrovic@xxxxxxxxx>
>>>
>>> This patch implements copy_to_guest/copy_from_guest functions for RISC-V.
>>> These functions are designed to facilitate data exchange between guest and 
>>> hypervisor.
>>>
>>> Signed-off-by: Milan Djokic <Milan.Djokic@xxxxxxxxx>
>>> Signed-off-by: Slavisa Petrovic <Slavisa.Petrovic@xxxxxxxxx>
>>> ---
>>> Tested on qemu with xcp-ng latest branch 
>>> https://gitlab.com/xen-project/people/olkur/xen/-/merge_requests/6
>>> Full description on how to setup test environment can be found in attached 
>>> PR link (Linux kernel patching to support copy_from_guest/copy_to_guest for 
>>> RISC-V).
>>> Linux kernel patch shall be upstreamed after these changes are merged.
>> Several things.  First, it's probably worth setting you up with Gitlab
>> access, seeing as that's the first step of RISC-V CI.
>>
> We already have access to mirror gitlab repo owned by Oleksii where
> our changes are tested through CI jobs
> Or you refer to mainstream xen gitlab? For this one, I don't know who
> should we contact for access

I wanted to check that you were running your patches through regular CI,
but if you can use Oleksii's tree then that's fine.

>
>> Second, where can I read about the semantics of h{l,s}v?  That looks
>> alarmingly like a virtual address, and there's a world supply of corner
>> cases that can come with it, including incorrect translations.
>>
>> Also, I very desperately want RISC-V (and PPC) not to inherit
>> 2-decade-old x86-ISMs which we're currently trying to remove, because
>> starting without them is 10x easier than to fix them after the fact.
>>
> hlv/hsv are part of the RISC-V ISA H extension
> (https://five-embeddev.com/riscv-priv-isa-manual/Priv-v1.12/hypervisor.html,
> Chapter 5.3 Hypervisor Instructions).
> Handling of corner cases with possible incorrect translations will be
> part of the upcoming patch version.

Ok, that's interesting.  Thanks for sharing.

There are separate instructions for data and instruction accesses, which
avoids the main source of incorrect translations.  I don't see anything
requiring accesses to be coherent with the TLB (ARM for example permits
the AT instruction to bypass the TLBs).

However, they're all virtual address based.  The first wrinkle here is
already realised in the documentation; the mode of the access is based
on hstatus.spvp.

The reason we're getting rid of virtual addressed based accesses is that
there is not an appropriate mode to use for hypervisor access in a
binary permissions system (supervisor/user), and it fails completely in
the presence of SMEP/SMAP (x86) or PXN/PAN (ARM).

The introduction to 5.5 Two-Stage Translation suggests we can turn off
guest virtual translations by zeroing vsatp which I suspect means we can
"turn" HLV/HSV into guest-physical accessors.

~Andrew