Re: [PATCH v8 03/13] KVM: s390: Fix kvm_s390_vcpu_unsetup_cmma()

[Janosch Frank]

On 8/3/26 14:40, Claudio Imbrenda wrote:
> In some cases kvm_s390_vcpu_unsetup_cmma() can be called with a 0
> cbrlo; in such cases, if running with V != R, free_page() will attempt
> to free physical page 0.

Secure guests without cmma in the unsecure guest?

> Fix by freeing cbrlo only if it's non-zero.
>
> Fixes: b31605c12f4e ("KVM: s390: make cmma usage conditionally")
> Fixes: 29b40f105ec8 ("KVM: s390: protvirt: Add initial vm and cpu lifecycle handling")
> Signed-off-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>
> ---
>   arch/s390/kvm/kvm-s390.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 1b3290a5ad1a..9be27db0a21e 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -3678,7 +3678,8 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
>   
>   void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
>   {
> -	free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
> +	if (vcpu->arch.sie_block->cbrlo)
> +		free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
>   	vcpu->arch.sie_block->cbrlo = 0;
>   }
>   

This is fenced for destroy cpu, but not for the transition into secure 
mode. Can we check "use_cmma" instead of cbrlo and then remove the 
fencing in the cpu destroy path?

If cmma is not set-up in vcpu_create, then we exit vcpu creation.
Maybe sprinkle in a warn on for !cbrlo