Re: [PATCH] KVM: s390: Beautify skey enable check

[David Hildenbrand <david@xxxxxxxxxx>]

On 20.07.2018 15:10, Janosch Frank wrote:
> Let's introduce an explicit check if skeys have already been enabled
> for the vcpu, so we don't have to check the mm context if we don't
> have the storage key facility.
> 
> This let's us check for enablement without having to take the mm
> semaphore and thus speedup skey emulation.
> 
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>
> ---
>  arch/s390/include/asm/kvm_host.h | 1 +
>  arch/s390/kvm/priv.c             | 5 ++---
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
> index a2188e309bd6..2916f0a5585c 100644
> --- a/arch/s390/include/asm/kvm_host.h
> +++ b/arch/s390/include/asm/kvm_host.h
> @@ -655,6 +655,7 @@ struct kvm_vcpu_arch {
>  	seqcount_t cputm_seqcount;
>  	__u64 cputm_start;
>  	bool gs_enabled;
> +	bool skey_enabled;
>  };
>  
>  struct kvm_vm_stat {
> diff --git a/arch/s390/kvm/priv.c b/arch/s390/kvm/priv.c
> index 83c678266588..e78c381c8a24 100644
> --- a/arch/s390/kvm/priv.c
> +++ b/arch/s390/kvm/priv.c
> @@ -209,9 +209,7 @@ int kvm_s390_skey_check_enable(struct kvm_vcpu *vcpu)
>  
>  	trace_kvm_s390_skey_related_inst(vcpu);
>  	/* Already enabled? */
> -	if (vcpu->kvm->arch.use_skf &&
> -	    !(sie_block->ictl & (ICTL_ISKE | ICTL_SSKE | ICTL_RRBE)) &&
> -	    !kvm_s390_test_cpuflags(vcpu, CPUSTAT_KSS))
> +	if (vcpu->arch.skey_enabled)
>  		return 0;
>  
>  	rc = s390_enable_skey();
> @@ -225,6 +223,7 @@ int kvm_s390_skey_check_enable(struct kvm_vcpu *vcpu)
>  		sie_block->ictl |= ICTL_ISKE | ICTL_SSKE | ICTL_RRBE;
>  	else
>  		sie_block->ictl &= ~(ICTL_ISKE | ICTL_SSKE | ICTL_RRBE);
> +	vcpu->arch.skey_enabled = true;
>  	return 0;
>  }
>  
> 

I never liked this part of the code. You might no be able to go ahead
and turn the

if (!vcpu->kvm->arch.use_skf)
    sie_block->ictl |= ICTL_ISKE | ICTL_SSKE | ICTL_RRBE;
else
    sie_block->ictl &= ~(ICTL_ISKE | ICTL_SSKE | ICTL_RRBE);

into a

if (vcpu->kvm->arch.use_skf)

    sie_block->ictl &= ~(ICTL_ISKE | ICTL_SSKE | ICTL_RRBE);

-- 

Thanks,

David / dhildenb