Re: [PATCH 5/5] arm/arm64: KVM: Don't panic on failure to properly reset system registers

[Andrew Jones <drjones@xxxxxxxxxx>]

On Fri, Jan 25, 2019 at 10:46:56AM +0100, Christoffer Dall wrote:
> From: Marc Zyngier <marc.zyngier@xxxxxxx>
> 
> Failing to properly reset system registers is pretty bad. But not
> quite as bad as bringing the whole machine down... So warn loudly,
> but slightly more gracefully.
> 
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> Acked-by: Christoffer Dall <christoffer.dall@xxxxxxx>
> ---
>  arch/arm/kvm/coproc.c     | 4 ++--
>  arch/arm64/kvm/sys_regs.c | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/kvm/coproc.c b/arch/arm/kvm/coproc.c
> index 222c1635bc7a..e8bd288fd5be 100644
> --- a/arch/arm/kvm/coproc.c
> +++ b/arch/arm/kvm/coproc.c
> @@ -1450,6 +1450,6 @@ void kvm_reset_coprocs(struct kvm_vcpu *vcpu)
>  	reset_coproc_regs(vcpu, table, num);
>  
>  	for (num = 1; num < NR_CP15_REGS; num++)
> -		if (vcpu_cp15(vcpu, num) == 0x42424242)
> -			panic("Didn't reset vcpu_cp15(vcpu, %zi)", num);
> +		WARN(vcpu_cp15(vcpu, num) == 0x42424242,
> +		     "Didn't reset vcpu_cp15(vcpu, %zi)", num);
>  }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 86096774abcd..4f067545c7d2 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2609,6 +2609,6 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu)
>  	reset_sys_reg_descs(vcpu, table, num);
>  
>  	for (num = 1; num < NR_SYS_REGS; num++)
> -		if (__vcpu_sys_reg(vcpu, num) == 0x4242424242424242)
> -			panic("Didn't reset __vcpu_sys_reg(%zi)", num);
> +		WARN(__vcpu_sys_reg(vcpu, num) == 0x4242424242424242,
> +		     "Didn't reset __vcpu_sys_reg(%zi)\n", num);
>  }
> -- 
> 2.18.0
>

If we only get halfway through resetting, then we'll get a warn splat,
complete with a backtrace, for each register. Should we do something
like the following instead?

  for (num = 1; num < NR_SYS_REGS; num++)
     if (__vcpu_sys_reg(vcpu, num) == 0x4242424242424242)
        failed++;
  WARN(failed, "Didn't reset %d system registers", failed);

Thanks,
drew