On 20.07.2018 15:57, David Hildenbrand wrote:
> 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);
>
Oh and if you touch it, please remove the nasty local variable "sie_block".
--
Thanks,
David / dhildenb
