[PATCH 1/3] RISC-V: KVM: Disable SBI extension when its probe fails

[Anup Patel]

On Wed, Apr 26, 2023 at 10:43 PM Andrew Jones <ajones at ventanamicro.com> wrote:
>
> When an SBI extension specific probe function exists and fails, then
> use the extension_disabled context to disable the extension. This
> ensures the extension's functions cannot be invoked. Doing the
> disabling in kvm_vcpu_sbi_find_ext() allows it to be done lazily
> on its first use. Checking extension_disabled prior to probing
> ensures the probe is only executed once for disabled extensions.
> Later patches ensure we only execute probe once for enabled
> extensions as well.
>
> Signed-off-by: Andrew Jones <ajones at ventanamicro.com>
> ---
>  arch/riscv/kvm/vcpu_sbi.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index e52fde504433..aa3c126d2e3c 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -307,18 +307,25 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
>                                 struct kvm_vcpu *vcpu, unsigned long extid)
>  {
> -       int i;
> -       const struct kvm_riscv_sbi_extension_entry *sext;
>         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++) {
> -               sext = &sbi_ext[i];
> -               if (sext->ext_ptr->extid_start <= extid &&
> -                   sext->ext_ptr->extid_end >= extid) {
> -                       if (sext->dis_idx < KVM_RISCV_SBI_EXT_MAX &&
> -                           scontext->extension_disabled[sext->dis_idx])
> +               entry = &sbi_ext[i];
> +               ext = entry->ext_ptr;
> +
> +               if (ext->extid_start <= extid && ext->extid_end >= extid) {
> +                       if (entry->dis_idx >= KVM_RISCV_SBI_EXT_MAX)
> +                               return ext;
> +                       if (scontext->extension_disabled[entry->dis_idx])
> +                               return NULL;
> +                       if (ext->probe && !ext->probe(vcpu)) {

Calling probe() upon every kvm_vcpu_sbi_find_ext() will simply slow down
all SBI calls.

How about caching probe return values in "struct kvm_vcpu_sbi_context"
as an array?

Maybe we can have two arrays:
1) probe_done[] : Boolean array to check whether probe was done.
2) probe_retval[] : Cached probe() return values.

The SBI_EXT_BASE_PROBE_EXT call should also return the
cached value.

> +                               scontext->extension_disabled[entry->dis_idx] = true;
>                                 return NULL;
> -                       return sbi_ext[i].ext_ptr;
> +                       }
> +                       return ext;
>                 }
>         }
>
> --
> 2.39.2
>

Regards,
Anup