On 04.02.20 13:13, David Hildenbrand wrote:
> [...]
>
>> +#ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
>> +static int kvm_s390_handle_pv(struct kvm *kvm, struct kvm_pv_cmd *cmd)
>> +{
>> + int r = 0;
>> + void __user *argp = (void __user *)cmd->data;
>> +
>> + switch (cmd->cmd) {
>> + case KVM_PV_VM_CREATE: {
>> + r = -EINVAL;
>> + if (kvm_s390_pv_is_protected(kvm))
>> + break;
>> +
>> + r = kvm_s390_pv_alloc_vm(kvm);
>> + if (r)
>> + break;
>> +
>> + mutex_lock(&kvm->lock);
>> + kvm_s390_vcpu_block_all(kvm);
>> + /* FMT 4 SIE needs esca */
>> + r = sca_switch_to_extended(kvm);
>> + if (!r)
>> + r = kvm_s390_pv_create_vm(kvm);
>> + kvm_s390_vcpu_unblock_all(kvm);
>> + mutex_unlock(&kvm->lock);
>> + break;
>> + }
>
> I think KVM_PV_VM_ENABLE/KVM_PV_VM_DISABLE would be a better fit. You're
> not creating/deleting VMs, aren't you? All you're doing is allocating
> some data and performing some kind of a mode switch.
I kind of like the idea. Need to talk to Janosch about this.
> [...]
>
>> VM_EVENT(kvm, 3, "create cpu %d at 0x%pK, sie block at 0x%pK", id, vcpu,
>> vcpu->arch.sie_block);
>> trace_kvm_s390_create_vcpu(id, vcpu, vcpu->arch.sie_block);
>> @@ -4353,6 +4502,37 @@ long kvm_arch_vcpu_async_ioctl(struct file *filp,
>> return -ENOIOCTLCMD;
>> }
>>
>> +#ifdef CONFIG_KVM_S390_PROTECTED_VIRTUALIZATION_HOST
>> +static int kvm_s390_handle_pv_vcpu(struct kvm_vcpu *vcpu,
>> + struct kvm_pv_cmd *cmd)
>> +{
>> + int r = 0;
>> +
>> + if (!kvm_s390_pv_is_protected(vcpu->kvm))
>> + return -EINVAL;
>> +
>> + switch (cmd->cmd) {
>> + case KVM_PV_VCPU_CREATE: {
>> + if (kvm_s390_pv_handle_cpu(vcpu))
>> + return -EINVAL;
>> +
>> + r = kvm_s390_pv_create_cpu(vcpu);
>> + break;
>> + }
>> + case KVM_PV_VCPU_DESTROY: {
>> + if (!kvm_s390_pv_handle_cpu(vcpu))
>> + return -EINVAL;
>> +
>> + r = kvm_s390_pv_destroy_cpu(vcpu);
>> + break;
>> + }
>> + default:
>> + r = -ENOTTY;
>> + }
>> + return r;
>> +}
>
> I asked this already and didn't get an answer (lost in the flood of
> comments :) )
>
> Can't we simply convert all VCPUs via KVM_PV_VM_CREATE and destoy them
> via KVM_PV_VM_DESTROY? Then you can easily handle hotplug as well in the
> kernel when a new VCPU is created and PV is active - oh and I see you
> are already doing that in kvm_arch_vcpu_create(). So that screams for
> doing either this a) completely triggered by user space or b) completely
> in the kernel. I prefer the latter. One interface less required.
>
> I would assume that no VCPU is allowed to be running inside KVM while
right.
> performing the PV switch, which would make this even easier.
Same as above. I like the idea. Will need to talk to Janosch.
