On 09/19/2014 04:19 PM, Jason J. Herne wrote:
> From: "Jason J. Herne" <jjherne@xxxxxxxxxx>
>
> Enable KVM_SET_CLOCK and KVM_GET_CLOCK Ioctls on S390 for managing guest TOD
> clock value.
>
Just some education. On s390 the guest visible TOD clock is thehost TOD clock + hypervisor programmable offset in the control block. There is only one TOD per system, so the offset must be the same for every CPU.
> we add the KVM_CLOCK_FORWARD_ONLY flag to indicate to KVM_SET_CLOCK that the
> given clock value should only be set if it is >= the current guest TOD clock
host TOD, (right?)
The alternative scheme would be to simply get/set the guest TOD time. This works perfect for migration, but for managedsave the guest time is in the past.
Your approach has the advantange that after managedsave the guest will (most of the time) have the host time of the target system, avoiding that the guest has a time that is in the past (e.g. after 1 week managedsave the guest would live in the past).
Question for Paolo (maybe others) is. Does it make sense to reuse/extend the existing ioctl (I think so, but defining a new one could also be ok)
Christian
> value. This guarantees a monotonically increasing time.
>
> NOTE: In the event that the KVM_CLOCK_FORWARD_ONLY flag is set and the given
> time would cause the guest time to jump backward, then we set the guest TOD
> clock equal to the host TOD clock. Does this behavior make sense, or is it too
> weird? I could believe that other architectures might not want this exact
> behavior. Instead they might prefer to implement the function such that an
> error code is returned instead of syncing the guest time to host time? In that
> case S390 would need another bit KVM_CLOCK_SET_TO_HOST which we could call to
> sync host time when the preferred guest time value would otherwise violate
> the monotonic property of the KVM_CLOCK_FORWARD_ONLY flag.
>
> Signed-off-by: Jason J. Herne <jjherne@xxxxxxxxxx>
> ---
> Documentation/virtual/kvm/api.txt | 5 +++
> arch/s390/kvm/kvm-s390.c | 80 +++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 3 ++
> 3 files changed, 88 insertions(+)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index beae3fd..615c2e4 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -779,6 +779,11 @@ struct kvm_clock_data {
> __u32 pad[9];
> };
>
> +S390: KVM_CLOCK_FORWARD_ONLY is used by KVM_SET_CLOCK to indicate that the guest
> +TOD clock should not be allowed to jump back in time. This flag guarantees a
> +monotonically increasing guest clock. If the clock value specified would cause
> +the guest to jump back in time then the guest TOD clock is set to the host
> +TOD clock value.
>
> 4.31 KVM_GET_VCPU_EVENTS
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 81b0e11..2450db3 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -31,6 +31,7 @@
> #include <asm/switch_to.h>
> #include <asm/facility.h>
> #include <asm/sclp.h>
> +#include<asm/timex.h>
> #include "kvm-s390.h"
> #include "gaccess.h"
>
> @@ -169,6 +170,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_S390_IRQCHIP:
> case KVM_CAP_VM_ATTRIBUTES:
> case KVM_CAP_MP_STATE:
> + case KVM_CAP_ADJUST_CLOCK:
> r = 1;
> break;
> case KVM_CAP_NR_VCPUS:
> @@ -338,6 +340,63 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
> return ret;
> }
>
> +static int kvm_s390_get_guest_tod(struct kvm *kvm, struct kvm_clock_data *user_clock)
> +{
> + u64 current_host_tod;
> + u64 epoch = 0;
> + struct kvm_vcpu *vcpu;
> + unsigned int vcpu_idx;
> + int r;
> +
> + /* All vcpu's epochs are in sync. Just Grab the 1st one */
> + kvm_for_each_vcpu(vcpu_idx, vcpu, kvm)
> + {
> + epoch = vcpu->arch.sie_block->epoch;
> + break;
> + }
> +
> + r = store_tod_clock(¤t_host_tod);
> + if (r)
> + return r;
> +
> + user_clock->clock = current_host_tod + epoch;
> + return 0;
> +}
> +
> +/*
> +Set the guest's effective TOD clock to the given value. The guest's
> +TOD clock is determined by the following formula: gtod = host_tod + epoch.
> +NOTE: Even though the epoch value is associated with a "vcpu", there is only
> +one TOD clock and epoch value per guest. All vcpu's epoch values must be kept
> +synchronized.
> +NOTE: The KVM_CLOCK_FORWARD_ONLY flag is used to indicate that the guest clock
> +should only be set to the provided value if doing so does not cause guest time
> +to jump backwards. In this case we zero the epoch thereby making the guest TOD
> +clock equal to the host TOD clock.
> +*/
> +static int kvm_s390_set_guest_tod(struct kvm *kvm, struct kvm_clock_data *user_clock)
> +{
> + u64 current_host_tod, epoch;
> + struct kvm_vcpu *vcpu;
> + unsigned int vcpu_idx;
> + int r;
> +
> + r = store_tod_clock(¤t_host_tod);
> + if (r)
> + return r;
> +
> + if ((user_clock->flags & KVM_CLOCK_FORWARD_ONLY) &&
> + (current_host_tod > user_clock->clock))
> + epoch = 0;
> + else
> + epoch = user_clock->clock - current_host_tod;
> +
> + kvm_for_each_vcpu(vcpu_idx, vcpu, kvm)
> + vcpu->arch.sie_block->epoch = epoch;
> +
> + return 0;
> +}
> +
> long kvm_arch_vm_ioctl(struct file *filp,
> unsigned int ioctl, unsigned long arg)
> {
> @@ -397,6 +456,27 @@ long kvm_arch_vm_ioctl(struct file *filp,
> r = kvm_s390_vm_has_attr(kvm, &attr);
> break;
> }
> + case KVM_GET_CLOCK: {
> + struct kvm_clock_data user_clock;
> +
> + r = kvm_s390_get_guest_tod(kvm, &user_clock);
> + if (r)
> + break;
> +
> + if (copy_to_user(argp, &user_clock, sizeof(user_clock)))
> + r = -EFAULT;
> +
> + break;
> + }
> + case KVM_SET_CLOCK: {
> + struct kvm_clock_data user_clock;
> +
> + r = -EFAULT;
> + if (copy_from_user(&user_clock, argp, sizeof(struct kvm_clock_data)))
> + break;
> + r = kvm_s390_set_guest_tod(kvm, &user_clock);
> + break;
> + }
> default:
> r = -ENOTTY;
> }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index cf3a2ff..a832b6a 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -857,6 +857,9 @@ struct kvm_irqfd {
> __u8 pad[16];
> };
>
> +/* Flag definitions for kvm_clock_data.flags */
> +#define KVM_CLOCK_FORWARD_ONLY (1 << 0)
> +
> struct kvm_clock_data {
> __u64 clock;
> __u32 flags;
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
