Message ID | 20250106154847.1100344-4-cleger@rivosinc.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | riscv: add SBI FWFT misaligned exception delegation support | expand |
Hi Clément, On 2025-01-06 9:48 AM, Clément Léger wrote: > The FWFT SBI extension will need to dynamically allocate memory and do > init time specific initialization. Add an init/deinit callbacks that > allows to do so. > > Signed-off-by: Clément Léger <cleger@rivosinc.com> > --- > arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 ++++++++ > arch/riscv/kvm/vcpu.c | 2 ++ > arch/riscv/kvm/vcpu_sbi.c | 31 ++++++++++++++++++++++++++- > 3 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h > index b96705258cf9..8c465ce90e73 100644 > --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h > +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h > @@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension { > > /* Extension specific probe function */ > unsigned long (*probe)(struct kvm_vcpu *vcpu); > + > + /* > + * Init/deinit function called once during VCPU init/destroy. These > + * might be use if the SBI extensions need to allocate or do specific > + * init time only configuration. > + */ > + int (*init)(struct kvm_vcpu *vcpu); > + void (*deinit)(struct kvm_vcpu *vcpu); > }; > > void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); > @@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( > 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); > > int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num, > unsigned long *reg_val); > diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c > index e048dcc6e65e..3420a4a62c94 100644 > --- a/arch/riscv/kvm/vcpu.c > +++ b/arch/riscv/kvm/vcpu.c > @@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) > > void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) > { > + kvm_riscv_vcpu_sbi_deinit(vcpu); > + > /* Cleanup VCPU AIA context */ > kvm_riscv_vcpu_aia_deinit(vcpu); > > diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c > index 6e704ed86a83..d2dbb0762072 100644 > --- a/arch/riscv/kvm/vcpu_sbi.c > +++ b/arch/riscv/kvm/vcpu_sbi.c > @@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) > 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 idx, i; > + int idx, i, ret; > > for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { > entry = &sbi_ext[i]; > @@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) > continue; > } > > + if (ext->init) { > + ret = ext->init(vcpu); > + if (ret) > + scontext->ext_status[idx] = > + KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE; > + } > + > scontext->ext_status[idx] = ext->default_disabled ? > KVM_RISCV_SBI_EXT_STATUS_DISABLED : > KVM_RISCV_SBI_EXT_STATUS_ENABLED; This will overwrite the KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE set above. > } > } > + > +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu) > +{ > + 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 idx, i; > + > + for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { > + entry = &sbi_ext[i]; > + ext = entry->ext_ptr; > + idx = entry->ext_idx; > + > + if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status)) > + continue; > + > + if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit) Given that an extension can be enabled/disabled after initialization, this should only skip deinit if the status is UNAVAILABLE. Regards, Samuel > + continue; > + > + ext->deinit(vcpu); > + } > +}
On 11/01/2025 00:42, Samuel Holland wrote: > Hi Clément, > > On 2025-01-06 9:48 AM, Clément Léger wrote: >> The FWFT SBI extension will need to dynamically allocate memory and do >> init time specific initialization. Add an init/deinit callbacks that >> allows to do so. >> >> Signed-off-by: Clément Léger <cleger@rivosinc.com> >> --- >> arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 ++++++++ >> arch/riscv/kvm/vcpu.c | 2 ++ >> arch/riscv/kvm/vcpu_sbi.c | 31 ++++++++++++++++++++++++++- >> 3 files changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h >> index b96705258cf9..8c465ce90e73 100644 >> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h >> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h >> @@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension { >> >> /* Extension specific probe function */ >> unsigned long (*probe)(struct kvm_vcpu *vcpu); >> + >> + /* >> + * Init/deinit function called once during VCPU init/destroy. These >> + * might be use if the SBI extensions need to allocate or do specific >> + * init time only configuration. >> + */ >> + int (*init)(struct kvm_vcpu *vcpu); >> + void (*deinit)(struct kvm_vcpu *vcpu); >> }; >> >> void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); >> @@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( >> 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); >> >> int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num, >> unsigned long *reg_val); >> diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c >> index e048dcc6e65e..3420a4a62c94 100644 >> --- a/arch/riscv/kvm/vcpu.c >> +++ b/arch/riscv/kvm/vcpu.c >> @@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) >> >> void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) >> { >> + kvm_riscv_vcpu_sbi_deinit(vcpu); >> + >> /* Cleanup VCPU AIA context */ >> kvm_riscv_vcpu_aia_deinit(vcpu); >> >> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c >> index 6e704ed86a83..d2dbb0762072 100644 >> --- a/arch/riscv/kvm/vcpu_sbi.c >> +++ b/arch/riscv/kvm/vcpu_sbi.c >> @@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) >> 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 idx, i; >> + int idx, i, ret; >> >> for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { >> entry = &sbi_ext[i]; >> @@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) >> continue; >> } >> >> + if (ext->init) { >> + ret = ext->init(vcpu); >> + if (ret) >> + scontext->ext_status[idx] = >> + KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE; >> + } >> + >> scontext->ext_status[idx] = ext->default_disabled ? >> KVM_RISCV_SBI_EXT_STATUS_DISABLED : >> KVM_RISCV_SBI_EXT_STATUS_ENABLED; > > This will overwrite the KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE set above. Oh yes indeed, I should add a "continue" under the if above. > >> } >> } >> + >> +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu) >> +{ >> + 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 idx, i; >> + >> + for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { >> + entry = &sbi_ext[i]; >> + ext = entry->ext_ptr; >> + idx = entry->ext_idx; >> + >> + if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status)) >> + continue; >> + >> + if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit) > > Given that an extension can be enabled/disabled after initialization, this > should only skip deinit if the status is UNAVAILABLE. yeah, make sense as well, Thanks for the review, Clément > > Regards, > Samuel > >> + continue; >> + >> + ext->deinit(vcpu); >> + } >> +} >
diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h index b96705258cf9..8c465ce90e73 100644 --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h @@ -49,6 +49,14 @@ struct kvm_vcpu_sbi_extension { /* Extension specific probe function */ unsigned long (*probe)(struct kvm_vcpu *vcpu); + + /* + * Init/deinit function called once during VCPU init/destroy. These + * might be use if the SBI extensions need to allocate or do specific + * init time only configuration. + */ + int (*init)(struct kvm_vcpu *vcpu); + void (*deinit)(struct kvm_vcpu *vcpu); }; void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run); @@ -69,6 +77,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext( 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); int kvm_riscv_vcpu_get_reg_sbi_sta(struct kvm_vcpu *vcpu, unsigned long reg_num, unsigned long *reg_val); diff --git a/arch/riscv/kvm/vcpu.c b/arch/riscv/kvm/vcpu.c index e048dcc6e65e..3420a4a62c94 100644 --- a/arch/riscv/kvm/vcpu.c +++ b/arch/riscv/kvm/vcpu.c @@ -180,6 +180,8 @@ void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu) void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu) { + kvm_riscv_vcpu_sbi_deinit(vcpu); + /* Cleanup VCPU AIA context */ kvm_riscv_vcpu_aia_deinit(vcpu); diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c index 6e704ed86a83..d2dbb0762072 100644 --- a/arch/riscv/kvm/vcpu_sbi.c +++ b/arch/riscv/kvm/vcpu_sbi.c @@ -486,7 +486,7 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) 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 idx, i; + int idx, i, ret; for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { entry = &sbi_ext[i]; @@ -501,8 +501,37 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu) continue; } + if (ext->init) { + ret = ext->init(vcpu); + if (ret) + scontext->ext_status[idx] = + KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE; + } + scontext->ext_status[idx] = ext->default_disabled ? KVM_RISCV_SBI_EXT_STATUS_DISABLED : KVM_RISCV_SBI_EXT_STATUS_ENABLED; } } + +void kvm_riscv_vcpu_sbi_deinit(struct kvm_vcpu *vcpu) +{ + 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 idx, i; + + for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) { + entry = &sbi_ext[i]; + ext = entry->ext_ptr; + idx = entry->ext_idx; + + if (idx < 0 || idx >= ARRAY_SIZE(scontext->ext_status)) + continue; + + if (scontext->ext_status[idx] != KVM_RISCV_SBI_EXT_STATUS_ENABLED || !ext->deinit) + continue; + + ext->deinit(vcpu); + } +}
The FWFT SBI extension will need to dynamically allocate memory and do init time specific initialization. Add an init/deinit callbacks that allows to do so. Signed-off-by: Clément Léger <cleger@rivosinc.com> --- arch/riscv/include/asm/kvm_vcpu_sbi.h | 9 ++++++++ arch/riscv/kvm/vcpu.c | 2 ++ arch/riscv/kvm/vcpu_sbi.c | 31 ++++++++++++++++++++++++++- 3 files changed, 41 insertions(+), 1 deletion(-)