Re: [PATCH v2 1/1] target/riscv/kvm: tolerate KVM disable ext errors

[Andrew Jones <ajones@ventanamicro.com>]

On Mon, Apr 22, 2024 at 02:14:25PM GMT, Daniel Henrique Barboza wrote:
> Running a KVM guest using a 6.9-rc3 kernel, in a 6.8 host that has zkr
> enabled, will fail with a kernel oops SIGILL right at the start. The
> reason is that we can't expose zkr without implementing the SEED CSR.
> Disabling zkr in the guest would be a workaround, but if the KVM doesn't
> allow it we'll error out and never boot.
> 
> In hindsight this is too strict. If we keep proceeding, despite not
> disabling the extension in the KVM vcpu, we'll not add the extension in
> the riscv,isa. The guest kernel will be unaware of the extension, i.e.
> it doesn't matter if the KVM vcpu has it enabled underneath or not. So
> it's ok to keep booting in this case.
> 
> Change our current logic to not error out if we fail to disable an
> extension in kvm_set_one_reg(), but show a warning and keep booting. It
> is important to throw a warning because we must make the user aware that
> the extension is still available in the vcpu, meaning that an
> ill-behaved guest can ignore the riscv,isa settings and  use the
> extension.
> 
> The case we're handling happens with an EINVAL error code. If we fail to
> disable the extension in KVM for any other reason, error out.
> 
> We'll also keep erroring out when we fail to enable an extension in KVM,
> since adding the extension in riscv,isa at this point will cause a guest
> malfunction because the extension isn't enabled in the vcpu.
> 
> Suggested-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
> ---
>  target/riscv/kvm/kvm-cpu.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
> index 6a6c6cae80..03e3fee607 100644
> --- a/target/riscv/kvm/kvm-cpu.c
> +++ b/target/riscv/kvm/kvm-cpu.c
> @@ -427,10 +427,14 @@ static void kvm_riscv_update_cpu_cfg_isa_ext(RISCVCPU *cpu, CPUState *cs)
>          reg = kvm_cpu_cfg_get(cpu, multi_ext_cfg);
>          ret = kvm_set_one_reg(cs, id, &reg);
>          if (ret != 0) {
> -            error_report("Unable to %s extension %s in KVM, error %d",
> -                         reg ? "enable" : "disable",
> -                         multi_ext_cfg->name, ret);
> -            exit(EXIT_FAILURE);
> +            if (!reg && ret == -EINVAL) {
> +                warn_report("KVM cannot disable extension %s",
> +                            multi_ext_cfg->name);
> +            } else {
> +                error_report("Unable to enable extension %s in KVM, error %d",
> +                             multi_ext_cfg->name, ret);
> +                exit(EXIT_FAILURE);
> +            }
>          }
>      }
>  }
> -- 
> 2.44.0
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>