Message ID | dae753618491b2a6e42f7ed3f24190d0dc13fe3f.1740754166.git.Slavisa.Petrovic@rt-rk.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | xen/riscv: copy_to_guest/copy_from_guest functionality. | expand |
On 28/02/2025 2:59 pm, Milan Djokic wrote: > From: Slavisa Petrovic <Slavisa.Petrovic@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.Djokic@rt-rk.com> > Signed-off-by: Slavisa Petrovic <Slavisa.Petrovic@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. 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. > 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. ~Andrew
On 2/28/25 3:59 PM, Milan Djokic wrote: > From: Slavisa Petrovic<Slavisa.Petrovic@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.Djokic@rt-rk.com> > Signed-off-by: Slavisa Petrovic<Slavisa.Petrovic@rt-rk.com> > --- > Tested on qemu with xcp-ng latest branchhttps://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. > --- > xen/arch/riscv/Makefile | 1 + > xen/arch/riscv/addr_translation.S | 63 +++++++++++++++++++++++ > xen/arch/riscv/arch.mk | 6 ++- > xen/arch/riscv/guestcopy.c | 43 ++++++++++++++++ > xen/arch/riscv/include/asm/guest_access.h | 5 ++ > 5 files changed, 117 insertions(+), 1 deletion(-) > create mode 100644 xen/arch/riscv/addr_translation.S > create mode 100644 xen/arch/riscv/guestcopy.c > > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > index a5eb2aed4b..1bd61cc993 100644 > --- a/xen/arch/riscv/Makefile > +++ b/xen/arch/riscv/Makefile > @@ -10,6 +10,7 @@ obj-y += smp.o > obj-y += stubs.o > obj-y += traps.o > obj-y += vm_event.o > +obj-y += addr_translation.o It should be sorted in alphabetical order. > > $(TARGET): $(TARGET)-syms > $(OBJCOPY) -O binary -S $< $@ > 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: > + la t0, copy_done /* Load address of return label */ > + mv t2, zero /* Initialize counter to zero */ > +loop_check: > + beq t2, a2, copy_done /* Check if all bytes are copied */ > + lb t3, (a1) /* Load byte from source (guest address) */ > +store_byte: > + hsv.b t3, (a0) /* Store byte to destination (host address) */ > + add_extable store_byte /* Add exception table for this location */ > + addi a0, a0, 1 /* Increment destination pointer */ > + addi a1, a1, 1 /* Increment source pointer */ > + addi t2, t2, 1 /* Increment byte counter */ > + j loop_check /* Loop back if not done */ > + > +/* > + * size_t _copy_from(void *dest, uint64_t src, size_t len) > + * > + * a0 - dest > + * a1 - src > + * a2 - len > + * > + */ > + > +.global _copy_from > +_copy_from: > + la t0, copy_done > + mv t2, zero > +loop_check_from: > + beq t2, a2, copy_done > +load_byte: > + hlv.b t3, (a1) /* Load byte from guest address */ > + add_extable load_byte > + sb t3, (a0) > + addi a0, a0, 1 > + addi a1, a1, 1 > + addi t2, t2, 1 > + j loop_check_from > + > +copy_done: > + mv a0, t2 /* Return number of bytes copied */ > + ret Can't we done this functions fully in C? (it doesn't something mandatory) > diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk > index 17827c302c..f4098f5b5e 100644 > --- a/xen/arch/riscv/arch.mk > +++ b/xen/arch/riscv/arch.mk > @@ -23,13 +23,17 @@ $(eval $(1) := \ > $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value $(1)-insn),_$(1))) > endef > > +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h) > +$(h-extension-name)-insn := "hfence.gvma" > +$(call check-extension,$(h-extension-name)) > + > zbb-insn := "andn t0$(comma)t0$(comma)t0" > $(call check-extension,zbb) > > zihintpause-insn := "pause" > $(call check-extension,zihintpause) > > -extensions := $(zbb) $(zihintpause) > +extensions := $(value $(h-extension-name)) $(zbb) $(zihintpause) > > extensions := $(subst $(space),,$(extensions)) This patch should take into account another one patch series (https://lore.kernel.org/xen-devel/cover.1740071755.git.oleksii.kurochko@gmail.com/T/#t) update for which I am going to sent today. Also, these changes would be better to move into separate commit with explanation why what is so specific with 1301 and why it is needed to introduce 'hh'. I believe that these changes were taken based on my patch:https://gitlab.com/xen-project/people/olkur/xen/-/commit/154f75e943f1668dbf2c7be0f0fdff5b30132e89 Probably it will make sense just to get it and rebase on top of mentioned above patch series. > > diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c > new file mode 100644 > index 0000000000..0325956845 > --- /dev/null > +++ b/xen/arch/riscv/guestcopy.c > @@ -0,0 +1,43 @@ > +#include <xen/bug.h> > +#include <xen/domain_page.h> > +#include <xen/errno.h> > +#include <xen/sched.h> > + > +#include <asm/csr.h> > +#include <asm/guest_access.h> > +#include <asm/system.h> > +#include <asm/traps.h> > + > +unsigned long copy_to(uint64_t dest, void* src, size_t len) > +{ > + size_t bytes; > + > + bytes = _copy_to(dest, src, len); > + > + if (bytes == len) > + return 0; > + else > + return -ENOSYS; > +} Why do we have a different prototypes with copy_from() below? If we will have void *dest then ... > + > +unsigned long copy_from(void *dest, uint64_t src, size_t len) > +{ > + size_t bytes; > + > + bytes = _copy_from(dest, src, len); > + > + if (bytes == len) > + return 0; > + else > + return -ENOSYS; > +} > + > +unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > +{ > + return copy_to((vaddr_t)to, (void *)from, len); ... we won't need cast for `to` and wo we really need cast for copy_to()? Why `const` is dropped when passed to copy_to()? ~ Oleksii > +} > + > +unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned len) > +{ > + return copy_from((void *)to, (vaddr_t)from, len); > +} > diff --git a/xen/arch/riscv/include/asm/guest_access.h b/xen/arch/riscv/include/asm/guest_access.h > index 7cd51fbbde..4fcf3fbf68 100644 > --- a/xen/arch/riscv/include/asm/guest_access.h > +++ b/xen/arch/riscv/include/asm/guest_access.h > @@ -2,6 +2,11 @@ > #ifndef ASM__RISCV__GUEST_ACCESS_H > #define ASM__RISCV__GUEST_ACCESS_H > > +#include <xen/types.h> > + > +extern size_t _copy_to(uint64_t dest, const void *src, size_t len); > +extern size_t _copy_from(void *dest, uint64_t src, size_t len); > + > unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len); > unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len); > unsigned long raw_clear_guest(void *to, unsigned int len);
Hello Andrew, On Fri, Feb 28, 2025 at 4:47 PM Andrew Cooper <andrew.cooper3@citrix.com> wrote: > > On 28/02/2025 2:59 pm, Milan Djokic wrote: > > From: Slavisa Petrovic <Slavisa.Petrovic@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.Djokic@rt-rk.com> > > Signed-off-by: Slavisa Petrovic <Slavisa.Petrovic@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
Hello Oleksii, On Fri, Feb 28, 2025 at 5:03 PM Oleksii Kurochko <oleksii.kurochko@gmail.com> wrote: > > > On 2/28/25 3:59 PM, Milan Djokic wrote: > > From: Slavisa Petrovic <Slavisa.Petrovic@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.Djokic@rt-rk.com> > Signed-off-by: Slavisa Petrovic <Slavisa.Petrovic@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. > --- > xen/arch/riscv/Makefile | 1 + > xen/arch/riscv/addr_translation.S | 63 +++++++++++++++++++++++ > xen/arch/riscv/arch.mk | 6 ++- > xen/arch/riscv/guestcopy.c | 43 ++++++++++++++++ > xen/arch/riscv/include/asm/guest_access.h | 5 ++ > 5 files changed, 117 insertions(+), 1 deletion(-) > create mode 100644 xen/arch/riscv/addr_translation.S > create mode 100644 xen/arch/riscv/guestcopy.c > > diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile > index a5eb2aed4b..1bd61cc993 100644 > --- a/xen/arch/riscv/Makefile > +++ b/xen/arch/riscv/Makefile > @@ -10,6 +10,7 @@ obj-y += smp.o > obj-y += stubs.o > obj-y += traps.o > obj-y += vm_event.o > +obj-y += addr_translation.o > > It should be sorted in alphabetical order. > Will be updated in next version > > $(TARGET): $(TARGET)-syms > $(OBJCOPY) -O binary -S $< $@ > 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: > + la t0, copy_done /* Load address of return label */ > + mv t2, zero /* Initialize counter to zero */ > +loop_check: > + beq t2, a2, copy_done /* Check if all bytes are copied */ > + lb t3, (a1) /* Load byte from source (guest address) */ > +store_byte: > + hsv.b t3, (a0) /* Store byte to destination (host address) */ > + add_extable store_byte /* Add exception table for this location */ > + addi a0, a0, 1 /* Increment destination pointer */ > + addi a1, a1, 1 /* Increment source pointer */ > + addi t2, t2, 1 /* Increment byte counter */ > + j loop_check /* Loop back if not done */ > + > +/* > + * size_t _copy_from(void *dest, uint64_t src, size_t len) > + * > + * a0 - dest > + * a1 - src > + * a2 - len > + * > + */ > + > +.global _copy_from > +_copy_from: > + la t0, copy_done > + mv t2, zero > +loop_check_from: > + beq t2, a2, copy_done > +load_byte: > + hlv.b t3, (a1) /* Load byte from guest address */ > + add_extable load_byte > + sb t3, (a0) > + addi a0, a0, 1 > + addi a1, a1, 1 > + addi t2, t2, 1 > + j loop_check_from > + > +copy_done: > + mv a0, t2 /* Return number of bytes copied */ > + ret > > Can't we done this functions fully in C? (it doesn't something mandatory) > Yes, we have changed this part to be completely handled in C > diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk > index 17827c302c..f4098f5b5e 100644 > --- a/xen/arch/riscv/arch.mk > +++ b/xen/arch/riscv/arch.mk > @@ -23,13 +23,17 @@ $(eval $(1) := \ > $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value $(1)-insn),_$(1))) > endef > > +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h) > +$(h-extension-name)-insn := "hfence.gvma" > +$(call check-extension,$(h-extension-name)) > + > zbb-insn := "andn t0$(comma)t0$(comma)t0" > $(call check-extension,zbb) > > zihintpause-insn := "pause" > $(call check-extension,zihintpause) > > -extensions := $(zbb) $(zihintpause) > +extensions := $(value $(h-extension-name)) $(zbb) $(zihintpause) > > extensions := $(subst $(space),,$(extensions)) > > This patch should take into account another one patch series ( https://lore.kernel.org/xen-devel/cover.1740071755.git.oleksii.kurochko@gmail.com/T/#t) > update for which I am going to sent today. > > Also, these changes would be better to move into separate commit with explanation why what is so specific with 1301 and why it is needed to introduce > 'hh'. > I believe that these changes were taken based on my patch: https://gitlab.com/xen-project/people/olkur/xen/-/commit/154f75e943f1668dbf2c7be0f0fdff5b30132e89 > Probably it will make sense just to get it and rebase on top of mentioned above patch series. > Yes, it is based on your patch. Sorry, I was not aware that you already have an active patch series which contains this part. In that case we'll wait for your patch series to be merged first. And we'll split it into 2 commits where first one will only introduce h extension handling in arch.mk and the second one with copy_from/to_guest functionallity > > diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c > new file mode 100644 > index 0000000000..0325956845 > --- /dev/null > +++ b/xen/arch/riscv/guestcopy.c > @@ -0,0 +1,43 @@ > +#include <xen/bug.h> > +#include <xen/domain_page.h> > +#include <xen/errno.h> > +#include <xen/sched.h> > + > +#include <asm/csr.h> > +#include <asm/guest_access.h> > +#include <asm/system.h> > +#include <asm/traps.h> > + > +unsigned long copy_to(uint64_t dest, void* src, size_t len) > +{ > + size_t bytes; > + > + bytes = _copy_to(dest, src, len); > + > + if (bytes == len) > + return 0; > + else > + return -ENOSYS; > +} > > Why do we have a different prototypes with copy_from() below? If we will have > void *dest then ... > > + > +unsigned long copy_from(void *dest, uint64_t src, size_t len) > +{ > + size_t bytes; > + > + bytes = _copy_from(dest, src, len); > + > + if (bytes == len) > + return 0; > + else > + return -ENOSYS; > +} > + > +unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) > +{ > + return copy_to((vaddr_t)to, (void *)from, len); > > ... we won't need cast for `to` and wo we really need cast for copy_to()? Why `const` is > dropped when passed to copy_to()? > These are just mismatches (prototype and const drop), fixed for the next version. BR, Milan
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@citrix.com> wrote: >> On 28/02/2025 2:59 pm, Milan Djokic wrote: >>> From: Slavisa Petrovic <Slavisa.Petrovic@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.Djokic@rt-rk.com> >>> Signed-off-by: Slavisa Petrovic <Slavisa.Petrovic@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 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
On 3/4/25 1:09 PM, Milan Đokić wrote: >> diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk >> index 17827c302c..f4098f5b5e 100644 >> --- a/xen/arch/riscv/arch.mk >> +++ b/xen/arch/riscv/arch.mk >> @@ -23,13 +23,17 @@ $(eval $(1) := \ >> $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value $(1)-insn),_$(1))) >> endef >> >> +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h) >> +$(h-extension-name)-insn := "hfence.gvma" >> +$(call check-extension,$(h-extension-name)) >> + >> zbb-insn := "andn t0$(comma)t0$(comma)t0" >> $(call check-extension,zbb) >> >> zihintpause-insn := "pause" >> $(call check-extension,zihintpause) >> >> -extensions := $(zbb) $(zihintpause) >> +extensions := $(value $(h-extension-name)) $(zbb) $(zihintpause) >> >> extensions := $(subst $(space),,$(extensions)) >> >> This patch should take into account another one patch series (https://lore.kernel.org/xen-devel/cover.1740071755.git.oleksii.kurochko@gmail.com/T/#t) >> update for which I am going to sent today. >> >> Also, these changes would be better to move into separate commit with explanation why what is so specific with 1301 and why it is needed to introduce >> 'hh'. >> I believe that these changes were taken based on my patch:https://gitlab.com/xen-project/people/olkur/xen/-/commit/154f75e943f1668dbf2c7be0f0fdff5b30132e89 >> Probably it will make sense just to get it and rebase on top of mentioned above patch series. >> > Yes, it is based on your patch. Sorry, I was not aware that you > already have an active patch series which contains this part. > In that case we'll wait for your patch series to be merged first. And > we'll split it into 2 commits where first one will only introduce h > extension handling in arch.mk and the second one with > copy_from/to_guest functionallity This is not really contains this part directly but that another one patch series will affect these changes. So the final changes should look like: https://gitlab.com/xen-project/people/olkur/xen/-/commit/eb75bc3482ffe025cf36c320e2e13a77deeea883 I planned to send this patch to upstream on Monday. Best regards, Oleksii
diff --git a/xen/arch/riscv/Makefile b/xen/arch/riscv/Makefile index a5eb2aed4b..1bd61cc993 100644 --- a/xen/arch/riscv/Makefile +++ b/xen/arch/riscv/Makefile @@ -10,6 +10,7 @@ obj-y += smp.o obj-y += stubs.o obj-y += traps.o obj-y += vm_event.o +obj-y += addr_translation.o $(TARGET): $(TARGET)-syms $(OBJCOPY) -O binary -S $< $@ 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: + la t0, copy_done /* Load address of return label */ + mv t2, zero /* Initialize counter to zero */ +loop_check: + beq t2, a2, copy_done /* Check if all bytes are copied */ + lb t3, (a1) /* Load byte from source (guest address) */ +store_byte: + hsv.b t3, (a0) /* Store byte to destination (host address) */ + add_extable store_byte /* Add exception table for this location */ + addi a0, a0, 1 /* Increment destination pointer */ + addi a1, a1, 1 /* Increment source pointer */ + addi t2, t2, 1 /* Increment byte counter */ + j loop_check /* Loop back if not done */ + +/* + * size_t _copy_from(void *dest, uint64_t src, size_t len) + * + * a0 - dest + * a1 - src + * a2 - len + * + */ + +.global _copy_from +_copy_from: + la t0, copy_done + mv t2, zero +loop_check_from: + beq t2, a2, copy_done +load_byte: + hlv.b t3, (a1) /* Load byte from guest address */ + add_extable load_byte + sb t3, (a0) + addi a0, a0, 1 + addi a1, a1, 1 + addi t2, t2, 1 + j loop_check_from + +copy_done: + mv a0, t2 /* Return number of bytes copied */ + ret diff --git a/xen/arch/riscv/arch.mk b/xen/arch/riscv/arch.mk index 17827c302c..f4098f5b5e 100644 --- a/xen/arch/riscv/arch.mk +++ b/xen/arch/riscv/arch.mk @@ -23,13 +23,17 @@ $(eval $(1) := \ $(call as-insn,$(CC) $(riscv-generic-flags)_$(1),$(value $(1)-insn),_$(1))) endef +h-extension-name := $(call cc-ifversion,-lt,1301, hh, h) +$(h-extension-name)-insn := "hfence.gvma" +$(call check-extension,$(h-extension-name)) + zbb-insn := "andn t0$(comma)t0$(comma)t0" $(call check-extension,zbb) zihintpause-insn := "pause" $(call check-extension,zihintpause) -extensions := $(zbb) $(zihintpause) +extensions := $(value $(h-extension-name)) $(zbb) $(zihintpause) extensions := $(subst $(space),,$(extensions)) diff --git a/xen/arch/riscv/guestcopy.c b/xen/arch/riscv/guestcopy.c new file mode 100644 index 0000000000..0325956845 --- /dev/null +++ b/xen/arch/riscv/guestcopy.c @@ -0,0 +1,43 @@ +#include <xen/bug.h> +#include <xen/domain_page.h> +#include <xen/errno.h> +#include <xen/sched.h> + +#include <asm/csr.h> +#include <asm/guest_access.h> +#include <asm/system.h> +#include <asm/traps.h> + +unsigned long copy_to(uint64_t dest, void* src, size_t len) +{ + size_t bytes; + + bytes = _copy_to(dest, src, len); + + if (bytes == len) + return 0; + else + return -ENOSYS; +} + +unsigned long copy_from(void *dest, uint64_t src, size_t len) +{ + size_t bytes; + + bytes = _copy_from(dest, src, len); + + if (bytes == len) + return 0; + else + return -ENOSYS; +} + +unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len) +{ + return copy_to((vaddr_t)to, (void *)from, len); +} + +unsigned long raw_copy_from_guest(void *to, const void __user *from, unsigned len) +{ + return copy_from((void *)to, (vaddr_t)from, len); +} diff --git a/xen/arch/riscv/include/asm/guest_access.h b/xen/arch/riscv/include/asm/guest_access.h index 7cd51fbbde..4fcf3fbf68 100644 --- a/xen/arch/riscv/include/asm/guest_access.h +++ b/xen/arch/riscv/include/asm/guest_access.h @@ -2,6 +2,11 @@ #ifndef ASM__RISCV__GUEST_ACCESS_H #define ASM__RISCV__GUEST_ACCESS_H +#include <xen/types.h> + +extern size_t _copy_to(uint64_t dest, const void *src, size_t len); +extern size_t _copy_from(void *dest, uint64_t src, size_t len); + unsigned long raw_copy_to_guest(void *to, const void *from, unsigned len); unsigned long raw_copy_from_guest(void *to, const void *from, unsigned len); unsigned long raw_clear_guest(void *to, unsigned int len);