linux-kernel - Re: [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 07 Feb 2012 22:28:20 +0100
From:	Jan Kiszka <jan.kiszka@....de>
To:	Marcelo Tosatti <mtosatti@...hat.com>
CC:	Eric B Munson <emunson@...bm.net>, avi@...hat.com,
	mingo@...hat.com, hpa@...or.com, ryanh@...ux.vnet.ibm.com,
	aliguori@...ibm.com, kvm@...r.kernel.org,
	linux-arch@...r.kernel.org, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4 V11] Add ioctl for KVMCLOCK_GUEST_STOPPED

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@...bm.net>
>>
>> Cc: mingo@...hat.com
>> Cc: hpa@...or.com
>> Cc: ryanh@...ux.vnet.ibm.com
>> Cc: aliguori@...ibm.com
>> Cc: mtosatti@...hat.com
>> Cc: kvm@...r.kernel.org
>> Cc: linux-arch@...r.kernel.org
>> Cc: x86@...nel.org
>> Cc: linux-kernel@...r.kernel.org
>> ---
>> 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



Download attachment "signature.asc" of type "application/pgp-signature" (263 bytes)