This series adds ONE_REG interface for SBI FWFT extension implemented by KVM RISC-V. This was missed out in accepted SBI FWFT patches for KVM RISC-V.
These patches can also be found in the riscv_kvm_fwft_one_reg_v1 branch at: https://github.com/avpatel/linux.git
Anup Patel (6): RISC-V: KVM: Set initial value of hedeleg in kvm_arch_vcpu_create() RISC-V: KVM: Introduce feature specific reset for SBI FWFT RISC-V: KVM: Introduce optional ONE_REG callbacks for SBI extensions RISC-V: KVM: Move copy_sbi_ext_reg_indices() to SBI implementation RISC-V: KVM: Implement ONE_REG interface for SBI FWFT state KVM: riscv: selftests: Add SBI FWFT to get-reg-list test
arch/riscv/include/asm/kvm_vcpu_sbi.h | 23 +- arch/riscv/include/uapi/asm/kvm.h | 14 ++ arch/riscv/kvm/vcpu.c | 3 +- arch/riscv/kvm/vcpu_onereg.c | 60 +----- arch/riscv/kvm/vcpu_sbi.c | 172 ++++++++++++--- arch/riscv/kvm/vcpu_sbi_fwft.c | 199 ++++++++++++++++-- arch/riscv/kvm/vcpu_sbi_sta.c | 64 ++++-- .../selftests/kvm/riscv/get-reg-list.c | 28 +++ 8 files changed, 436 insertions(+), 127 deletions(-)
The hedeleg may be updated by ONE_REG interface before the VCPU is run at least once hence set the initial value of hedeleg in kvm_arch_vcpu_create() instead of kvm_riscv_vcpu_setup_config().
Signed-off-by: Anup Patel apatel@ventanamicro.com --- arch/riscv/kvm/vcpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c index f001e56403f9..86025f68c374 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -133,6 +133,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
/* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false; + + vcpu->arch.cfg.hedeleg = KVM_HEDELEG_DEFAULT; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO; bitmap_zero(vcpu->arch.isa, RISCV_ISA_EXT_MAX);
@@ -570,7 +572,6 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu) cfg->hstateen0 |= SMSTATEEN0_SSTATEEN0; }
- cfg->hedeleg = KVM_HEDELEG_DEFAULT; if (vcpu->guest_debug) cfg->hedeleg &= ~BIT(EXC_BREAKPOINT); }
On Thu, Aug 14, 2025 at 09:25:43PM +0530, Anup Patel wrote:
The hedeleg may be updated by ONE_REG interface before the VCPU is run at least once hence set the initial value of hedeleg in kvm_arch_vcpu_create() instead of kvm_riscv_vcpu_setup_config().
Signed-off-by: Anup Patel apatel@ventanamicro.com
arch/riscv/kvm/vcpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c index f001e56403f9..86025f68c374 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -133,6 +133,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) /* Mark this VCPU never ran */ vcpu->arch.ran_atleast_once = false;
- vcpu->arch.cfg.hedeleg = KVM_HEDELEG_DEFAULT; vcpu->arch.mmu_page_cache.gfp_zero = __GFP_ZERO; bitmap_zero(vcpu->arch.isa, RISCV_ISA_EXT_MAX);
@@ -570,7 +572,6 @@ static void kvm_riscv_vcpu_setup_config(struct kvm_vcpu *vcpu) cfg->hstateen0 |= SMSTATEEN0_SSTATEEN0; }
- cfg->hedeleg = KVM_HEDELEG_DEFAULT; if (vcpu->guest_debug) cfg->hedeleg &= ~BIT(EXC_BREAKPOINT);
}
2.43.0
Reviewed-by: Andrew Jones ajones@ventanamicro.com
The SBI FWFT feature values must be reset upon VCPU reset so introduce feature specific reset callback for this purpose.
Signed-off-by: Anup Patel apatel@ventanamicro.com --- arch/riscv/kvm/vcpu_sbi_fwft.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c index 164a01288b0a..5a3bad0f9330 100644 --- a/arch/riscv/kvm/vcpu_sbi_fwft.c +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -30,6 +30,13 @@ struct kvm_sbi_fwft_feature { */ bool (*supported)(struct kvm_vcpu *vcpu);
+ /** + * @reset: Reset the feature value irrespective whether feature is supported or not + * + * This callback is mandatory + */ + void (*reset)(struct kvm_vcpu *vcpu); + /** * @set: Set the feature value * @@ -75,6 +82,13 @@ static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu) return misaligned_traps_can_delegate(); }
+static void kvm_sbi_fwft_reset_misaligned_delegation(struct kvm_vcpu *vcpu) +{ + struct kvm_vcpu_config *cfg = &vcpu->arch.cfg; + + cfg->hedeleg &= ~MIS_DELEG; +} + static long kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value) @@ -124,6 +138,11 @@ static bool kvm_sbi_fwft_pointer_masking_pmlen_supported(struct kvm_vcpu *vcpu) return fwft->have_vs_pmlen_7 || fwft->have_vs_pmlen_16; }
+static void kvm_sbi_fwft_reset_pointer_masking_pmlen(struct kvm_vcpu *vcpu) +{ + vcpu->arch.cfg.henvcfg &= ~ENVCFG_PMM; +} + static long kvm_sbi_fwft_set_pointer_masking_pmlen(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value) @@ -180,6 +199,7 @@ static const struct kvm_sbi_fwft_feature features[] = { { .id = SBI_FWFT_MISALIGNED_EXC_DELEG, .supported = kvm_sbi_fwft_misaligned_delegation_supported, + .reset = kvm_sbi_fwft_reset_misaligned_delegation, .set = kvm_sbi_fwft_set_misaligned_delegation, .get = kvm_sbi_fwft_get_misaligned_delegation, }, @@ -187,6 +207,7 @@ static const struct kvm_sbi_fwft_feature features[] = { { .id = SBI_FWFT_POINTER_MASKING_PMLEN, .supported = kvm_sbi_fwft_pointer_masking_pmlen_supported, + .reset = kvm_sbi_fwft_reset_pointer_masking_pmlen, .set = kvm_sbi_fwft_set_pointer_masking_pmlen, .get = kvm_sbi_fwft_get_pointer_masking_pmlen, }, @@ -321,11 +342,16 @@ static void kvm_sbi_ext_fwft_deinit(struct kvm_vcpu *vcpu)
static void kvm_sbi_ext_fwft_reset(struct kvm_vcpu *vcpu) { - int i; struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu); + const struct kvm_sbi_fwft_feature *feature; + int i;
- for (i = 0; i < ARRAY_SIZE(features); i++) + for (i = 0; i < ARRAY_SIZE(features); i++) { fwft->configs[i].flags = 0; + feature = &features[i]; + if (feature->reset) + feature->reset(vcpu); + } }
const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = {
On Thu, Aug 14, 2025 at 09:25:44PM +0530, Anup Patel wrote:
The SBI FWFT feature values must be reset upon VCPU reset so introduce feature specific reset callback for this purpose.
Signed-off-by: Anup Patel apatel@ventanamicro.com
arch/riscv/kvm/vcpu_sbi_fwft.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c index 164a01288b0a..5a3bad0f9330 100644 --- a/arch/riscv/kvm/vcpu_sbi_fwft.c +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -30,6 +30,13 @@ struct kvm_sbi_fwft_feature { */ bool (*supported)(struct kvm_vcpu *vcpu);
- /**
* @reset: Reset the feature value irrespective whether feature is supported or not
*
* This callback is mandatory
*/
- void (*reset)(struct kvm_vcpu *vcpu);
- /**
- @set: Set the feature value
@@ -75,6 +82,13 @@ static bool kvm_sbi_fwft_misaligned_delegation_supported(struct kvm_vcpu *vcpu) return misaligned_traps_can_delegate(); } +static void kvm_sbi_fwft_reset_misaligned_delegation(struct kvm_vcpu *vcpu) +{
- struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
- cfg->hedeleg &= ~MIS_DELEG;
+}
static long kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value) @@ -124,6 +138,11 @@ static bool kvm_sbi_fwft_pointer_masking_pmlen_supported(struct kvm_vcpu *vcpu) return fwft->have_vs_pmlen_7 || fwft->have_vs_pmlen_16; } +static void kvm_sbi_fwft_reset_pointer_masking_pmlen(struct kvm_vcpu *vcpu) +{
- vcpu->arch.cfg.henvcfg &= ~ENVCFG_PMM;
+}
static long kvm_sbi_fwft_set_pointer_masking_pmlen(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value) @@ -180,6 +199,7 @@ static const struct kvm_sbi_fwft_feature features[] = { { .id = SBI_FWFT_MISALIGNED_EXC_DELEG, .supported = kvm_sbi_fwft_misaligned_delegation_supported,
.set = kvm_sbi_fwft_set_misaligned_delegation, .get = kvm_sbi_fwft_get_misaligned_delegation, },.reset = kvm_sbi_fwft_reset_misaligned_delegation,
@@ -187,6 +207,7 @@ static const struct kvm_sbi_fwft_feature features[] = { { .id = SBI_FWFT_POINTER_MASKING_PMLEN, .supported = kvm_sbi_fwft_pointer_masking_pmlen_supported,
.set = kvm_sbi_fwft_set_pointer_masking_pmlen, .get = kvm_sbi_fwft_get_pointer_masking_pmlen, },.reset = kvm_sbi_fwft_reset_pointer_masking_pmlen,
@@ -321,11 +342,16 @@ static void kvm_sbi_ext_fwft_deinit(struct kvm_vcpu *vcpu) static void kvm_sbi_ext_fwft_reset(struct kvm_vcpu *vcpu) {
- int i; struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu);
- const struct kvm_sbi_fwft_feature *feature;
- int i;
- for (i = 0; i < ARRAY_SIZE(features); i++)
- for (i = 0; i < ARRAY_SIZE(features); i++) { fwft->configs[i].flags = 0;
feature = &features[i];
if (feature->reset)
feature->reset(vcpu);
- }
} const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = { -- 2.43.0
Reviewed-by: Andrew Jones ajones@ventanamicro.com
SBI extensions can have per-VCPU state which needs to be saved/restored through ONE_REG interface for Guest/VM migration. Introduce optional ONE_REG callbacks for SBI extensions so that ONE_REG implementation for an SBI extenion is part of the extension sources.
Signed-off-by: Anup Patel apatel@ventanamicro.com --- arch/riscv/include/asm/kvm_vcpu_sbi.h | 21 ++-- arch/riscv/kvm/vcpu_onereg.c | 31 +----- arch/riscv/kvm/vcpu_sbi.c | 145 ++++++++++++++++++++++---- arch/riscv/kvm/vcpu_sbi_sta.c | 64 ++++++++---- 4 files changed, 178 insertions(+), 83 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 766031e80960..144c3f6d5eb9 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -59,6 +59,15 @@ struct kvm_vcpu_sbi_extension { void (*deinit)(struct kvm_vcpu *vcpu);
void (*reset)(struct kvm_vcpu *vcpu); + + bool have_state; + unsigned long state_reg_subtype; + unsigned long (*get_state_reg_count)(struct kvm_vcpu *vcpu); + int (*get_state_reg_id)(struct kvm_vcpu *vcpu, int index, u64 *reg_id); + int (*get_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, void *reg_val); + int (*set_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, const void *reg_val); };
void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -73,10 +82,9 @@ int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); -int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, - const struct kvm_one_reg *reg); -int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, - const struct kvm_one_reg *reg); +int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices); +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( struct kvm_vcpu *vcpu, unsigned long extid); bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx); @@ -85,11 +93,6 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_reset(struct kvm_vcpu *vcpu);
-int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num, - unsigned long *reg_val); -int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num, - unsigned long reg_val); - #ifdef CONFIG_RISCV_SBI_V01 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01; #endif diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index b77748a56a59..5843b0519224 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -1090,36 +1090,9 @@ static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu) return copy_sbi_ext_reg_indices(vcpu, NULL); }
-static int copy_sbi_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) -{ - struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context; - int total = 0; - - if (scontext->ext_status[KVM_RISCV_SBI_EXT_STA] == KVM_RISCV_SBI_EXT_STATUS_ENABLED) { - u64 size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64; - int n = sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long); - - for (int i = 0; i < n; i++) { - u64 reg = KVM_REG_RISCV | size | - KVM_REG_RISCV_SBI_STATE | - KVM_REG_RISCV_SBI_STA | i; - - if (uindices) { - if (put_user(reg, uindices)) - return -EFAULT; - uindices++; - } - } - - total += n; - } - - return total; -} - static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu) { - return copy_sbi_reg_indices(vcpu, NULL); + return kvm_riscv_vcpu_reg_indices_sbi(vcpu, NULL); }
static inline unsigned long num_vector_regs(const struct kvm_vcpu *vcpu) @@ -1247,7 +1220,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu, return ret; uindices += ret;
- ret = copy_sbi_reg_indices(vcpu, uindices); + ret = kvm_riscv_vcpu_reg_indices_sbi(vcpu, uindices); if (ret < 0) return ret; uindices += ret; diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 01a93f4fdb16..8b3c393e0c83 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -364,64 +364,163 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, return 0; }
-int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, - const struct kvm_one_reg *reg) +int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices) +{ + struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context; + const struct kvm_riscv_sbi_extension_entry *entry; + const struct kvm_vcpu_sbi_extension *ext; + unsigned long state_reg_count; + int i, j, rc, count = 0; + u64 reg; + + for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { + entry = &sbi_ext[i]; + ext = entry->ext_ptr; + + if (!ext->have_state || + scontext->ext_status[entry->ext_idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED) + continue; + + state_reg_count = ext->get_state_reg_count(vcpu); + if (!uindices) + goto skip_put_user; + + for (j = 0; j < state_reg_count; j++) { + if (ext->get_state_reg_id) { + rc = ext->get_state_reg_id(vcpu, j, ®); + if (rc) + return rc; + } else { + reg = KVM_REG_RISCV | + (IS_ENABLED(CONFIG_32BIT) ? + KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64) | + KVM_REG_RISCV_SBI_STATE | + ext->state_reg_subtype | j; + } + + if (put_user(reg, uindices)) + return -EFAULT; + uindices++; + } + +skip_put_user: + count += state_reg_count; + } + + return count; +} + +static const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext_withstate(struct kvm_vcpu *vcpu, + unsigned long subtype) +{ + struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context; + const struct kvm_riscv_sbi_extension_entry *entry; + const struct kvm_vcpu_sbi_extension *ext; + int i; + + for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { + entry = &sbi_ext[i]; + ext = entry->ext_ptr; + + if (ext->have_state && + ext->state_reg_subtype == subtype && + scontext->ext_status[entry->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_ENABLED) + return ext; + } + + return NULL; +} + +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE); - unsigned long reg_subtype, reg_val; - - if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long)) + const struct kvm_vcpu_sbi_extension *ext; + unsigned long reg_subtype; + void *reg_val; + u64 data64; + u32 data32; + u16 data16; + u8 data8; + + switch (KVM_REG_SIZE(reg->id)) { + case 1: + reg_val = &data8; + break; + case 2: + reg_val = &data16; + break; + case 4: + reg_val = &data32; + break; + case 8: + reg_val = &data64; + break; + default: return -EINVAL; + };
- if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id))) + if (copy_from_user(reg_val, uaddr, KVM_REG_SIZE(reg->id))) return -EFAULT;
reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
- switch (reg_subtype) { - case KVM_REG_RISCV_SBI_STA: - return kvm_riscv_vcpu_set_reg_sbi_sta(vcpu, reg_num, reg_val); - default: + ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype); + if (!ext || !ext->set_state_reg) return -EINVAL; - }
- return 0; + return ext->set_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val); }
-int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, - const struct kvm_one_reg *reg) +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE); - unsigned long reg_subtype, reg_val; + const struct kvm_vcpu_sbi_extension *ext; + unsigned long reg_subtype; + void *reg_val; + u64 data64; + u32 data32; + u16 data16; + u8 data8; int ret;
- if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long)) + switch (KVM_REG_SIZE(reg->id)) { + case 1: + reg_val = &data8; + break; + case 2: + reg_val = &data16; + break; + case 4: + reg_val = &data32; + break; + case 8: + reg_val = &data64; + break; + default: return -EINVAL; + };
reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
- switch (reg_subtype) { - case KVM_REG_RISCV_SBI_STA: - ret = kvm_riscv_vcpu_get_reg_sbi_sta(vcpu, reg_num, ®_val); - break; - default: + ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype); + if (!ext || !ext->get_state_reg) return -EINVAL; - }
+ ret = ext->get_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val); if (ret) return ret;
- if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id))) + if (copy_to_user(uaddr, reg_val, KVM_REG_SIZE(reg->id))) return -EFAULT;
return 0; diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index cc6cb7c8f0e4..d14cf6255d83 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -151,63 +151,83 @@ static unsigned long kvm_sbi_ext_sta_probe(struct kvm_vcpu *vcpu) return !!sched_info_on(); }
-const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = { - .extid_start = SBI_EXT_STA, - .extid_end = SBI_EXT_STA, - .handler = kvm_sbi_ext_sta_handler, - .probe = kvm_sbi_ext_sta_probe, - .reset = kvm_riscv_vcpu_sbi_sta_reset, -}; +static unsigned long kvm_sbi_ext_sta_get_state_reg_count(struct kvm_vcpu *vcpu) +{ + return sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long); +}
-int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, - unsigned long reg_num, - unsigned long *reg_val) +static int kvm_sbi_ext_sta_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, void *reg_val) { + unsigned long *value; + + if (reg_size != sizeof(unsigned long)) + return -EINVAL; + value = reg_val; + switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo): - *reg_val = (unsigned long)vcpu->arch.sta.shmem; + *value = (unsigned long)vcpu->arch.sta.shmem; break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT)) - *reg_val = upper_32_bits(vcpu->arch.sta.shmem); + *value = upper_32_bits(vcpu->arch.sta.shmem); else - *reg_val = 0; + *value = 0; break; default: - return -EINVAL; + return -ENOENT; }
return 0; }
-int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu, - unsigned long reg_num, - unsigned long reg_val) +static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, const void *reg_val) { + unsigned long value; + + if (reg_size != sizeof(unsigned long)) + return -EINVAL; + value = *(const unsigned long *)reg_val; + switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t hi = upper_32_bits(vcpu->arch.sta.shmem);
- vcpu->arch.sta.shmem = reg_val; + vcpu->arch.sta.shmem = value; vcpu->arch.sta.shmem |= hi << 32; } else { - vcpu->arch.sta.shmem = reg_val; + vcpu->arch.sta.shmem = value; } break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t lo = lower_32_bits(vcpu->arch.sta.shmem);
- vcpu->arch.sta.shmem = ((gpa_t)reg_val << 32); + vcpu->arch.sta.shmem = ((gpa_t)value << 32); vcpu->arch.sta.shmem |= lo; - } else if (reg_val != 0) { + } else if (value != 0) { return -EINVAL; } break; default: - return -EINVAL; + return -ENOENT; }
return 0; } + +const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = { + .extid_start = SBI_EXT_STA, + .extid_end = SBI_EXT_STA, + .handler = kvm_sbi_ext_sta_handler, + .probe = kvm_sbi_ext_sta_probe, + .reset = kvm_riscv_vcpu_sbi_sta_reset, + .have_state = true, + .state_reg_subtype = KVM_REG_RISCV_SBI_STA, + .get_state_reg_count = kvm_sbi_ext_sta_get_state_reg_count, + .get_state_reg = kvm_sbi_ext_sta_get_reg, + .set_state_reg = kvm_sbi_ext_sta_set_reg, +};
On Thu, Aug 14, 2025 at 09:25:45PM +0530, Anup Patel wrote:
SBI extensions can have per-VCPU state which needs to be saved/restored through ONE_REG interface for Guest/VM migration. Introduce optional ONE_REG callbacks for SBI extensions so that ONE_REG implementation for an SBI extenion is part of the extension sources.
Signed-off-by: Anup Patel apatel@ventanamicro.com
arch/riscv/include/asm/kvm_vcpu_sbi.h | 21 ++-- arch/riscv/kvm/vcpu_onereg.c | 31 +----- arch/riscv/kvm/vcpu_sbi.c | 145 ++++++++++++++++++++++---- arch/riscv/kvm/vcpu_sbi_sta.c | 64 ++++++++---- 4 files changed, 178 insertions(+), 83 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 766031e80960..144c3f6d5eb9 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -59,6 +59,15 @@ struct kvm_vcpu_sbi_extension { void (*deinit)(struct kvm_vcpu *vcpu); void (*reset)(struct kvm_vcpu *vcpu);
- bool have_state;
- unsigned long state_reg_subtype;
- unsigned long (*get_state_reg_count)(struct kvm_vcpu *vcpu);
I think we can drop 'have_state'. When 'get_state_reg_count' is NULL, then the state reg count must be zero (i.e. have_state == false).
- int (*get_state_reg_id)(struct kvm_vcpu *vcpu, int index, u64 *reg_id);
- int (*get_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, void *reg_val);
- int (*set_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, const void *reg_val);
}; void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -73,10 +82,9 @@ int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); -int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg);
-int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg);
+int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices); +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( struct kvm_vcpu *vcpu, unsigned long extid); bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx); @@ -85,11 +93,6 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_reset(struct kvm_vcpu *vcpu); -int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long *reg_val);
-int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_val);
#ifdef CONFIG_RISCV_SBI_V01 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01; #endif diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index b77748a56a59..5843b0519224 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -1090,36 +1090,9 @@ static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu) return copy_sbi_ext_reg_indices(vcpu, NULL); } -static int copy_sbi_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) -{
- struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
- int total = 0;
- if (scontext->ext_status[KVM_RISCV_SBI_EXT_STA] == KVM_RISCV_SBI_EXT_STATUS_ENABLED) {
u64 size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
int n = sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long);
for (int i = 0; i < n; i++) {
u64 reg = KVM_REG_RISCV | size |
KVM_REG_RISCV_SBI_STATE |
KVM_REG_RISCV_SBI_STA | i;
if (uindices) {
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
}
total += n;
- }
- return total;
-}
static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu) {
- return copy_sbi_reg_indices(vcpu, NULL);
- return kvm_riscv_vcpu_reg_indices_sbi(vcpu, NULL);
} static inline unsigned long num_vector_regs(const struct kvm_vcpu *vcpu) @@ -1247,7 +1220,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu, return ret; uindices += ret;
- ret = copy_sbi_reg_indices(vcpu, uindices);
- ret = kvm_riscv_vcpu_reg_indices_sbi(vcpu, uindices); if (ret < 0) return ret; uindices += ret;
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 01a93f4fdb16..8b3c393e0c83 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -364,64 +364,163 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, return 0; } -int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
+int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices) +{
- struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
- const struct kvm_riscv_sbi_extension_entry *entry;
- const struct kvm_vcpu_sbi_extension *ext;
- unsigned long state_reg_count;
- int i, j, rc, count = 0;
- u64 reg;
- for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
entry = &sbi_ext[i];
ext = entry->ext_ptr;
if (!ext->have_state ||
scontext->ext_status[entry->ext_idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED)
continue;
state_reg_count = ext->get_state_reg_count(vcpu);
if (!uindices)
goto skip_put_user;
for (j = 0; j < state_reg_count; j++) {
if (ext->get_state_reg_id) {
rc = ext->get_state_reg_id(vcpu, j, ®);
if (rc)
return rc;
} else {
reg = KVM_REG_RISCV |
(IS_ENABLED(CONFIG_32BIT) ?
KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64) |
KVM_REG_RISCV_SBI_STATE |
ext->state_reg_subtype | j;
}
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
+skip_put_user:
count += state_reg_count;
- }
- return count;
+}
+static const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext_withstate(struct kvm_vcpu *vcpu,
unsigned long subtype)
+{
- struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
- const struct kvm_riscv_sbi_extension_entry *entry;
- const struct kvm_vcpu_sbi_extension *ext;
- int i;
- for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
entry = &sbi_ext[i];
ext = entry->ext_ptr;
if (ext->have_state &&
ext->state_reg_subtype == subtype &&
scontext->ext_status[entry->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_ENABLED)
return ext;
- }
- return NULL;
+}
+int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE);
- unsigned long reg_subtype, reg_val;
- if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
- const struct kvm_vcpu_sbi_extension *ext;
- unsigned long reg_subtype;
- void *reg_val;
- u64 data64;
- u32 data32;
- u16 data16;
- u8 data8;
- switch (KVM_REG_SIZE(reg->id)) {
- case 1:
reg_val = &data8;
break;
- case 2:
reg_val = &data16;
break;
- case 4:
reg_val = &data32;
break;
- case 8:
reg_val = &data64;
break;
- default: return -EINVAL;
- };
superfluous ';'
- if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id)))
- if (copy_from_user(reg_val, uaddr, KVM_REG_SIZE(reg->id))) return -EFAULT;
reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
- switch (reg_subtype) {
- case KVM_REG_RISCV_SBI_STA:
return kvm_riscv_vcpu_set_reg_sbi_sta(vcpu, reg_num, reg_val);
- default:
- ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype);
- if (!ext || !ext->set_state_reg) return -EINVAL;
- }
- return 0;
- return ext->set_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val);
} -int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
+int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE);
- unsigned long reg_subtype, reg_val;
- const struct kvm_vcpu_sbi_extension *ext;
- unsigned long reg_subtype;
- void *reg_val;
- u64 data64;
- u32 data32;
- u16 data16;
- u8 data8; int ret;
- if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
- switch (KVM_REG_SIZE(reg->id)) {
- case 1:
reg_val = &data8;
break;
- case 2:
reg_val = &data16;
break;
- case 4:
reg_val = &data32;
break;
- case 8:
reg_val = &data64;
break;
- default: return -EINVAL;
- };
superfluous ';'
reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
- switch (reg_subtype) {
- case KVM_REG_RISCV_SBI_STA:
ret = kvm_riscv_vcpu_get_reg_sbi_sta(vcpu, reg_num, ®_val);
break;
- default:
- ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype);
- if (!ext || !ext->get_state_reg) return -EINVAL;
- }
- ret = ext->get_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val); if (ret) return ret;
- if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
- if (copy_to_user(uaddr, reg_val, KVM_REG_SIZE(reg->id))) return -EFAULT;
return 0; diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index cc6cb7c8f0e4..d14cf6255d83 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -151,63 +151,83 @@ static unsigned long kvm_sbi_ext_sta_probe(struct kvm_vcpu *vcpu) return !!sched_info_on(); } -const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = {
- .extid_start = SBI_EXT_STA,
- .extid_end = SBI_EXT_STA,
- .handler = kvm_sbi_ext_sta_handler,
- .probe = kvm_sbi_ext_sta_probe,
- .reset = kvm_riscv_vcpu_sbi_sta_reset,
-}; +static unsigned long kvm_sbi_ext_sta_get_state_reg_count(struct kvm_vcpu *vcpu) +{
- return sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long);
+} -int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu,
unsigned long reg_num,
unsigned long *reg_val)
+static int kvm_sbi_ext_sta_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, void *reg_val)
{
- unsigned long *value;
- if (reg_size != sizeof(unsigned long))
return -EINVAL;
- value = reg_val;
- switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo):
*reg_val = (unsigned long)vcpu->arch.sta.shmem;
break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT))*value = (unsigned long)vcpu->arch.sta.shmem;
*reg_val = upper_32_bits(vcpu->arch.sta.shmem);
else*value = upper_32_bits(vcpu->arch.sta.shmem);
*reg_val = 0;
break; default:*value = 0;
return -EINVAL;
}return -ENOENT;
return 0; } -int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu,
unsigned long reg_num,
unsigned long reg_val)
+static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, const void *reg_val)
{
- unsigned long value;
- if (reg_size != sizeof(unsigned long))
return -EINVAL;
- value = *(const unsigned long *)reg_val;
- switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t hi = upper_32_bits(vcpu->arch.sta.shmem);
vcpu->arch.sta.shmem = reg_val;
} else {vcpu->arch.sta.shmem = value; vcpu->arch.sta.shmem |= hi << 32;
vcpu->arch.sta.shmem = reg_val;
} break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t lo = lower_32_bits(vcpu->arch.sta.shmem);vcpu->arch.sta.shmem = value;
vcpu->arch.sta.shmem = ((gpa_t)reg_val << 32);
vcpu->arch.sta.shmem = ((gpa_t)value << 32); vcpu->arch.sta.shmem |= lo;
} else if (reg_val != 0) {
} break; default:} else if (value != 0) { return -EINVAL;
return -EINVAL;
}return -ENOENT;
return 0; }
+const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = {
- .extid_start = SBI_EXT_STA,
- .extid_end = SBI_EXT_STA,
- .handler = kvm_sbi_ext_sta_handler,
- .probe = kvm_sbi_ext_sta_probe,
- .reset = kvm_riscv_vcpu_sbi_sta_reset,
- .have_state = true,
- .state_reg_subtype = KVM_REG_RISCV_SBI_STA,
- .get_state_reg_count = kvm_sbi_ext_sta_get_state_reg_count,
- .get_state_reg = kvm_sbi_ext_sta_get_reg,
- .set_state_reg = kvm_sbi_ext_sta_set_reg,
+};
2.43.0
Otherwise,
Reviewed-by: Andrew Jones ajones@ventanamicro.com
On Sat, Aug 16, 2025 at 2:30 AM Andrew Jones ajones@ventanamicro.com wrote:
On Thu, Aug 14, 2025 at 09:25:45PM +0530, Anup Patel wrote:
SBI extensions can have per-VCPU state which needs to be saved/restored through ONE_REG interface for Guest/VM migration. Introduce optional ONE_REG callbacks for SBI extensions so that ONE_REG implementation for an SBI extenion is part of the extension sources.
Signed-off-by: Anup Patel apatel@ventanamicro.com
arch/riscv/include/asm/kvm_vcpu_sbi.h | 21 ++-- arch/riscv/kvm/vcpu_onereg.c | 31 +----- arch/riscv/kvm/vcpu_sbi.c | 145 ++++++++++++++++++++++---- arch/riscv/kvm/vcpu_sbi_sta.c | 64 ++++++++---- 4 files changed, 178 insertions(+), 83 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 766031e80960..144c3f6d5eb9 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -59,6 +59,15 @@ struct kvm_vcpu_sbi_extension { void (*deinit)(struct kvm_vcpu *vcpu);
void (*reset)(struct kvm_vcpu *vcpu);
bool have_state;
unsigned long state_reg_subtype;
unsigned long (*get_state_reg_count)(struct kvm_vcpu *vcpu);
I think we can drop 'have_state'. When 'get_state_reg_count' is NULL, then the state reg count must be zero (i.e. have_state == false).
Good suggestion. I will update in the next revision.
int (*get_state_reg_id)(struct kvm_vcpu *vcpu, int index, u64 *reg_id);
int (*get_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, void *reg_val);
int (*set_state_reg)(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, const void *reg_val);
};
void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -73,10 +82,9 @@ int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); -int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg);
-int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg);
+int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices); +int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); +int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( struct kvm_vcpu *vcpu, unsigned long extid); bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx); @@ -85,11 +93,6 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_reset(struct kvm_vcpu *vcpu);
-int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long *reg_val);
-int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_val);
#ifdef CONFIG_RISCV_SBI_V01 extern const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_v01; #endif diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index b77748a56a59..5843b0519224 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -1090,36 +1090,9 @@ static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu) return copy_sbi_ext_reg_indices(vcpu, NULL); }
-static int copy_sbi_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) -{
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
int total = 0;
if (scontext->ext_status[KVM_RISCV_SBI_EXT_STA] == KVM_RISCV_SBI_EXT_STATUS_ENABLED) {
u64 size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
int n = sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long);
for (int i = 0; i < n; i++) {
u64 reg = KVM_REG_RISCV | size |
KVM_REG_RISCV_SBI_STATE |
KVM_REG_RISCV_SBI_STA | i;
if (uindices) {
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
}
total += n;
}
return total;
-}
static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu) {
return copy_sbi_reg_indices(vcpu, NULL);
return kvm_riscv_vcpu_reg_indices_sbi(vcpu, NULL);
}
static inline unsigned long num_vector_regs(const struct kvm_vcpu *vcpu) @@ -1247,7 +1220,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu, return ret; uindices += ret;
ret = copy_sbi_reg_indices(vcpu, uindices);
ret = kvm_riscv_vcpu_reg_indices_sbi(vcpu, uindices); if (ret < 0) return ret; uindices += ret;
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 01a93f4fdb16..8b3c393e0c83 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -364,64 +364,163 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, return 0; }
-int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
+int kvm_riscv_vcpu_reg_indices_sbi(struct kvm_vcpu *vcpu, u64 __user *uindices) +{
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
const struct kvm_riscv_sbi_extension_entry *entry;
const struct kvm_vcpu_sbi_extension *ext;
unsigned long state_reg_count;
int i, j, rc, count = 0;
u64 reg;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
entry = &sbi_ext[i];
ext = entry->ext_ptr;
if (!ext->have_state ||
scontext->ext_status[entry->ext_idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED)
continue;
state_reg_count = ext->get_state_reg_count(vcpu);
if (!uindices)
goto skip_put_user;
for (j = 0; j < state_reg_count; j++) {
if (ext->get_state_reg_id) {
rc = ext->get_state_reg_id(vcpu, j, ®);
if (rc)
return rc;
} else {
reg = KVM_REG_RISCV |
(IS_ENABLED(CONFIG_32BIT) ?
KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64) |
KVM_REG_RISCV_SBI_STATE |
ext->state_reg_subtype | j;
}
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
+skip_put_user:
count += state_reg_count;
}
return count;
+}
+static const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext_withstate(struct kvm_vcpu *vcpu,
unsigned long subtype)
+{
struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
const struct kvm_riscv_sbi_extension_entry *entry;
const struct kvm_vcpu_sbi_extension *ext;
int i;
for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
entry = &sbi_ext[i];
ext = entry->ext_ptr;
if (ext->have_state &&
ext->state_reg_subtype == subtype &&
scontext->ext_status[entry->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_ENABLED)
return ext;
}
return NULL;
+}
+int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE);
unsigned long reg_subtype, reg_val;
if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
const struct kvm_vcpu_sbi_extension *ext;
unsigned long reg_subtype;
void *reg_val;
u64 data64;
u32 data32;
u16 data16;
u8 data8;
switch (KVM_REG_SIZE(reg->id)) {
case 1:
reg_val = &data8;
break;
case 2:
reg_val = &data16;
break;
case 4:
reg_val = &data32;
break;
case 8:
reg_val = &data64;
break;
default: return -EINVAL;
};
superfluous ';'
Okay, I will update in the next revision.
if (copy_from_user(®_val, uaddr, KVM_REG_SIZE(reg->id)))
if (copy_from_user(reg_val, uaddr, KVM_REG_SIZE(reg->id))) return -EFAULT; reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
switch (reg_subtype) {
case KVM_REG_RISCV_SBI_STA:
return kvm_riscv_vcpu_set_reg_sbi_sta(vcpu, reg_num, reg_val);
default:
ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype);
if (!ext || !ext->set_state_reg) return -EINVAL;
}
return 0;
return ext->set_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val);
}
-int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu,
const struct kvm_one_reg *reg)
+int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { unsigned long __user *uaddr = (unsigned long __user *)(unsigned long)reg->addr; unsigned long reg_num = reg->id & ~(KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_RISCV_SBI_STATE);
unsigned long reg_subtype, reg_val;
const struct kvm_vcpu_sbi_extension *ext;
unsigned long reg_subtype;
void *reg_val;
u64 data64;
u32 data32;
u16 data16;
u8 data8; int ret;
if (KVM_REG_SIZE(reg->id) != sizeof(unsigned long))
switch (KVM_REG_SIZE(reg->id)) {
case 1:
reg_val = &data8;
break;
case 2:
reg_val = &data16;
break;
case 4:
reg_val = &data32;
break;
case 8:
reg_val = &data64;
break;
default: return -EINVAL;
};
superfluous ';'
Okay, I will update in the next revision.
reg_subtype = reg_num & KVM_REG_RISCV_SUBTYPE_MASK; reg_num &= ~KVM_REG_RISCV_SUBTYPE_MASK;
switch (reg_subtype) {
case KVM_REG_RISCV_SBI_STA:
ret = kvm_riscv_vcpu_get_reg_sbi_sta(vcpu, reg_num, ®_val);
break;
default:
ext = kvm_vcpu_sbi_find_ext_withstate(vcpu, reg_subtype);
if (!ext || !ext->get_state_reg) return -EINVAL;
}
ret = ext->get_state_reg(vcpu, reg_num, KVM_REG_SIZE(reg->id), reg_val); if (ret) return ret;
if (copy_to_user(uaddr, ®_val, KVM_REG_SIZE(reg->id)))
if (copy_to_user(uaddr, reg_val, KVM_REG_SIZE(reg->id))) return -EFAULT; return 0;
diff --git a/arch/riscv/kvm/vcpu_sbi_sta.c b/arch/riscv/kvm/vcpu_sbi_sta.c index cc6cb7c8f0e4..d14cf6255d83 100644 --- a/arch/riscv/kvm/vcpu_sbi_sta.c +++ b/arch/riscv/kvm/vcpu_sbi_sta.c @@ -151,63 +151,83 @@ static unsigned long kvm_sbi_ext_sta_probe(struct kvm_vcpu *vcpu) return !!sched_info_on(); }
-const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = {
.extid_start = SBI_EXT_STA,
.extid_end = SBI_EXT_STA,
.handler = kvm_sbi_ext_sta_handler,
.probe = kvm_sbi_ext_sta_probe,
.reset = kvm_riscv_vcpu_sbi_sta_reset,
-}; +static unsigned long kvm_sbi_ext_sta_get_state_reg_count(struct kvm_vcpu *vcpu) +{
return sizeof(struct kvm_riscv_sbi_sta) / sizeof(unsigned long);
+}
-int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu,
unsigned long reg_num,
unsigned long *reg_val)
+static int kvm_sbi_ext_sta_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, void *reg_val)
{
unsigned long *value;
if (reg_size != sizeof(unsigned long))
return -EINVAL;
value = reg_val;
switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo):
*reg_val = (unsigned long)vcpu->arch.sta.shmem;
*value = (unsigned long)vcpu->arch.sta.shmem; break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT))
*reg_val = upper_32_bits(vcpu->arch.sta.shmem);
*value = upper_32_bits(vcpu->arch.sta.shmem); else
*reg_val = 0;
*value = 0; break; default:
return -EINVAL;
return -ENOENT; } return 0;
}
-int kvm_riscv_vcpu_set_reg_sbi_sta(struct kvm_vcpu *vcpu,
unsigned long reg_num,
unsigned long reg_val)
+static int kvm_sbi_ext_sta_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num,
unsigned long reg_size, const void *reg_val)
{
unsigned long value;
if (reg_size != sizeof(unsigned long))
return -EINVAL;
value = *(const unsigned long *)reg_val;
switch (reg_num) { case KVM_REG_RISCV_SBI_STA_REG(shmem_lo): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t hi = upper_32_bits(vcpu->arch.sta.shmem);
vcpu->arch.sta.shmem = reg_val;
vcpu->arch.sta.shmem = value; vcpu->arch.sta.shmem |= hi << 32; } else {
vcpu->arch.sta.shmem = reg_val;
vcpu->arch.sta.shmem = value; } break; case KVM_REG_RISCV_SBI_STA_REG(shmem_hi): if (IS_ENABLED(CONFIG_32BIT)) { gpa_t lo = lower_32_bits(vcpu->arch.sta.shmem);
vcpu->arch.sta.shmem = ((gpa_t)reg_val << 32);
vcpu->arch.sta.shmem = ((gpa_t)value << 32); vcpu->arch.sta.shmem |= lo;
} else if (reg_val != 0) {
} else if (value != 0) { return -EINVAL; } break; default:
return -EINVAL;
return -ENOENT; } return 0;
}
+const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_sta = {
.extid_start = SBI_EXT_STA,
.extid_end = SBI_EXT_STA,
.handler = kvm_sbi_ext_sta_handler,
.probe = kvm_sbi_ext_sta_probe,
.reset = kvm_riscv_vcpu_sbi_sta_reset,
.have_state = true,
.state_reg_subtype = KVM_REG_RISCV_SBI_STA,
.get_state_reg_count = kvm_sbi_ext_sta_get_state_reg_count,
.get_state_reg = kvm_sbi_ext_sta_get_reg,
.set_state_reg = kvm_sbi_ext_sta_set_reg,
+};
2.43.0
Otherwise,
Reviewed-by: Andrew Jones ajones@ventanamicro.com
Thanks, Anup
The ONE_REG handling of SBI extension enable/disable registers and SBI extension state registers is already under SBI implementation. On similar lines, let's move copy_sbi_ext_reg_indices() under SBI implementation.
Signed-off-by: Anup Patel apatel@ventanamicro.com --- arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 +- arch/riscv/kvm/vcpu_onereg.c | 29 ++------------------------- arch/riscv/kvm/vcpu_sbi.c | 27 ++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 144c3f6d5eb9..212c31629bc8 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -78,6 +78,7 @@ void kvm_riscv_vcpu_sbi_request_reset(struct kvm_vcpu *vcpu, unsigned long pc, unsigned long a1); void kvm_riscv_vcpu_sbi_load_reset_state(struct kvm_vcpu *vcpu); int kvm_riscv_vcpu_sbi_return(struct kvm_vcpu *vcpu, struct kvm_run *run); +int kvm_riscv_vcpu_reg_indices_sbi_ext(struct kvm_vcpu *vcpu, u64 __user *uindices); int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, @@ -87,7 +88,6 @@ int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg * int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( struct kvm_vcpu *vcpu, unsigned long extid); -bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx); int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run); void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu); diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index 5843b0519224..0894ab517525 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -1060,34 +1060,9 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu) return copy_isa_ext_reg_indices(vcpu, NULL); }
-static int copy_sbi_ext_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) -{ - unsigned int n = 0; - - for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) { - u64 size = IS_ENABLED(CONFIG_32BIT) ? - KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64; - u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT | - KVM_REG_RISCV_SBI_SINGLE | i; - - if (!riscv_vcpu_supports_sbi_ext(vcpu, i)) - continue; - - if (uindices) { - if (put_user(reg, uindices)) - return -EFAULT; - uindices++; - } - - n++; - } - - return n; -} - static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu) { - return copy_sbi_ext_reg_indices(vcpu, NULL); + return kvm_riscv_vcpu_reg_indices_sbi_ext(vcpu, NULL); }
static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu) @@ -1215,7 +1190,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu, return ret; uindices += ret;
- ret = copy_sbi_ext_reg_indices(vcpu, uindices); + ret = kvm_riscv_vcpu_reg_indices_sbi_ext(vcpu, uindices); if (ret < 0) return ret; uindices += ret; diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 8b3c393e0c83..19e0e3d7b598 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -110,7 +110,7 @@ riscv_vcpu_get_sbi_ext(struct kvm_vcpu *vcpu, unsigned long idx) return sext; }
-bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx) +static bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx) { struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context; const struct kvm_riscv_sbi_extension_entry *sext; @@ -288,6 +288,31 @@ static int riscv_vcpu_get_sbi_ext_multi(struct kvm_vcpu *vcpu, return 0; }
+int kvm_riscv_vcpu_reg_indices_sbi_ext(struct kvm_vcpu *vcpu, u64 __user *uindices) +{ + unsigned int n = 0; + + for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) { + u64 size = IS_ENABLED(CONFIG_32BIT) ? + KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64; + u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT | + KVM_REG_RISCV_SBI_SINGLE | i; + + if (!riscv_vcpu_supports_sbi_ext(vcpu, i)) + continue; + + if (uindices) { + if (put_user(reg, uindices)) + return -EFAULT; + uindices++; + } + + n++; + } + + return n; +} + int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) {
On Thu, Aug 14, 2025 at 09:25:46PM +0530, Anup Patel wrote:
The ONE_REG handling of SBI extension enable/disable registers and SBI extension state registers is already under SBI implementation. On similar lines, let's move copy_sbi_ext_reg_indices() under SBI implementation.
Signed-off-by: Anup Patel apatel@ventanamicro.com
arch/riscv/include/asm/kvm_vcpu_sbi.h | 2 +- arch/riscv/kvm/vcpu_onereg.c | 29 ++------------------------- arch/riscv/kvm/vcpu_sbi.c | 27 ++++++++++++++++++++++++- 3 files changed, 29 insertions(+), 29 deletions(-)
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index 144c3f6d5eb9..212c31629bc8 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -78,6 +78,7 @@ void kvm_riscv_vcpu_sbi_request_reset(struct kvm_vcpu *vcpu, unsigned long pc, unsigned long a1); void kvm_riscv_vcpu_sbi_load_reset_state(struct kvm_vcpu *vcpu); int kvm_riscv_vcpu_sbi_return(struct kvm_vcpu *vcpu, struct kvm_run *run); +int kvm_riscv_vcpu_reg_indices_sbi_ext(struct kvm_vcpu *vcpu, u64 __user *uindices); int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu, @@ -87,7 +88,6 @@ int kvm_riscv_vcpu_set_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg * int kvm_riscv_vcpu_get_reg_sbi(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg); const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( struct kvm_vcpu *vcpu, unsigned long extid); -bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx); int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run); void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu); void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu); diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c index 5843b0519224..0894ab517525 100644 --- a/arch/riscv/kvm/vcpu_onereg.c +++ b/arch/riscv/kvm/vcpu_onereg.c @@ -1060,34 +1060,9 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu) return copy_isa_ext_reg_indices(vcpu, NULL); } -static int copy_sbi_ext_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices) -{
- unsigned int n = 0;
- for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
u64 size = IS_ENABLED(CONFIG_32BIT) ?
KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
KVM_REG_RISCV_SBI_SINGLE | i;
if (!riscv_vcpu_supports_sbi_ext(vcpu, i))
continue;
if (uindices) {
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
n++;
- }
- return n;
-}
static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu) {
- return copy_sbi_ext_reg_indices(vcpu, NULL);
- return kvm_riscv_vcpu_reg_indices_sbi_ext(vcpu, NULL);
} static inline unsigned long num_sbi_regs(struct kvm_vcpu *vcpu) @@ -1215,7 +1190,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu, return ret; uindices += ret;
- ret = copy_sbi_ext_reg_indices(vcpu, uindices);
- ret = kvm_riscv_vcpu_reg_indices_sbi_ext(vcpu, uindices); if (ret < 0) return ret; uindices += ret;
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 8b3c393e0c83..19e0e3d7b598 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -110,7 +110,7 @@ riscv_vcpu_get_sbi_ext(struct kvm_vcpu *vcpu, unsigned long idx) return sext; } -bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx) +static bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx) { struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context; const struct kvm_riscv_sbi_extension_entry *sext; @@ -288,6 +288,31 @@ static int riscv_vcpu_get_sbi_ext_multi(struct kvm_vcpu *vcpu, return 0; } +int kvm_riscv_vcpu_reg_indices_sbi_ext(struct kvm_vcpu *vcpu, u64 __user *uindices) +{
- unsigned int n = 0;
- for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
u64 size = IS_ENABLED(CONFIG_32BIT) ?
KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
KVM_REG_RISCV_SBI_SINGLE | i;
if (!riscv_vcpu_supports_sbi_ext(vcpu, i))
continue;
if (uindices) {
if (put_user(reg, uindices))
return -EFAULT;
uindices++;
}
n++;
- }
- return n;
+}
int kvm_riscv_vcpu_set_reg_sbi_ext(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg) { -- 2.43.0
Reviewed-by: Andrew Jones ajones@ventanamicro.com
The KVM user-space needs a way to save/restore the state of SBI FWFT features so implement SBI extension ONE_REG callbacks.
Signed-off-by: Anup Patel apatel@ventanamicro.com --- arch/riscv/include/uapi/asm/kvm.h | 14 +++ arch/riscv/kvm/vcpu_sbi_fwft.c | 169 +++++++++++++++++++++++++++--- 2 files changed, 171 insertions(+), 12 deletions(-)
diff --git a/arch/riscv/include/uapi/asm/kvm.h b/arch/riscv/include/uapi/asm/kvm.h index a5ca0f4ce2d3..fc5624e89c7b 100644 --- a/arch/riscv/include/uapi/asm/kvm.h +++ b/arch/riscv/include/uapi/asm/kvm.h @@ -215,6 +215,17 @@ struct kvm_riscv_sbi_sta { unsigned long shmem_hi; };
+struct kvm_riscv_sbi_fwft_feature { + unsigned long flags; + unsigned long value; +}; + +/* SBI FWFT extension registers for KVM_GET_ONE_REG and KVM_SET_ONE_REG */ +struct kvm_riscv_sbi_fwft { + struct kvm_riscv_sbi_fwft_feature misaligned_deleg; + struct kvm_riscv_sbi_fwft_feature pointer_masking; +}; + /* Possible states for kvm_riscv_timer */ #define KVM_RISCV_TIMER_STATE_OFF 0 #define KVM_RISCV_TIMER_STATE_ON 1 @@ -298,6 +309,9 @@ struct kvm_riscv_sbi_sta { #define KVM_REG_RISCV_SBI_STA (0x0 << KVM_REG_RISCV_SUBTYPE_SHIFT) #define KVM_REG_RISCV_SBI_STA_REG(name) \ (offsetof(struct kvm_riscv_sbi_sta, name) / sizeof(unsigned long)) +#define KVM_REG_RISCV_SBI_FWFT (0x1 << KVM_REG_RISCV_SUBTYPE_SHIFT) +#define KVM_REG_RISCV_SBI_FWFT_REG(name) \ + (offsetof(struct kvm_riscv_sbi_fwft, name) / sizeof(unsigned long))
/* Device Control API: RISC-V AIA */ #define KVM_DEV_RISCV_APLIC_ALIGN 0x1000 diff --git a/arch/riscv/kvm/vcpu_sbi_fwft.c b/arch/riscv/kvm/vcpu_sbi_fwft.c index 5a3bad0f9330..0d740e7c5713 100644 --- a/arch/riscv/kvm/vcpu_sbi_fwft.c +++ b/arch/riscv/kvm/vcpu_sbi_fwft.c @@ -22,6 +22,11 @@ struct kvm_sbi_fwft_feature { */ enum sbi_fwft_feature_t id;
+ /** + * @flags_reg_num: ONE_REG index of the feature flag + */ + unsigned long flags_reg_num; + /** * @supported: Check if the feature is supported on the vcpu * @@ -44,7 +49,8 @@ struct kvm_sbi_fwft_feature { * * This callback is mandatory */ - long (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long value); + long (*set)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, + bool one_reg_access, unsigned long value);
/** * @get: Get the feature current value @@ -53,7 +59,8 @@ struct kvm_sbi_fwft_feature { * * This callback is mandatory */ - long (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, unsigned long *value); + long (*get)(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, + bool one_reg_access, unsigned long *value); };
static const enum sbi_fwft_feature_t kvm_fwft_defined_features[] = { @@ -91,16 +98,18 @@ static void kvm_sbi_fwft_reset_misaligned_delegation(struct kvm_vcpu *vcpu)
static long kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, - unsigned long value) + bool one_reg_access, unsigned long value) { struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
if (value == 1) { cfg->hedeleg |= MIS_DELEG; - csr_set(CSR_HEDELEG, MIS_DELEG); + if (!one_reg_access) + csr_set(CSR_HEDELEG, MIS_DELEG); } else if (value == 0) { cfg->hedeleg &= ~MIS_DELEG; - csr_clear(CSR_HEDELEG, MIS_DELEG); + if (!one_reg_access) + csr_clear(CSR_HEDELEG, MIS_DELEG); } else { return SBI_ERR_INVALID_PARAM; } @@ -110,10 +119,11 @@ static long kvm_sbi_fwft_set_misaligned_delegation(struct kvm_vcpu *vcpu,
static long kvm_sbi_fwft_get_misaligned_delegation(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, - unsigned long *value) + bool one_reg_access, unsigned long *value) { - *value = (csr_read(CSR_HEDELEG) & MIS_DELEG) == MIS_DELEG; + struct kvm_vcpu_config *cfg = &vcpu->arch.cfg;
+ *value = (cfg->hedeleg & MIS_DELEG) == MIS_DELEG; return SBI_SUCCESS; }
@@ -145,7 +155,7 @@ static void kvm_sbi_fwft_reset_pointer_masking_pmlen(struct kvm_vcpu *vcpu)
static long kvm_sbi_fwft_set_pointer_masking_pmlen(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, - unsigned long value) + bool one_reg_access, unsigned long value) { struct kvm_sbi_fwft *fwft = vcpu_to_fwft(vcpu); unsigned long pmm; @@ -167,14 +177,15 @@ static long kvm_sbi_fwft_set_pointer_masking_pmlen(struct kvm_vcpu *vcpu, * update here so that VCPU see's pointer masking mode change * immediately. */ - csr_write(CSR_HENVCFG, vcpu->arch.cfg.henvcfg); + if (!one_reg_access) + csr_write(CSR_HENVCFG, vcpu->arch.cfg.henvcfg);
return SBI_SUCCESS; }
static long kvm_sbi_fwft_get_pointer_masking_pmlen(struct kvm_vcpu *vcpu, struct kvm_sbi_fwft_config *conf, - unsigned long *value) + bool one_reg_access, unsigned long *value) { switch (vcpu->arch.cfg.henvcfg & ENVCFG_PMM) { case ENVCFG_PMM_PMLEN_0: @@ -198,6 +209,8 @@ static long kvm_sbi_fwft_get_pointer_masking_pmlen(struct kvm_vcpu *vcpu, static const struct kvm_sbi_fwft_feature features[] = { { .id = SBI_FWFT_MISALIGNED_EXC_DELEG, + .flags_reg_num = offsetof(struct kvm_riscv_sbi_fwft, misaligned_deleg.flags) / + sizeof(unsigned long), .supported = kvm_sbi_fwft_misaligned_delegation_supported, .reset = kvm_sbi_fwft_reset_misaligned_delegation, .set = kvm_sbi_fwft_set_misaligned_delegation, @@ -206,6 +219,8 @@ static const struct kvm_sbi_fwft_feature features[] = { #ifndef CONFIG_32BIT { .id = SBI_FWFT_POINTER_MASKING_PMLEN, + .flags_reg_num = offsetof(struct kvm_riscv_sbi_fwft, pointer_masking.flags) / + sizeof(unsigned long), .supported = kvm_sbi_fwft_pointer_masking_pmlen_supported, .reset = kvm_sbi_fwft_reset_pointer_masking_pmlen, .set = kvm_sbi_fwft_set_pointer_masking_pmlen, @@ -214,6 +229,21 @@ static const struct kvm_sbi_fwft_feature features[] = { #endif };
+static const struct kvm_sbi_fwft_feature *kvm_sbi_fwft_regnum_to_feature(unsigned long reg_num) +{ + const struct kvm_sbi_fwft_feature *feature; + int i; + + for (i = 0; i < ARRAY_SIZE(features); i++) { + feature = &features[i]; + if (feature->flags_reg_num == reg_num || + (feature->flags_reg_num + 1) == reg_num) + return feature; + } + + return NULL; +} + static struct kvm_sbi_fwft_config * kvm_sbi_fwft_get_config(struct kvm_vcpu *vcpu, enum sbi_fwft_feature_t feature) { @@ -267,7 +297,7 @@ static int kvm_sbi_fwft_set(struct kvm_vcpu *vcpu, u32 feature,
conf->flags = flags;
- return conf->feature->set(vcpu, conf, value); + return conf->feature->set(vcpu, conf, false, value); }
static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu, unsigned long feature, @@ -280,7 +310,7 @@ static int kvm_sbi_fwft_get(struct kvm_vcpu *vcpu, unsigned long feature, if (ret) return ret;
- return conf->feature->get(vcpu, conf, value); + return conf->feature->get(vcpu, conf, false, value); }
static int kvm_sbi_ext_fwft_handler(struct kvm_vcpu *vcpu, struct kvm_run *run, @@ -354,6 +384,115 @@ static void kvm_sbi_ext_fwft_reset(struct kvm_vcpu *vcpu) } }
+static unsigned long kvm_sbi_ext_fwft_get_reg_count(struct kvm_vcpu *vcpu) +{ + unsigned long max_reg_count = sizeof(struct kvm_riscv_sbi_fwft) / sizeof(unsigned long); + const struct kvm_sbi_fwft_feature *feature; + struct kvm_sbi_fwft_config *conf; + unsigned long reg, ret = 0; + + for (reg = 0; reg < max_reg_count; reg++) { + feature = kvm_sbi_fwft_regnum_to_feature(reg); + if (!feature) + continue; + + conf = kvm_sbi_fwft_get_config(vcpu, feature->id); + if (!conf || !conf->supported) + continue; + + ret++; + } + + return ret; +} + +static int kvm_sbi_ext_fwft_get_reg_id(struct kvm_vcpu *vcpu, int index, u64 *reg_id) +{ + int reg, max_reg_count = sizeof(struct kvm_riscv_sbi_fwft) / sizeof(unsigned long); + const struct kvm_sbi_fwft_feature *feature; + struct kvm_sbi_fwft_config *conf; + int idx = 0; + + for (reg = 0; reg < max_reg_count; reg++) { + feature = kvm_sbi_fwft_regnum_to_feature(reg); + if (!feature) + continue; + + conf = kvm_sbi_fwft_get_config(vcpu, feature->id); + if (!conf || !conf->supported) + continue; + + if (index == idx) { + *reg_id = KVM_REG_RISCV | + (IS_ENABLED(CONFIG_32BIT) ? + KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64) | + KVM_REG_RISCV_SBI_STATE | + KVM_REG_RISCV_SBI_FWFT | reg; + return 0; + } + + idx++; + } + + return -ENOENT; +} + +static int kvm_sbi_ext_fwft_get_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, void *reg_val) +{ + const struct kvm_sbi_fwft_feature *feature; + struct kvm_sbi_fwft_config *conf; + unsigned long *value; + int ret = 0; + + if (reg_size != sizeof(unsigned long)) + return -EINVAL; + value = reg_val; + + feature = kvm_sbi_fwft_regnum_to_feature(reg_num); + if (!feature) + return -ENOENT; + + conf = kvm_sbi_fwft_get_config(vcpu, feature->id); + if (!conf || !conf->supported) + return -ENOENT; + + if (feature->flags_reg_num == reg_num) + *value = conf->flags; + else + ret = conf->feature->get(vcpu, conf, true, value); + + return sbi_err_map_linux_errno(ret); +} + +static int kvm_sbi_ext_fwft_set_reg(struct kvm_vcpu *vcpu, unsigned long reg_num, + unsigned long reg_size, const void *reg_val) +{ + const struct kvm_sbi_fwft_feature *feature; + struct kvm_sbi_fwft_config *conf; + unsigned long value; + int ret = 0; + + if (reg_size != sizeof(unsigned long)) + return -EINVAL; + value = *(const unsigned long *)reg_val; + + feature = kvm_sbi_fwft_regnum_to_feature(reg_num); + if (!feature) + return -ENOENT; + + conf = kvm_sbi_fwft_get_config(vcpu, feature->id); + if (!conf || !conf->supported) + return -ENOENT; + + if (feature->flags_reg_num == reg_num) + conf->flags = value & SBI_FWFT_SET_FLAG_LOCK; + else + ret = conf->feature->set(vcpu, conf, true, value); + + return sbi_err_map_linux_errno(ret); +} + const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = { .extid_start = SBI_EXT_FWFT, .extid_end = SBI_EXT_FWFT, @@ -361,4 +500,10 @@ const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_fwft = { .init = kvm_sbi_ext_fwft_init, .deinit = kvm_sbi_ext_fwft_deinit, .reset = kvm_sbi_ext_fwft_reset, + .have_state = true, + .state_reg_subtype = KVM_REG_RISCV_SBI_FWFT, + .get_state_reg_count = kvm_sbi_ext_fwft_get_reg_count, + .get_state_reg_id = kvm_sbi_ext_fwft_get_reg_id, + .get_state_reg = kvm_sbi_ext_fwft_get_reg, + .set_state_reg = kvm_sbi_ext_fwft_set_reg, };
KVM RISC-V now supports SBI FWFT, so add it to the get-reg-list test.
Signed-off-by: Anup Patel apatel@ventanamicro.com --- .../selftests/kvm/riscv/get-reg-list.c | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c index a0b7dabb5040..1bc84f09b4ee 100644 --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c @@ -128,6 +128,7 @@ bool filter_reg(__u64 reg) case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN: case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SUSP: case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_STA: + case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_FWFT: case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL: case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR: return true; @@ -627,6 +628,7 @@ static const char *sbi_ext_single_id_to_str(__u64 reg_off) KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_DBCN), KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_SUSP), KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_STA), + KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_FWFT), KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_EXPERIMENTAL), KVM_SBI_EXT_ARR(KVM_RISCV_SBI_EXT_VENDOR), }; @@ -683,6 +685,17 @@ static const char *sbi_sta_id_to_str(__u64 reg_off) return strdup_printf("KVM_REG_RISCV_SBI_STA | %lld /* UNKNOWN */", reg_off); }
+static const char *sbi_fwft_id_to_str(__u64 reg_off) +{ + switch (reg_off) { + case 0: return "KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.flags)"; + case 1: return "KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.value)"; + case 2: return "KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.flags)"; + case 3: return "KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.value)"; + } + return strdup_printf("KVM_REG_RISCV_SBI_STA | %lld /* UNKNOWN */", reg_off); +} + static const char *sbi_id_to_str(const char *prefix, __u64 id) { __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_SBI_STATE); @@ -695,6 +708,8 @@ static const char *sbi_id_to_str(const char *prefix, __u64 id) switch (reg_subtype) { case KVM_REG_RISCV_SBI_STA: return sbi_sta_id_to_str(reg_off); + case KVM_REG_RISCV_SBI_FWFT: + return sbi_fwft_id_to_str(reg_off); }
return strdup_printf("%lld | %lld /* UNKNOWN */", reg_subtype, reg_off); @@ -859,6 +874,14 @@ static __u64 sbi_sta_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_STA | KVM_REG_RISCV_SBI_STA_REG(shmem_hi), };
+static __u64 sbi_fwft_regs[] = { + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_FWFT, + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.flags), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(misaligned_deleg.value), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.flags), + KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_STATE | KVM_REG_RISCV_SBI_FWFT | KVM_REG_RISCV_SBI_FWFT_REG(pointer_masking.value), +}; + static __u64 zicbom_regs[] = { KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size), KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM, @@ -1010,6 +1033,9 @@ static __u64 vector_regs[] = { #define SUBLIST_SBI_STA \ {"sbi-sta", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_STA, \ .regs = sbi_sta_regs, .regs_n = ARRAY_SIZE(sbi_sta_regs),} +#define SUBLIST_SBI_FWFT \ + {"sbi-fwft", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_FWFT, \ + .regs = sbi_fwft_regs, .regs_n = ARRAY_SIZE(sbi_fwft_regs),} #define SUBLIST_ZICBOM \ {"zicbom", .feature = KVM_RISCV_ISA_EXT_ZICBOM, .regs = zicbom_regs, .regs_n = ARRAY_SIZE(zicbom_regs),} #define SUBLIST_ZICBOZ \ @@ -1092,6 +1118,7 @@ KVM_SBI_EXT_SUBLIST_CONFIG(sta, STA); KVM_SBI_EXT_SIMPLE_CONFIG(pmu, PMU); KVM_SBI_EXT_SIMPLE_CONFIG(dbcn, DBCN); KVM_SBI_EXT_SIMPLE_CONFIG(susp, SUSP); +KVM_SBI_EXT_SUBLIST_CONFIG(fwft, FWFT);
KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA); KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F); @@ -1167,6 +1194,7 @@ struct vcpu_reg_list *vcpu_configs[] = { &config_sbi_pmu, &config_sbi_dbcn, &config_sbi_susp, + &config_sbi_fwft, &config_aia, &config_fp_f, &config_fp_d,
2025-08-14T21:25:42+05:30, Anup Patel apatel@ventanamicro.com:
This series adds ONE_REG interface for SBI FWFT extension implemented by KVM RISC-V.
I think it would be better to ONE_REG the CSRs (medeleg/menvcfg), or at least expose their CSR fields (each sensible medeleg bit, PMM, ...) through kvm_riscv_config, than to couple this with SBI/FWFT.
The controlled behavior is defined by the ISA, and userspace might want to configure the S-mode execution environment even when SBI/FWFT is not present, which is not possible with the current design.
Is there a benefit in expressing the ISA model through SBI/FWFT?
Thanks.
linux-kselftest-mirror@lists.linaro.org