| Message ID | cover.1690364259.git.haibo1.xu@intel.com |
|---|---|
| Headers | show |
| Series | RISCV: Add kvm Sstc timer selftest | expand |
On Thu, Jul 27, 2023 at 11:14 PM Sean Christopherson <seanjc@google.com> wrote: > > On Thu, Jul 27, 2023, Haibo Xu wrote: > > The sstc_timer selftest is used to validate Sstc timer functionality > > in a guest, which sets up periodic timer interrupts and check the > > basic interrupt status upon its receipt. > > > > This KVM selftest was ported from aarch64 arch_timer and tested > > with Linux v6.5-rc3 on a Qemu riscv64 virt machine. > > Would it be possible to extract the ARM bits from arch_timer and make the bulk of > the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste. Sure, I will have a try to consolidate the common code for ARM and RISC-V in v2. Thanks, Haibo
On Thu, Jul 27, 2023 at 03:20:05PM +0800, Haibo Xu wrote: > Borrow some of the csr definitions and operations from kernel's > arch/riscv/include/asm/csr.h to tools/ for riscv. You should copy the entire file verbatim. Thanks, drew
On Fri, Jul 28, 2023 at 09:37:36AM +0800, Haibo Xu wrote: > On Thu, Jul 27, 2023 at 11:14 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Thu, Jul 27, 2023, Haibo Xu wrote: > > > The sstc_timer selftest is used to validate Sstc timer functionality > > > in a guest, which sets up periodic timer interrupts and check the > > > basic interrupt status upon its receipt. > > > > > > This KVM selftest was ported from aarch64 arch_timer and tested > > > with Linux v6.5-rc3 on a Qemu riscv64 virt machine. > > > > Would it be possible to extract the ARM bits from arch_timer and make the bulk of > > the test common to ARM and RISC-V? At a glance, there is quite a bit of copy+paste. > > Sure, I will have a try to consolidate the common code for ARM and RISC-V in v2. > Yes, afaict, we should be able to make aarch64/arch_timer.c another "split test", like we did for aarch64/get-reg-list.c, but before we do that, I'd like to get an ack from the Arm maintainers on the get-reg-list split to be sure that approach is acceptable. Thanks, drew
On Fri, Jul 28, 2023 at 5:49 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > On Thu, Jul 27, 2023 at 03:20:07PM +0800, Haibo Xu wrote: > > Add guest_get_vcpuid() helper to simplify accessing to per-cpu > > private data. The sscratch CSR was used to store the vcpu id. > > > > Signed-off-by: Haibo Xu <haibo1.xu@intel.com> > > --- > > tools/testing/selftests/kvm/include/riscv/processor.h | 2 ++ > > tools/testing/selftests/kvm/lib/riscv/processor.c | 8 ++++++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h > > index 9ea6e7bedc61..ca53570ce6de 100644 > > --- a/tools/testing/selftests/kvm/include/riscv/processor.h > > +++ b/tools/testing/selftests/kvm/include/riscv/processor.h > > @@ -165,4 +165,6 @@ struct sbiret sbi_ecall(int ext, int fid, unsigned long arg0, > > unsigned long arg3, unsigned long arg4, > > unsigned long arg5); > > > > +uint32_t guest_get_vcpuid(void); > > I'd also put this prototype somewhere common. > > > + > > #endif /* SELFTEST_KVM_PROCESSOR_H */ > > diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c > > index f1b0be58a5dc..b8ad3e69a697 100644 > > --- a/tools/testing/selftests/kvm/lib/riscv/processor.c > > +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c > > @@ -316,6 +316,9 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id, > > vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.sp), stack_vaddr + stack_size); > > vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code); > > > > + /* Setup scratch regiter of guest */ > > typo: register > > The comment above is pretty useless since it just states what the code > states, but with even less information, since it doesn't state how the > sscratch register is getting set up. I'd either drop it or write it > as > > /* Setup sscratch for guest_get_vcpuid() */ > > > + vcpu_set_reg(vcpu, RISCV_CSR_REG(sscratch), vcpu_id); > > + > > /* Setup default exception vector of guest */ > > vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap); > > > > @@ -424,3 +427,8 @@ void vm_install_interrupt_handler(struct kvm_vm *vm, void (*handler)(struct ex_r > > > > handlers->exception_handlers[1][0] = handler; > > } > > + > > +uint32_t guest_get_vcpuid(void) > > +{ > > + return csr_read(CSR_SSCRATCH); > > +} > > -- > > 2.34.1 > > > Sure! will fix them in v2. Thanks, Haibo > Thanks, > drew
On Wed, Aug 02, 2023 at 11:13:34PM -0400, Guo Ren wrote: > On Wed, Aug 02, 2023 at 10:05:00AM +0800, Haibo Xu wrote: > > On Fri, Jul 28, 2023 at 5:43 PM Andrew Jones <ajones@ventanamicro.com> wrote: > > > > > > On Thu, Jul 27, 2023 at 03:20:05PM +0800, Haibo Xu wrote: > > > > Borrow some of the csr definitions and operations from kernel's > > > > arch/riscv/include/asm/csr.h to tools/ for riscv. > > > > > > You should copy the entire file verbatim. > > > > > > > Ok, will copy all the definitions in the original csr.h > Why not include the original one? Maintain the one csr.h is more > comfortable. selftests and other userspace tools can't always compile when including a kernel header without modifying the header in some way. Rather than polluting headers with #ifdeffery, the practice has been to copy necessary headers to tools/include and modify if necessary. Thanks, drew
On Thu, Aug 3, 2023 at 6:16 AM Sean Christopherson <seanjc@google.com> wrote: > > On Thu, Jul 27, 2023, Haibo Xu wrote: > > The sstc_timer selftest is used to validate Sstc timer functionality > > in a guest, which sets up periodic timer interrupts and check the > > basic interrupt status upon its receipt. > > > > This KVM selftest was ported from aarch64 arch_timer and tested > > with Linux v6.5-rc3 on a Qemu riscv64 virt machine. > > > > Haibo Xu (4): > > tools: riscv: Add header file csr.h > > KVM: riscv: selftests: Add exception handling support > > KVM: riscv: selftests: Add guest helper to get vcpu id > > KVM: riscv: selftests: Add sstc_timer test > > FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will > reworking the existing arch_timer test. My plan is to create an immutable tag > later this week (waiting to make sure nothing explodes). I highly recommend basing > v2 on top of that. > Hi Sean, Could you help point me to the immutable tag for the guest printf changes? Regards, Haibo > [*] https://lore.kernel.org/all/20230729003643.1053367-1-seanjc@google.com
On Sun, Aug 27, 2023, Haibo Xu wrote: > On Thu, Aug 3, 2023 at 6:16 AM Sean Christopherson <seanjc@google.com> wrote: > > > > On Thu, Jul 27, 2023, Haibo Xu wrote: > > > The sstc_timer selftest is used to validate Sstc timer functionality > > > in a guest, which sets up periodic timer interrupts and check the > > > basic interrupt status upon its receipt. > > > > > > This KVM selftest was ported from aarch64 arch_timer and tested > > > with Linux v6.5-rc3 on a Qemu riscv64 virt machine. > > > > > > Haibo Xu (4): > > > tools: riscv: Add header file csr.h > > > KVM: riscv: selftests: Add exception handling support > > > KVM: riscv: selftests: Add guest helper to get vcpu id > > > KVM: riscv: selftests: Add sstc_timer test > > > > FYI, patch 4 will conflict with the in-flight guest printf changes[*], as will > > reworking the existing arch_timer test. My plan is to create an immutable tag > > later this week (waiting to make sure nothing explodes). I highly recommend basing > > v2 on top of that. > > > > Hi Sean, > > Could you help point me to the immutable tag for the guest printf changes? Sorry, I forgot to create the tag until last week, probably made it a bit hard to find... https://github.com/kvm-x86/linux.git tags/kvm-x86-selftests-immutable-6.6