Re: [PATCH 3/4] KVM: riscv: selftests: Fix ISA_EXT register handling in get-reg-list

On Mon, Sep 18, 2023 at 11:36:45PM +0530, Anup Patel wrote:

Same set of ISA_EXT registers are not present on all host because ISA_EXT registers are visible to the KVM user space based on the ISA extensions available on the host. Also, disabling an ISA extension using corresponding ISA_EXT register does not affect the visibility of the ISA_EXT register itself.

Based on the above, we should filter-out all ISA_EXT registers.

Fixes: 477069398ed6 ("KVM: riscv: selftests: Add get-reg-list test") Signed-off-by: Anup Patel apatel@ventanamicro.com

---

.../selftests/kvm/riscv/get-reg-list.c | 35 +++++++++++-------- 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index d8ecacd03ecf..76c0ad11e423 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -14,17 +14,33 @@ bool filter_reg(__u64 reg) {

• switch (reg & ~REG_MASK) { /*

• * Some ISA extensions are optional and not present on all host,
  

• * but they can't be disabled through ISA_EXT registers when present.
  

• * So, to make life easy, just filtering out these kind of registers.
  

• * Same set of ISA_EXT registers are not present on all host because
  

• * ISA_EXT registers are visible to the KVM user space based on the
  

• * ISA extensions available on the host. Also, disabling an ISA
  

• * extension using corresponding ISA_EXT register does not affect
  

• * the visibility of the ISA_EXT register itself.
  

• *
  

• * Based on above, we should filter-out all ISA_EXT registers.
  

  */

• switch (reg & ~REG_MASK) {

• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_H:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVPBMT: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSTC: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVINVAL: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOZ: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBB: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSAIA:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_V:
• case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVNAPOT: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBA: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBS: case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICNTR:

@@ -50,12 +66,7 @@ static inline bool vcpu_has_ext(struct kvm_vcpu *vcpu, int ext) unsigned long value; ret = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(ext), &value);

• if (ret) {

• printf("Failed to get ext %d", ext);
  

• return false;
  

• }
• return !!value;

• return (ret) ? false : !!value;

This is an unrelated change, but OK. It's consistent with the plan[1] we have on the timer test series

[1] https://lore.kernel.org/all/20230914-d6645bbc5ac80999674e9685@orel/

} void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c) @@ -506,10 +517,6 @@ static __u64 base_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(time), KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(compare), KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),

• KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A,
• KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C,
• KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I,
• KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M, KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01, KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME, KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI,

-- 2.34.1

Reviewed-by: Andrew Jones ajones@ventanamicro.com

Thanks, drew