Skip to content

Commit 384bb63

Browse files
author
Tobias Huschle
committed
KVM: s390: pv: don't allow userspace to set the clock under PV
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2154283 Upstream status: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git Tested: by IBM Build-Info: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=49687044 Conflicts: N commit 6973091 Author: Nico Boehr <nrb@linux.ibm.com> Date: Tue Oct 11 18:07:12 2022 +0200 KVM: s390: pv: don't allow userspace to set the clock under PV When running under PV, the guest's TOD clock is under control of the ultravisor and the hypervisor isn't allowed to change it. Hence, don't allow userspace to change the guest's TOD clock by returning -EOPNOTSUPP. When userspace changes the guest's TOD clock, KVM updates its kvm.arch.epoch field and, in addition, the epoch field in all state descriptions of all VCPUs. But, under PV, the ultravisor will ignore the epoch field in the state description and simply overwrite it on next SIE exit with the actual guest epoch. This leads to KVM having an incorrect view of the guest's TOD clock: it has updated its internal kvm.arch.epoch field, but the ultravisor ignores the field in the state description. Whenever a guest is now waiting for a clock comparator, KVM will incorrectly calculate the time when the guest should wake up, possibly causing the guest to sleep for much longer than expected. With this change, kvm_s390_set_tod() will now take the kvm->lock to be able to call kvm_s390_pv_is_protected(). Since kvm_s390_set_tod_clock() also takes kvm->lock, use __kvm_s390_set_tod_clock() instead. The function kvm_s390_set_tod_clock is now unused, hence remove it. Update the documentation to indicate the TOD clock attr calls can now return -EOPNOTSUPP. Fixes: 0f30350 ("KVM: s390: protvirt: Do only reset registers that are accessible") Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> Signed-off-by: Nico Boehr <nrb@linux.ibm.com> Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> Link: https://lore.kernel.org/r/20221011160712.928239-2-nrb@linux.ibm.com Message-Id: <20221011160712.928239-2-nrb@linux.ibm.com> Signed-off-by: Janosch Frank <frankja@linux.ibm.com> Signed-off-by: Tobias Huschle <thuschle@redhat.com>
1 parent 616a974 commit 384bb63

File tree

3 files changed

+20
-10
lines changed

3 files changed

+20
-10
lines changed

Documentation/virt/kvm/devices/vm.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ KVM_S390_VM_TOD_EXT).
215215
:Parameters: address of a buffer in user space to store the data (u8) to
216216
:Returns: -EFAULT if the given address is not accessible from kernel space;
217217
-EINVAL if setting the TOD clock extension to != 0 is not supported
218+
-EOPNOTSUPP for a PV guest (TOD managed by the ultravisor)
218219

219220
3.2. ATTRIBUTE: KVM_S390_VM_TOD_LOW
220221
-----------------------------------
@@ -224,6 +225,7 @@ the POP (u64).
224225

225226
:Parameters: address of a buffer in user space to store the data (u64) to
226227
:Returns: -EFAULT if the given address is not accessible from kernel space
228+
-EOPNOTSUPP for a PV guest (TOD managed by the ultravisor)
227229

228230
3.3. ATTRIBUTE: KVM_S390_VM_TOD_EXT
229231
-----------------------------------
@@ -237,6 +239,7 @@ it, it is stored as 0 and not allowed to be set to a value != 0.
237239
(kvm_s390_vm_tod_clock) to
238240
:Returns: -EFAULT if the given address is not accessible from kernel space;
239241
-EINVAL if setting the TOD clock extension to != 0 is not supported
242+
-EOPNOTSUPP for a PV guest (TOD managed by the ultravisor)
240243

241244
4. GROUP: KVM_S390_VM_CRYPTO
242245
============================

arch/s390/kvm/kvm-s390.c

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1205,6 +1205,8 @@ static int kvm_s390_vm_get_migration(struct kvm *kvm,
12051205
return 0;
12061206
}
12071207

1208+
static void __kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod);
1209+
12081210
static int kvm_s390_set_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
12091211
{
12101212
struct kvm_s390_vm_tod_clock gtod;
@@ -1214,7 +1216,7 @@ static int kvm_s390_set_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
12141216

12151217
if (!test_kvm_facility(kvm, 139) && gtod.epoch_idx)
12161218
return -EINVAL;
1217-
kvm_s390_set_tod_clock(kvm, &gtod);
1219+
__kvm_s390_set_tod_clock(kvm, &gtod);
12181220

12191221
VM_EVENT(kvm, 3, "SET: TOD extension: 0x%x, TOD base: 0x%llx",
12201222
gtod.epoch_idx, gtod.tod);
@@ -1245,7 +1247,7 @@ static int kvm_s390_set_tod_low(struct kvm *kvm, struct kvm_device_attr *attr)
12451247
sizeof(gtod.tod)))
12461248
return -EFAULT;
12471249

1248-
kvm_s390_set_tod_clock(kvm, &gtod);
1250+
__kvm_s390_set_tod_clock(kvm, &gtod);
12491251
VM_EVENT(kvm, 3, "SET: TOD base: 0x%llx", gtod.tod);
12501252
return 0;
12511253
}
@@ -1257,6 +1259,16 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
12571259
if (attr->flags)
12581260
return -EINVAL;
12591261

1262+
mutex_lock(&kvm->lock);
1263+
/*
1264+
* For protected guests, the TOD is managed by the ultravisor, so trying
1265+
* to change it will never bring the expected results.
1266+
*/
1267+
if (kvm_s390_pv_is_protected(kvm)) {
1268+
ret = -EOPNOTSUPP;
1269+
goto out_unlock;
1270+
}
1271+
12601272
switch (attr->attr) {
12611273
case KVM_S390_VM_TOD_EXT:
12621274
ret = kvm_s390_set_tod_ext(kvm, attr);
@@ -1271,6 +1283,9 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
12711283
ret = -ENXIO;
12721284
break;
12731285
}
1286+
1287+
out_unlock:
1288+
mutex_unlock(&kvm->lock);
12741289
return ret;
12751290
}
12761291

@@ -4381,13 +4396,6 @@ static void __kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_t
43814396
preempt_enable();
43824397
}
43834398

4384-
void kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod)
4385-
{
4386-
mutex_lock(&kvm->lock);
4387-
__kvm_s390_set_tod_clock(kvm, gtod);
4388-
mutex_unlock(&kvm->lock);
4389-
}
4390-
43914399
int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod)
43924400
{
43934401
if (!mutex_trylock(&kvm->lock))

arch/s390/kvm/kvm-s390.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,6 @@ int kvm_s390_handle_sigp(struct kvm_vcpu *vcpu);
363363
int kvm_s390_handle_sigp_pei(struct kvm_vcpu *vcpu);
364364

365365
/* implemented in kvm-s390.c */
366-
void kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod);
367366
int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod);
368367
long kvm_arch_fault_in_page(struct kvm_vcpu *vcpu, gpa_t gpa, int writable);
369368
int kvm_s390_store_status_unloaded(struct kvm_vcpu *vcpu, unsigned long addr);

0 commit comments

Comments
 (0)