Re: [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED
From: Jan Kiszka
Date: Tue Feb 07 2012 - 16:28:37 EST
On 2012-02-07 19:59, Marcelo Tosatti wrote:
> On Tue, Jan 31, 2012 at 10:35:31AM -0500, Eric B Munson wrote:
>> Now that we have a flag that will tell the guest it was suspended, create an
>> interface for that communication using a KVM ioctl.
>>
>> Signed-off-by: Eric B Munson <emunson@xxxxxxxxx>
>>
>> Cc: mingo@xxxxxxxxxx
>> Cc: hpa@xxxxxxxxx
>> Cc: ryanh@xxxxxxxxxxxxxxxxxx
>> Cc: aliguori@xxxxxxxxxx
>> Cc: mtosatti@xxxxxxxxxx
>> Cc: kvm@xxxxxxxxxxxxxxx
>> Cc: linux-arch@xxxxxxxxxxxxxxx
>> Cc: x86@xxxxxxxxxx
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> ---
>> Changes from V10:
>> Return the ioctl to per vcpu instead of per vm
>> Changes from V9:
>> Use kvm_for_each_vcpu to iterate online vcpu's
>> Changes from V8:
>> Make KVM_GUEST_PAUSED a per vm ioctl instead of per vcpu
>> Changes from V7:
>> Define KVM_CAP_GUEST_PAUSED and support check
>> Call mark_page_dirty () after setting PVCLOCK_GUEST_STOPPED
>> Changes from V4:
>> Rename KVM_GUEST_PAUSED to KVMCLOCK_GUEST_PAUSED
>> Add new ioctl description to api.txt
>>
>> Documentation/virtual/kvm/api.txt | 13 +++++++++++++
>> arch/x86/kvm/x86.c | 21 +++++++++++++++++++++
>> include/linux/kvm.h | 3 +++
>> 3 files changed, 37 insertions(+), 0 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index e1d94bf..1931e5c 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -1491,6 +1491,19 @@ following algorithm:
>> Some guests configure the LINT1 NMI input to cause a panic, aiding in
>> debugging.
>>
>> +4.65 KVMCLOCK_GUEST_PAUSED
>> +
>> +Capability: KVM_CAP_GUEST_PAUSED
>> +Architechtures: Any that implement pvclocks (currently x86 only)
>
> typo: Architectures.
>
>
>> +Type: vcpu ioctl
>> +Parameters: None
>> +Returns: 0 on success, -1 on error
>> +
>> +This signals to the host kernel that the specified guest is being paused by
>> +userspace. The host will set a flag in the pvclock structure that is checked
>> +from the soft lockup watchdog. This ioctl can be called during pause or
>> +unpause.
>> +
>> 5. The kvm_run structure
>>
>> Application code obtains a pointer to the kvm_run structure by
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 14d6cad..c9cabba 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2056,6 +2056,7 @@ int kvm_dev_ioctl_check_extension(long ext)
>> case KVM_CAP_X86_ROBUST_SINGLESTEP:
>> case KVM_CAP_XSAVE:
>> case KVM_CAP_ASYNC_PF:
>> + case KVM_CAP_GUEST_PAUSED:
>> case KVM_CAP_GET_TSC_KHZ:
>> r = 1;
>> break;
>> @@ -2503,6 +2504,22 @@ static int kvm_vcpu_ioctl_x86_set_xcrs(struct kvm_vcpu *vcpu,
>> return r;
>> }
>>
>> +/*
>> + * kvm_set_guest_paused() indicates to the guest kernel that it has been
>> + * stopped by the hypervisor. This function will be called from the host only.
>> + * EINVAL is returned when the host attempts to set the flag for a guest that
>> + * does not support pv clocks.
>> + */
>> +static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
>> +{
>> + struct pvclock_vcpu_time_info *src = &vcpu->arch.hv_clock;
>> + if (!vcpu->arch.time_page)
>> + return -EINVAL;
>> + src->flags |= PVCLOCK_GUEST_STOPPED;
>> + mark_page_dirty(vcpu->kvm, vcpu->arch.time >> PAGE_SHIFT);
>> + return 0;
>> +}
>
> It should also call kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu), so
> that the hv_clock is written to guest memory as soon as possible.
>
> Please fix the comments and rebase as soon as possible, it is annoying
> that this feature is taking so long to be integrated.
In addition:
- Avi asked for a KVM_XXX_ naming pattern of the IOCTL, to align it
with all the rest.
- He even suggested KVM_CLOCK_CTRL, thus an extensible interface for
passing flags via kvmclock on a per-vpcu basis.
- As this is now supposed to be per-vcpu, constants should be called
XXX_VCPU_STOPPED and no longer refer to "guest".
Thanks,
Jan
Attachment:
signature.asc
Description: OpenPGP digital signature