Hello Andrew,
On Fri, Feb 28, 2025 at 4:47 PM Andrew Cooper <andrew.coop...@citrix.com> wrote:
>
> On 28/02/2025 2:59 pm, Milan Djokic wrote:
> > From: Slavisa Petrovic <slavisa.petro...@rt-rk.com>
> >
> > 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.djo...@rt-rk.com>
> > Signed-off-by: Slavisa Petrovic <slavisa.petro...@rt-rk.com>
> > ---
> > 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
> 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.
> > diff --git a/xen/arch/riscv/addr_translation.S
> > b/xen/arch/riscv/addr_translation.S
> > new file mode 100644
> > index 0000000000..7e94774b47
> > --- /dev/null
> > +++ b/xen/arch/riscv/addr_translation.S
> > @@ -0,0 +1,63 @@
> > +#include <asm/riscv_encoding.h>
> > +#include <asm/asm.h>
> > +
> > +.macro add_extable lbl
> > +.pushsection .extable, "a" /* Start .extable section */
> > +.balign 8 /* Align to 8 bytes */
> > +.quad \lbl /* Add label address to extable */
> > +.popsection /* End section */
> > +.endm
> > +
> > +.section .text
> > +
> > +/*
> > + * size_t _copy_to(uint64_t dest, const void *src, size_t len)
> > + *
> > + * a0 - dest
> > + * a1 - src
> > + * a2 - len
> > + *
> > + */
> > +
> > +.global _copy_to
> > +_copy_to:
>
> For assembly code, please use the linkage macros. See 7015f337a217 as
> an example.
>
> However, as far as I can tell, the only interesting thing h{s,l}v.b, at
> which point a simple piece of inline asm is surely easier, and would
> simplify guestcopy.c too.
>
> Exception table entries are perfectly easy to do in inline asm. See
> _ASM_EXTABLE() in x86 for an example.
>
We have updated the patch where this part is done completely in c with
inline assembly for hsv/hlv.
Prepared for the next patch version, thanks for the detailed explanation.
Best regards,
Milan
