On 17.04.19 17:47, David Hildenbrand wrote:
> On 17.04.19 17:28, Christian Borntraeger wrote:
>> Instead of adding a new machine option to disable/enable the keywrapping
>> options of pckmo (like for AES and DEA) we can now use the CPU model to
>> decide.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> Reviewed-by: Collin Walling <walling@xxxxxxxxxxxxx>
>> ---
>> arch/s390/include/asm/kvm_host.h | 1 +
>> arch/s390/kvm/kvm-s390.c | 4 ++++
>> 2 files changed, 5 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
>> index c47e22bba87f..e224246ff93c 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -278,6 +278,7 @@ struct kvm_s390_sie_block {
>> #define ECD_HOSTREGMGMT 0x20000000
>> #define ECD_MEF 0x08000000
>> #define ECD_ETOKENF 0x02000000
>> +#define ECD_ECC 0x00200000
>> __u32 ecd; /* 0x01c8 */
>> __u8 reserved1cc[18]; /* 0x01cc */
>> __u64 pp; /* 0x01de */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 0dad61ccde3d..ff2444d935fd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2933,6 +2933,10 @@ int kvm_arch_vcpu_setup(struct kvm_vcpu *vcpu)
>> VCPU_EVENT(vcpu, 3, "AIV gisa format-%u enabled for cpu %03u",
>> vcpu->arch.sie_block->gd & 0x3, vcpu->vcpu_id);
>> }
>> + /* if any of 32,33,34,40,41 is active, enable pckmo for ecc */
>> + if ((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
>> + (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0))
>> + vcpu->arch.sie_block->ecd |= ECD_ECC;
>> vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
>> | SDNXC;
>> vcpu->arch.sie_block->riccbd = (unsigned long) &vcpu->run->s.regs.riccb;
>>
> Just noting here that user space can set pckmo and friends even though
> HW does not support it (if I remember correctly), resulting in ECD_ECC
> being able to be set from user space although not supported. Is that
> sane? Shouldn't we somehow check if the HW actually supports it?
>
> (features should only be used if the indication is on, not because it
> "might" work)
For those machines ECD_ECC will be ignored, but yes, adding an additional check
that we have these in the real machine might be safer.
