Skip to content

Commit 6973091

Browse files
Nico Boehrfrankjaa
authored andcommitted
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 <[email protected]> Signed-off-by: Nico Boehr <[email protected]> Reviewed-by: Claudio Imbrenda <[email protected]> Reviewed-by: Janosch Frank <[email protected]> Link: https://lore.kernel.org/r/[email protected] Message-Id: <[email protected]> Signed-off-by: Janosch Frank <[email protected]>
1 parent f0c4d9f commit 6973091

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
@@ -1207,6 +1207,8 @@ static int kvm_s390_vm_get_migration(struct kvm *kvm,
12071207
return 0;
12081208
}
12091209

1210+
static void __kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod);
1211+
12101212
static int kvm_s390_set_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
12111213
{
12121214
struct kvm_s390_vm_tod_clock gtod;
@@ -1216,7 +1218,7 @@ static int kvm_s390_set_tod_ext(struct kvm *kvm, struct kvm_device_attr *attr)
12161218

12171219
if (!test_kvm_facility(kvm, 139) && gtod.epoch_idx)
12181220
return -EINVAL;
1219-
kvm_s390_set_tod_clock(kvm, &gtod);
1221+
__kvm_s390_set_tod_clock(kvm, &gtod);
12201222

12211223
VM_EVENT(kvm, 3, "SET: TOD extension: 0x%x, TOD base: 0x%llx",
12221224
gtod.epoch_idx, gtod.tod);
@@ -1247,7 +1249,7 @@ static int kvm_s390_set_tod_low(struct kvm *kvm, struct kvm_device_attr *attr)
12471249
sizeof(gtod.tod)))
12481250
return -EFAULT;
12491251

1250-
kvm_s390_set_tod_clock(kvm, &gtod);
1252+
__kvm_s390_set_tod_clock(kvm, &gtod);
12511253
VM_EVENT(kvm, 3, "SET: TOD base: 0x%llx", gtod.tod);
12521254
return 0;
12531255
}
@@ -1259,6 +1261,16 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
12591261
if (attr->flags)
12601262
return -EINVAL;
12611263

1264+
mutex_lock(&kvm->lock);
1265+
/*
1266+
* For protected guests, the TOD is managed by the ultravisor, so trying
1267+
* to change it will never bring the expected results.
1268+
*/
1269+
if (kvm_s390_pv_is_protected(kvm)) {
1270+
ret = -EOPNOTSUPP;
1271+
goto out_unlock;
1272+
}
1273+
12621274
switch (attr->attr) {
12631275
case KVM_S390_VM_TOD_EXT:
12641276
ret = kvm_s390_set_tod_ext(kvm, attr);
@@ -1273,6 +1285,9 @@ static int kvm_s390_set_tod(struct kvm *kvm, struct kvm_device_attr *attr)
12731285
ret = -ENXIO;
12741286
break;
12751287
}
1288+
1289+
out_unlock:
1290+
mutex_unlock(&kvm->lock);
12761291
return ret;
12771292
}
12781293

@@ -4377,13 +4392,6 @@ static void __kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_t
43774392
preempt_enable();
43784393
}
43794394

4380-
void kvm_s390_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod)
4381-
{
4382-
mutex_lock(&kvm->lock);
4383-
__kvm_s390_set_tod_clock(kvm, gtod);
4384-
mutex_unlock(&kvm->lock);
4385-
}
4386-
43874395
int kvm_s390_try_set_tod_clock(struct kvm *kvm, const struct kvm_s390_vm_tod_clock *gtod)
43884396
{
43894397
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)