On 09/01/2018 10:14 AM, David Hildenbrand wrote:
>> int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
>> {
>> int code = kvm_s390_get_base_disp_rs(vcpu, NULL) & 0xffff;
>> @@ -254,6 +268,8 @@ int kvm_s390_handle_diag(struct kvm_vcpu *vcpu)
>> return __diag_page_ref_service(vcpu);
>> case 0x308:
>> return __diag_ipl_functions(vcpu);
>> + case 0x318:
>> + return __diag_set_control_prog_name(vcpu);
>> case 0x500:
>> return __diag_virtio_hypercall(vcpu);
>> default:
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 91ad4a9..678c9cb 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -156,6 +156,7 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
>> { "instruction_diag_9c", VCPU_STAT(diagnose_9c) },
>> { "instruction_diag_258", VCPU_STAT(diagnose_258) },
>> { "instruction_diag_308", VCPU_STAT(diagnose_308) },
>> + { "instruction_diag_318", VCPU_STAT(diagnose_318) },
>> { "instruction_diag_500", VCPU_STAT(diagnose_500) },
>> { "instruction_diag_other", VCPU_STAT(diagnose_other) },
>> { NULL }
>> @@ -370,6 +371,10 @@ static void kvm_s390_cpu_feat_init(void)
>>
>> if (MACHINE_HAS_ESOP)
>> allow_cpu_feat(KVM_S390_VM_CPU_FEAT_ESOP);
>> +
>> + /* Always allow diag318 for a guest */
>> + allow_cpu_feat(KVM_S390_VM_CPU_FEAT_DIAG318);
>
> So DIAG 318 emulation can always be provided. But how are we to handle
> if we have SIE support for CPNC or not? Shouldn't that also being taken
> into account?
>
> When do we have SIE support? Does that come with DIAG318 itself? How to
> check?>
> Also, I guess we should not allow access to the cnpc value if we don't
> have SIE support (read+write).
>
You certainly make a good point.
In a guest running without SIE, execution of DIAG 318 /should/ result in a no-op
(I'll run some tests to see if this is truly harmless or not).
>> +
>> /*
>> * We need SIE support, ESOP (PROT_READ protection for gmap_shadow),
>> * 64bit SCAO (SCA passthrough) and IDTE (for gmap_shadow unshadowing).
>> @@ -1140,6 +1145,72 @@ static int kvm_s390_get_tod(struct kvm *kvm, struct kvm_device_attr *attr)
>> return ret;
>> }
>>
>> +void kvm_s390_set_cpc(struct kvm *kvm, u64 cpc)
>> +{
>> + struct kvm_vcpu *vcpu;
>> + int i;
>> +
>> + mutex_lock(&kvm->lock);
>> + kvm->arch.cpnc = cpc >> 56;
>> + kvm->arch.cpvc = cpc & 0x00ffffffffffffffUL;
>> +
>> + VM_EVENT(kvm, 3, "SET: CPNC: 0x%x CPVC: 0x%llx",
>> + kvm->arch.cpnc, kvm->arch.cpvc);
>> +
>> + kvm_for_each_vcpu(i, vcpu, kvm) {
>> + vcpu->arch.sie_block->cpnc = kvm->arch.cpnc;
>> + }
>
> I guess the right thing to do is a synchronous all-VCPU request. Let
> them then update it themselves when handling the request. (this can
> effectively be called while other VCPUs are already executing)
>
> As an alternative, block/unblock all VCPUs, but I guess a request is the
> easiest and cleanest way.
>
My apologies, but I found the above rather confusing. You mention a "synchronous all-VCPU
request" that each VCPU can handle and update the cpnc? I am unfamiliar with such a request...
The latter suggestion with blocking/unblocking sounds easier to implement, and is what
other get/set functions do (TOD-Clock, crypto). Please enlighten me on the former.
>> + mutex_unlock(&kvm->lock);
>> +}
>> +
>> +static int kvm_s390_set_misc(struct kvm *kvm, struct kvm_device_attr *attr)
>> +{
>> + int ret;
>> + u64 cpc;
>> +
>> + switch (attr->attr) {
>> + case KVM_S390_VM_MISC_CPC:
>> + ret = -EFAULT;
>> + if (get_user(cpc, (u64 __user *)attr->addr))
>> + break;
>> + kvm_s390_set_cpc(kvm, cpc);
>> + ret = 0;
>> + break;
>> + default:
>> + ret = -ENXIO;
>> + break;
>> + }
>> + return ret;
>> +}
>
>
>
Thanks for the review!
--
Respectfully,
- Collin Walling
