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?
untested patch, something like this on top
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 7dae78ccefc6..dadd1105b5a4 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2985,9 +2985,14 @@ 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))
+ /*
+ * if any of 32,33,34,40,41 is active in host AND guest,
+ * we enable pckmo for ecc
+ */
+ if (((vcpu->kvm->arch.model.subfuncs.pckmo[4] & 0xe0) ||
+ (vcpu->kvm->arch.model.subfuncs.pckmo[5] & 0xc0)) &&
+ ((kvm_s390_available_subfunc.pckmo[4] & 0xe0) ||
+ (kvm_s390_available_subfunc.pckmo[5] & 0xc0)))
vcpu->arch.sie_block->ecd |= ECD_ECC;
vcpu->arch.sie_block->sdnxo = ((unsigned long) &vcpu->run->s.regs.sdnx)
| SDNXC;
>
> (features should only be used if the indication is on, not because it
> "might" work)
>
