On 20.09.2017 19:42, Ken Hofsass wrote:
> This commit implements an enhanced x86 version of S390
> KVM_CAP_SYNC_REGS functionality. KVM_CAP_SYNC_REGS "allow[s]
> userspace to access certain guest registers without having
> to call SET/GET_*REGS”. This reduces ioctl overhead which
> is particularly important when userspace is making synchronous
> guest state modifications (e.g. when emulating and/or intercepting
> instructions).
>
> Originally implemented upstream for the S390, the x86 differences
> follow:
> - the capability can be enabled/disabled
> - the register sets to be copied out out to kvm_run are selectable
> by userspace
Why is this necessary? Why not simply store everything? And mark via
kvm_valid_regs which fields are actually valid?
Also, I wonder if user space could simply modify (reduce)
vcpu->run->kvm_valid_regs to achieve the same behavior (when storing in
the kernel, simply check if the valid bit is set).
> - vcpu_events and selectable MSRs are available in addition to the
> basic register sets (regs, sregs, debug_regs, and fpu).
>
> Signed-off-by: Ken Hofsass <hofsass@xxxxxxxxxx>
> ---
> Documentation/virtual/kvm/api.txt | 49 ++++++++++++++
> arch/x86/include/uapi/asm/kvm.h | 32 ++++++++-
> arch/x86/kvm/x86.c | 134 +++++++++++++++++++++++++++++++++++++-
> include/uapi/linux/kvm.h | 6 +-
> 4 files changed, 216 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index e63a35fafef0..9536eb310d04 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -3951,6 +3951,55 @@ Once this is done the KVM_REG_MIPS_VEC_* and KVM_REG_MIPS_MSA_* registers can be
> accessed, and the Config5.MSAEn bit is accessible via the KVM API and also from
> the guest.
>
> +6.74 KVM_CAP_SYNC_REGS
> +Architectures: s390, x86 (vmx-only)
> +Target: s390: always enabled, x86: vcpu
> +Parameters: s390: none, x86: args[0] takes bitmask selecting which register
> +sets to copy out (bitfields defined in arch/x86/include/uapi/asm/kvm.h).
> +
> +As described above in the kvm_sync_regs struct info in section 5 (kvm_run):
> +KVM_CAP_SYNC_REGS "allow[s] userspace to access certain guest registers
> +without having to call SET/GET_*REGS". This reduces overhead by eliminating
> +repeated ioctl calls for setting and/or getting register values. This is
> +particularly important when userspace is making synchronous guest state
> +modifications, e.g. when emulating and/or intercepting instructions in
> +userspace.
> +
> +For s390 specifics, please the source code.
"... please have a look at the source code."?
> +
> +For x86:
> +- the capability can be enabled/disabled (s390 is always enabled).
Move that s390 comment to "For s390".
You could simply always store and let user space control via
vcpu->run->kvm_valid_regs which ones to actually store. This would store
for old QEMUs, but do we care? This would get rid of the need for the
capability. User space could simply check vcpu->run->kvm_valid_regs
(assuming it was always set to 0, we would have to verify).
> +- the register sets to be copied out out to kvm_run are selectable
> + by userspace (rather that all sets being copied out for every exit).
> +- vcpu_events and selectable MSRs are available in addition to the
> + basic register sets (regs, sregs, debug_regs, and fpu).
> +> +In addition to the register sets, the x86 'kvm_sync_regs' struct
definition
> +includes the bitarray, 'sync_regs', that toggles whether specific register
> +sets are to be copied out. When enabling SYNC_REGS via KVM_CAP_ENABLE, the
> +value in args[0] is copied into the 'sync_regs' bitarray value by KVM. But
> +like the register sets, the copy-out field can be modified while handling
> +an exit so that additonal KVM_CAP_ENABLE ioctl calls can be avoided.
> +
> +To indicate when userspace has modified values that should be copied into
> +the vCPU, the all architecture bitarray field, 'kvm_dirty_regs' must be set.
> +This is done using the same bitflags as for the 'sync_regs' copy-out field.
> +If the dirty bit is not set, then the register values will not be copied
> +into the vCPU even if they've been modified.
> +
> +Unused bitfields in the bitarrays must be set to zero.
> +
> +struct kvm_sync_regs {
> + __u64 sync_regs;
> + struct kvm_regs regs;
> + struct kvm_sregs sregs;
> + struct kvm_debugregs debugregs;
> + struct kvm_fpu fpu;
> + struct kvm_vcpu_events events;
> + struct kvm_msrs msrs;
> + struct kvm_msr_entry msr_entries[SYNC_REGS_NUM_MSRS];
> +};
> +
--
Thanks,
David
