Messages in this thread | | | Subject | Re: [PATCH v5 06/13] KVM: s390: interfaces to manage guest's AP matrix | From | Halil Pasic <> | Date | Fri, 11 May 2018 18:08:02 +0200 |
| |
On 05/07/2018 05:11 PM, Tony Krowiak wrote: > Provides interfaces to manage the AP adapters, usage domains > and control domains assigned to a KVM guest. > > The guest's SIE state description has a satellite structure called the > Crypto Control Block (CRYCB) containing three bitmask fields > identifying the adapters, queues (domains) and control domains > assigned to the KVM guest:
[..]
> index 00bcfb0..98b53c7 100644 > --- a/arch/s390/kvm/kvm-ap.c > +++ b/arch/s390/kvm/kvm-ap.c > @@ -7,6 +7,7 @@
[..]
> + > +/** > + * kvm_ap_validate_queue_sharing > + * > + * Verifies that the APQNs derived from the cross product of the AP adapter IDs > + * and AP queue indexes comprising the AP matrix are not configured for > + * another guest. AP queue sharing is not allowed. > + * > + * @kvm: the KVM guest > + * @matrix: the AP matrix > + * > + * Returns 0 if the APQNs are valid, otherwise; returns -EBUSY. > + */ > +static int kvm_ap_validate_queue_sharing(struct kvm *kvm, > + struct kvm_ap_matrix *matrix) > +{ > + struct kvm *vm; > + unsigned long *apm, *aqm; > + unsigned long apid, apqi; > + > + > + /* No other VM may share an AP Queue with the input VM */ > + list_for_each_entry(vm, &vm_list, vm_list) { > + if (kvm == vm) > + continue; > + > + apm = kvm_ap_get_crycb_apm(vm); > + if (!bitmap_and(apm, apm, matrix->apm, matrix->apm_max + 1)) > + continue; > + > + aqm = kvm_ap_get_crycb_aqm(vm); > + if (!bitmap_and(aqm, aqm, matrix->aqm, matrix->aqm_max + 1)) > + continue; > + > + for_each_set_bit_inv(apid, apm, matrix->apm_max + 1) > + for_each_set_bit_inv(apqi, aqm, matrix->aqm_max + 1) > + kvm_ap_log_sharing_err(vm, apid, apqi); > + > + return -EBUSY; > + } > + > + return 0; > +} > + > +int kvm_ap_configure_matrix(struct kvm *kvm, struct kvm_ap_matrix *matrix) > +{ > + int ret = 0; > + > + mutex_lock(&kvm->lock);
You seem to take only kvm->lock, vm_list however (used in kvm_ap_validate_queue_sharing()) seems to be protected by kvm_lock.
Can you tell me why is this supposed to be safe?
What is supposed to prevent an execution like vm1: call kvm_ap_configure_matrix(m1) vm2: call kvm_ap_configure_matrix(m2) vm1: call kvm_ap_validate_queue_sharing(m1) vm2: call kvm_ap_validate_queue_sharing(m2) vm1: call kvm_ap_set_crycb_masks(m1) vm2: call kvm_ap_set_crycb_masks(m2)
where, let's say, m1 and m2 are equal in the sense that the mask values are the same?
Regards, Halil
> + > + ret = kvm_ap_validate_queue_sharing(kvm, matrix); > + if (ret) > + goto done; > + > + kvm_ap_set_crycb_masks(kvm, matrix); > + > +done: > + mutex_unlock(&kvm->lock); > + > + return ret; > +} > +EXPORT_SYMBOL(kvm_ap_configure_matrix); > +
| |