KVM: arm64: pvtime: Fixes and a new cap
Subsystems (6)
v2 cover KVM: arm64: pvtime: Fixes and a new cap
v2:
- ARM_SMCCC_HV_PV_TIME_FEATURES now also returns SMCCC_RET_NOT_SUPPORTED when steal time is not supported
- Added READ_ONCE() for the run_delay read
- Reworked kvm_put/get_guest to not require type as a parameter
- Added some more text to the documentation for KVM_CAP_STEAL_TIME
- Enough changed that I didn't pick up Steven's r-b's
The first four patches in the series are fixes that come from testing and reviewing pvtime code while writing the QEMU support[*]. The last patch is only a convenience for userspace, and I wouldn't be heartbroken if it wasn't deemed worth it. The QEMU patches are currently written without the cap. However, if the cap is accepted, then I'll change the QEMU code to use it.
Thanks,
drew
[*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg03856.html
(a v2 of this series will also be posted shortly)
Andrew Jones (6):
KVM: arm64: pvtime: steal-time is only supported when configured
KVM: arm64: pvtime: Fix potential loss of stolen time
KVM: arm64: Drop type input from kvm_put_guest
KVM: arm64: pvtime: Fix stolen time accounting across migration
KVM: Documentation: Minor fixups
arm64/x86: KVM: Introduce steal-time cap
Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++---- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 3 +++ arch/arm64/kvm/pvtime.c | 29 +++++++++++++---------------- arch/x86/kvm/x86.c | 3 +++ include/linux/kvm_host.h | 31 ++++++++++++++++++++++++++----- include/uapi/linux/kvm.h | 1 + 7 files changed, 65 insertions(+), 26 deletions(-)
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 1/6 KVM: arm64: pvtime: steal-time is only supported when configured
Don't confuse the guest by saying steal-time is supported when it hasn't been configured by userspace and won't work.
--- arch/arm64/kvm/pvtime.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/arm64/kvm/pvtime.c +2 -1
@@ -43,7 +43,8 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu)switch (feature) {case ARM_SMCCC_HV_PV_TIME_FEATURES :case ARM_SMCCC_HV_PV_TIME_ST : - val =SMCCC_RET_SUCCESS ; +if (vcpu->arch.steal.base !=GPA_INVALID ) + val =SMCCC_RET_SUCCESS ;break ; }
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 2/6 KVM: arm64: pvtime: Fix potential loss of stolen time
In reply to Andrew JonesWe should only check current->sched_info.run_delay once when updating stolen time. Otherwise there's a chance there could be a change between checks that we miss (preemption disabling comes after vcpu request checks).
--- arch/arm64/kvm/pvtime.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/arm64/kvm/pvtime.c +3 -2
@@ -13,6 +13,7 @@void kvm_update_stolen_time (struct kvm_vcpu *vcpu) {struct kvm *kvm = vcpu->kvm; +u64 last_steal = vcpu->arch.steal.last_steal;u64 steal;__le64 steal_le;u64 offset;
@@ -24,8 +25,8 @@ void kvm_update_stolen_time(struct kvm_vcpu *vcpu)/* Let's do the local bookkeeping */ steal = vcpu->arch.steal.steal; - steal += current->sched_info.run_delay - vcpu->arch.steal.last_steal; - vcpu->arch.steal.last_steal = current->sched_info.run_delay; + vcpu->arch.steal.last_steal =READ_ONCE (current->sched_info.run_delay); + steal += vcpu->arch.steal.last_steal - last_steal; vcpu->arch.steal.steal = steal; steal_le =cpu_to_le64 (steal);
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 3/6 KVM: arm64: Drop type input from kvm_put_guest
In reply to Andrew JonesWe can use typeof() to avoid the need for the type input.
--- arch/arm64/kvm/pvtime.c | 2 +- include/linux/kvm_host.h | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-)
arch/arm64/kvm/pvtime.c +1 -1
@@ -32,7 +32,7 @@ void kvm_update_stolen_time(struct kvm_vcpu *vcpu) steal_le =cpu_to_le64 (steal); idx =srcu_read_lock (&kvm->srcu); offset = offsetof(struct pvclock_vcpu_stolen_time , stolen_time); -kvm_put_guest (kvm, base + offset, steal_le, u64); +kvm_put_guest (kvm, base + offset, steal_le);srcu_read_unlock (&kvm->srcu, idx); }
include/linux/kvm_host.h +6 -5
@@ -749,25 +749,26 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,int kvm_gfn_to_hva_cache_init (struct kvm *kvm,struct gfn_to_hva_cache *ghc,gpa_t gpa,unsigned long len); -#define __kvm_put_guest (kvm, gfn, offset, value, type) \ +#define __kvm_put_guest (kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \ - type __user *__uaddr = (type __user *)(__addr + offset); \ + typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \ int __ret = -EFAULT; \ \ if (!kvm_is_error_hva(__addr)) \ - __ret = put_user(value, __uaddr); \ + __ret = put_user(v, __uaddr); \ if (!__ret) \ mark_page_dirty(kvm, gfn); \ __ret; \ }) -#define kvm_put_guest (kvm, gpa, value, type) \ +#define kvm_put_guest (kvm, gpa, v) \ ({ \ gpa_t __gpa = gpa; \ struct kvm *__kvm = kvm; \ + \ __kvm_put_guest(__kvm, __gpa >> PAGE_SHIFT, \ - offset_in_page(__gpa), (value), type); \ + offset_in_page(__gpa), v); \ })int kvm_clear_guest_page (struct kvm *kvm,gfn_t gfn,int offset,int len);
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 4/6 KVM: arm64: pvtime: Fix stolen time accounting across migration
In reply to Andrew JonesWhen updating the stolen time we should always read the current stolen time from the user provided memory, not from a kernel cache. If we use a cache then we'll end up resetting stolen time to zero on the first update after migration.
--- arch/arm64/include/asm/kvm_host.h | 1 - arch/arm64/kvm/pvtime.c | 23 +++++++++-------------- include/linux/kvm_host.h | 20 ++++++++++++++++++++ 3 files changed, 29 insertions(+), 15 deletions(-)
arch/arm64/include/asm/kvm_host.h +0 -1
@@ -343,7 +343,6 @@ struct kvm_vcpu_arch {/* Guest PV state */ struct { -u64 steal;u64 last_steal;gpa_t base; } steal;
arch/arm64/kvm/pvtime.c +9 -14
@@ -13,26 +13,22 @@void kvm_update_stolen_time (struct kvm_vcpu *vcpu) {struct kvm *kvm = vcpu->kvm; +u64 base = vcpu->arch.steal.base;u64 last_steal = vcpu->arch.steal.last_steal; -u64 steal; -__le64 steal_le; -u64 offset; +u64 offset = offsetof(struct pvclock_vcpu_stolen_time , stolen_time); +u64 steal =0 ;int idx; -u64 base = vcpu->arch.steal.base;if (base ==GPA_INVALID )return ; -/* Let's do the local bookkeeping */ - steal = vcpu->arch.steal.steal; - vcpu->arch.steal.last_steal =READ_ONCE (current->sched_info.run_delay); - steal += vcpu->arch.steal.last_steal - last_steal; - vcpu->arch.steal.steal = steal; - - steal_le =cpu_to_le64 (steal); idx =srcu_read_lock (&kvm->srcu); - offset = offsetof(struct pvclock_vcpu_stolen_time , stolen_time); -kvm_put_guest (kvm, base + offset, steal_le); +if (!kvm_get_guest (kvm, base + offset, steal)) { + steal =le64_to_cpu (steal); + vcpu->arch.steal.last_steal =READ_ONCE (current->sched_info.run_delay); + steal += vcpu->arch.steal.last_steal - last_steal; +kvm_put_guest (kvm, base + offset,cpu_to_le64 (steal)); + }srcu_read_unlock (&kvm->srcu, idx); }
@@ -66,7 +62,6 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)* Start counting stolen time from the time the guest requests * the feature enabled. */ - vcpu->arch.steal.steal =0 ; vcpu->arch.steal.last_steal = current->sched_info.run_delay; idx =srcu_read_lock (&kvm->srcu);
include/linux/kvm_host.h +20 -0
@@ -749,6 +749,26 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,int kvm_gfn_to_hva_cache_init (struct kvm *kvm,struct gfn_to_hva_cache *ghc,gpa_t gpa,unsigned long len); +#define __kvm_get_guest (kvm, gfn, offset, v) \ +({ \ + unsigned long __addr = gfn_to_hva(kvm, gfn); \ + typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset); \ + int __ret = -EFAULT; \ + \ + if (!kvm_is_error_hva(__addr)) \ + __ret = get_user(v, __uaddr); \ + __ret; \ +}) + +#define kvm_get_guest (kvm, gpa, v) \ +({ \ + gpa_t __gpa = gpa; \ + struct kvm *__kvm = kvm; \ + \ + __kvm_get_guest(__kvm, __gpa >> PAGE_SHIFT, \ + offset_in_page(__gpa), v); \ +}) +#define __kvm_put_guest (kvm, gfn, offset, v) \ ({ \ unsigned long __addr = gfn_to_hva(kvm, gfn); \
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 5/6 KVM: Documentation: Minor fixups
In reply to Andrew JonesIn preparation for documenting a new capability let's fix up the formatting of the current ones.
--- Documentation/virt/kvm/api.rst | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
Documentation/virt/kvm/api.rst +5 -4
@@ -6122,7 +6122,7 @@ HvCallSendSyntheticClusterIpi, HvCallSendSyntheticClusterIpiEx. 8.21 KVM_CAP_HYPERV_DIRECT_TLBFLUSH ----------------------------------- -:Architecture: x86 +:Architectures: x86 This capability indicates that KVM running on top of Hyper-V hypervisor enables Direct TLB flush for its guests meaning that TLB flush
@@ -6135,16 +6135,17 @@ in CPUID and only exposes Hyper-V identification. In this case, guest thinks it's running on Hyper-V and only use Hyper-V hypercalls. 8.22 KVM_CAP_S390_VCPU_RESETS +----------------------------- -Architectures: s390 +:Architectures: s390 This capability indicates that the KVM_S390_NORMAL_RESET and KVM_S390_CLEAR_RESET ioctls are available. 8.23 KVM_CAP_S390_PROTECTED +--------------------------- -Architecture: s390 - +:Architectures: s390 This capability indicates that the Ultravisor has been initialized and KVM can therefore start protected VMs.
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
PATCH v2 6/6 arm64/x86: KVM: Introduce steal-time cap
In reply to Andrew Jonesarm64 requires a vcpu fd (KVM_HAS_DEVICE_ATTR vcpu ioctl) to probe support for steal-time. However this is unnecessary, as only a KVM fd is required, and it complicates userspace (userspace may prefer delaying vcpu creation until after feature probing). Introduce a cap that can be checked instead. While x86 can already probe steal-time support with a kvm fd (KVM_GET_SUPPORTED_CPUID), we add the cap there too for consistency.
--- Documentation/virt/kvm/api.rst | 13 +++++++++++++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/arm.c | 3 +++ arch/arm64/kvm/pvtime.c | 2 +- arch/x86/kvm/x86.c | 3 +++ include/uapi/linux/kvm.h | 1 + 6 files changed, 22 insertions(+), 1 deletion(-)
Documentation/virt/kvm/api.rst +13 -0
@@ -6152,3 +6152,16 @@ KVM can therefore start protected VMs. This capability governs the KVM_S390_PV_COMMAND ioctl and the KVM_MP_STATE_LOAD MP_STATE. KVM_SET_MP_STATE can fail for protected guests when the state change is invalid. + +8.24 KVM_CAP_STEAL_TIME +----------------------- + +:Architectures: arm64, x86 + +This capability indicates that KVM supports steal time accounting. +When steal time accounting is supported it may be enabled with +architecture-specific interfaces. This capability and the architecture- +specific interfaces must be consistent, i.e. if one says the feature +is supported, than the other should as well and vice versa. For arm64 +see Documentation/virt/kvm/devices/vcpu.rst "KVM_ARM_VCPU_PVTIME_CTRL". +For x86 see Documentation/virt/kvm/msr.rst "MSR_KVM_STEAL_TIME".
arch/arm64/include/asm/kvm_host.h +1 -0
@@ -505,6 +505,7 @@ long kvm_hypercall_pv_features(struct kvm_vcpu *vcpu);gpa_t kvm_init_stolen_time (struct kvm_vcpu *vcpu);void kvm_update_stolen_time (struct kvm_vcpu *vcpu); +bool kvm_arm_pvtime_supported (void );int kvm_arm_pvtime_set_attr (struct kvm_vcpu *vcpu,struct kvm_device_attr *attr); int kvm_arm_pvtime_get_attr(struct kvm_vcpu *vcpu,
arch/arm64/kvm/arm.c +3 -0
@@ -222,6 +222,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)*/ r =1 ;break ; +case KVM_CAP_STEAL_TIME : + r =kvm_arm_pvtime_supported (); +break ;default : r =kvm_arch_vm_ioctl_check_extension (kvm, ext);break ;
arch/arm64/kvm/pvtime.c +1 -1
@@ -71,7 +71,7 @@ gpa_t kvm_init_stolen_time(struct kvm_vcpu *vcpu)return base; } -static bool kvm_arm_pvtime_supported (void ) +bool kvm_arm_pvtime_supported (void ) {return !!sched_info_on (); }
arch/x86/kvm/x86.c +3 -0
@@ -3538,6 +3538,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)case KVM_CAP_HYPERV_ENLIGHTENED_VMCS : r = kvm_x86_ops.nested_ops->enable_evmcs !=NULL ;break ; +case KVM_CAP_STEAL_TIME : + r =sched_info_on (); +break ;default :break ; }
include/uapi/linux/kvm.h +1 -0
@@ -1031,6 +1031,7 @@ struct kvm_ppc_resize_hpt {#define KVM_CAP_PPC_SECURE_GUEST 181#define KVM_CAP_HALT_POLL 182#define KVM_CAP_ASYNC_PF_INT 183 +#define KVM_CAP_STEAL_TIME 184#ifdef KVM_CAP_IRQ_ROUTING
Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 7 lines · Andrew Jones
v2:
-ARM_SMCCC_HV_PV_TIME_FEATURESnow also returnsSMCCC_RET_NOT_SUPPORTEDwhen steal time is not supported
- AddedREAD_ONCE()for therun_delayread
- Reworked kvm_put/get_guest to not require type as a parameter
- Added some more text to the documentation forKVM_CAP_STEAL_TIME
- Enough changed that I didn't pick up Steven's r-b's
Feel free to add my r-b's - the changes all look fine to me.
Thanks,
Steve
Quoted 22 lines
The first four patches in the series are fixes that come from testing and reviewing pvtime code while writing the QEMU support[*]. The last patch is only a convenience for userspace, and I wouldn't be heartbroken if it wasn't deemed worth it. The QEMU patches are currently written without the cap. However, if the cap is accepted, then I'll change the QEMU code to use it.
Thanks,
drew[*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg03856.html
(a v2 of this series will also be posted shortly)Andrew Jones (6):
KVM: arm64: pvtime: steal-time is only supported when configured
KVM: arm64: pvtime: Fix potential loss of stolen time
KVM: arm64: Drop type input fromkvm_put_guest
KVM: arm64: pvtime: Fix stolen time accounting across migration
KVM: Documentation: Minor fixups
arm64/x86: KVM: Introduce steal-time cap
Quoted 9 lines
Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++---- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 3 +++ arch/arm64/kvm/pvtime.c | 29 +++++++++++++---------------- arch/x86/kvm/x86.c | 3 +++ include/linux/kvm_host.h | 31 ++++++++++++++++++++++++++----- include/uapi/linux/kvm.h | 1 + 7 files changed, 65 insertions(+), 26 deletions(-)
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 30 lines · Andrew Jones
v2:
-ARM_SMCCC_HV_PV_TIME_FEATURESnow also returnsSMCCC_RET_NOT_SUPPORTEDwhen steal time is not supported
- AddedREAD_ONCE()for therun_delayread
- Reworked kvm_put/get_guest to not require type as a parameter
- Added some more text to the documentation forKVM_CAP_STEAL_TIME
- Enough changed that I didn't pick up Steven's r-b'sThe first four patches in the series are fixes that come from testing and reviewing pvtime code while writing the QEMU support[*]. The last patch is only a convenience for userspace, and I wouldn't be heartbroken if it wasn't deemed worth it. The QEMU patches are currently written without the cap. However, if the cap is accepted, then I'll change the QEMU code to use it.
Thanks,
drew[*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg03856.html
(a v2 of this series will also be posted shortly)Andrew Jones (6):
KVM: arm64: pvtime: steal-time is only supported when configured
KVM: arm64: pvtime: Fix potential loss of stolen time
KVM: arm64: Drop type input fromkvm_put_guest
KVM: arm64: pvtime: Fix stolen time accounting across migration
KVM: Documentation: Minor fixups
arm64/x86: KVM: Introduce steal-time cap
Quoted 17 lines
Documentation/virt/kvm/api.rst | 22 ++++++++++++++++++---- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 3 +++ arch/arm64/kvm/pvtime.c | 29 +++++++++++++---------------- arch/x86/kvm/x86.c | 3 +++ include/linux/kvm_host.h | 31 ++++++++++++++++++++++++++----- include/uapi/linux/kvm.h | 1 + 7 files changed, 65 insertions(+), 26 deletions(-)Signature · 6 lines
2.25.4 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Hi Marc,
Gentle ping. I'd like to to switch the QEMU code to using the proposed KVM cap, if the cap is accepted.
Thanks,
drew
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerQuoted 51 lines · Andrew Jones
Quoted 41 lines · Andrew Jones
v2:
-ARM_SMCCC_HV_PV_TIME_FEATURESnow also returnsSMCCC_RET_NOT_SUPPORTED
when steal time is not supported
- AddedREAD_ONCE()for therun_delayread
- Reworked kvm_put/get_guest to not require type as a parameter
- Added some more text to the documentation forKVM_CAP_STEAL_TIME
- Enough changed that I didn't pick up Steven's r-b'sThe first four patches in the series are fixes that come from testing and reviewing pvtime code while writing the QEMU support[*]. The last patch is only a convenience for userspace, and I wouldn't be heartbroken if it wasn't deemed worth it. The QEMU patches are currently written without the cap. However, if the cap is accepted, then I'll change the QEMU code to use it.
Thanks,
drew[*] https://lists.gnu.org/archive/html/qemu-devel/2020-07/msg03856.html
(a v2 of this series will also be posted shortly)Andrew Jones (6):
KVM: arm64: pvtime: steal-time is only supported when configured
KVM: arm64: pvtime: Fix potential loss of stolen time
KVM: arm64: Drop type input fromkvm_put_guest
KVM: arm64: pvtime: Fix stolen time accounting across migration
KVM: Documentation: Minor fixups
arm64/x86: KVM: Introduce steal-time capDocumentation/virt/kvm/api.rst | 22 ++++++++++++++++++---- arch/arm64/include/asm/kvm_host.h | 2 +- arch/arm64/kvm/arm.c | 3 +++ arch/arm64/kvm/pvtime.c | 29 +++++++++++++---------------- arch/x86/kvm/x86.c | 3 +++ include/linux/kvm_host.h | 31 ++++++++++++++++++++++++++----- include/uapi/linux/kvm.h | 1 + 7 files changed, 65 insertions(+), 26 deletions(-)Quoted 8 lines
--
2.25.4_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 4 lines
Hi Marc,
Gentle ping. I'd like to to switch the QEMU code to using the proposed KVM cap, if the cap is accepted.
I'm fine with it. To be honest, this series is mostly fixes, except for that last patch.
Paolo, are you OK with me sending the whole thing as fixes, including the UAPI patch? At least we'd have something consistent for 5.9.
Thanks,
M.
Signature · 5 lines
Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm