Re: [PATCH 1/4] kvm: provide synchronous registers in kvm_run

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 05.01.2012, at 10:54, Christian Borntraeger wrote:

> On some cpus the overhead for virtualization instructions is in the same
> range as a system call. Having to call multiple ioctls to get set registers
> will make certain userspace handled exits more expensive than necessary.
> Lets provide two sections in kvm_run to have a shared save area for
> guest registers.
> 1. the first section is read-only, to handle registers that have side-effects
> 2. the second section is read/write, e.g. for general purpose registers.
> We also provide two 64bit flags fields (architecture specific), that will
> specify which parts of these fields are valid. Each bit will define that
> a group of registers (like general purpose) or a single register is valid.
> In that way we can extend and shrink the interface. (The structure definition
> itself can only grow of course).
> 
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> ---
> Documentation/virtual/kvm/api.txt |   28 ++++++++++++++++++++++++++++
> arch/ia64/include/asm/kvm.h       |    7 +++++++
> arch/powerpc/include/asm/kvm.h    |    7 +++++++
> arch/s390/include/asm/kvm.h       |    6 ++++++
> arch/x86/include/asm/kvm.h        |    7 +++++++
> include/linux/kvm.h               |   20 ++++++++++++++++++++
> 6 files changed, 75 insertions(+), 0 deletions(-)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index da1f8fd..a149e22 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -1693,6 +1693,34 @@ developer registration required to access it).
> 		/* Fix the size of the union. */
> 		char padding[256];
> 	};
> +
> +	/*
> +	 * Here are two fields that allow to access often used registers
> +         * directly, to avoid the overhead of the ioctl system call. Each
> +         * register can be extended or reduced by having flag bits specifying
> +         * if a group of registers is valid.
> +         */
> +	__u64 kvm_valid_sync_ro_regs;
> +	__u64 kvm_valid_sync_rw_regs;
> +	union {
> +		/* registers which can be only read */
> +		struct kvm_sync_ro_regs sync_ro;
> +		char padding[1024];
> +	};
> +	union {
> +		/* read/write guest registers */
> +		struct kvm_sync_rw_regs sync_rw;
> +		char padding[1024];
> +	};

Ah, new patch set. Same comment here. Please give the union a name. Also, I still don't understand why we need 2 structs. All we need is a bitmap that says "this field is dirty" and then kernel space can decide to ignore it.


Alex

--
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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux