[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <716f64d5-a7db-e96f-6d4e-a99a42d684a0@redhat.com>
Date: Wed, 2 Aug 2017 09:50:45 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Peng Hao <peng.hao2@....com.cn>, rkrcmar@...hat.com,
tglx@...utronix.de, mingo@...hat.com, hpa@...or.com
Cc: x86@...nel.org, kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kvm: x86: reduce rtc 0x70 access vm-exit time
On 02/08/2017 17:24, Peng Hao wrote:
> some versions of windows guest access rtc frequently because of
> rtc as system tick.guest access rtc like this: write register index
> to 0x70, then write or read data from 0x71. writing 0x70 port is
> just as index and do nothing else. So writing 0x70 is not necessory
> to exit to userspace every time and caching rtc register index in kvm
> can reduce VM-EXIT time.
> without my patch, get the vm-exit time of accessing rtc 0x70 using
> perf tools: (guest OS : windows 7 64bit)
> IO Port Access Samples Samples% Time% Min Time Max Time Avg time
> 0x70:POUT 86 30.99% 74.59% 9us 29us 10.75us (+- 3.41%)
>
> with my patch
> IO Port Access Samples Samples% Time% Min Time Max Time Avg time
> 0x70:POUT 106 32.02% 29.47% 0us 10us 1.57us (+- 7.38%)
>
> the patch is a part of optimizing rtc 0x70 port access.another is in
> qemu.
>
> Signed-off-by: Peng Hao <peng.hao2@....com.cn>
> Reviewed-by: Liu Yi <liu.yi24@....com.cn>
Technically, bit 7 of port 0x70 disables NMI, but QEMU doesn't implement
that part so I guess that's fine.
How would userspace read the index? Could you instead extend the
existing coalesced MMIO support to I/O ports?
Paolo
> ---
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/Makefile | 2 +-
> arch/x86/kvm/vrtc.c | 101 ++++++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/vrtc.h | 19 ++++++++
> arch/x86/kvm/x86.c | 15 ++++++
> include/uapi/linux/kvm.h | 2 +
> 6 files changed, 139 insertions(+), 1 deletion(-)
> create mode 100644 arch/x86/kvm/vrtc.c
> create mode 100644 arch/x86/kvm/vrtc.h
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 1588e9e..41fde27 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -761,6 +761,7 @@ struct kvm_arch {
> struct kvm_pic *vpic;
> struct kvm_ioapic *vioapic;
> struct kvm_pit *vpit;
> + struct kvm_vrtc *vrtc;
> atomic_t vapics_in_nmi_mode;
> struct mutex apic_map_lock;
> struct kvm_apic_map *apic_map;
> diff --git a/arch/x86/kvm/Makefile b/arch/x86/kvm/Makefile
> index 09d4b17..dfd4364 100644
> --- a/arch/x86/kvm/Makefile
> +++ b/arch/x86/kvm/Makefile
> @@ -13,7 +13,7 @@ kvm-$(CONFIG_KVM_ASYNC_PF) += $(KVM)/async_pf.o
>
> kvm-y += x86.o mmu.o emulate.o i8259.o irq.o lapic.o \
> i8254.o ioapic.o irq_comm.o cpuid.o pmu.o mtrr.o \
> - hyperv.o page_track.o debugfs.o
> + hyperv.o page_track.o debugfs.o vrtc.o
>
> kvm-intel-y += vmx.o pmu_intel.o
> kvm-amd-y += svm.o pmu_amd.o
> diff --git a/arch/x86/kvm/vrtc.c b/arch/x86/kvm/vrtc.c
> new file mode 100644
> index 0000000..c8b45c0
> --- /dev/null
> +++ b/arch/x86/kvm/vrtc.c
> @@ -0,0 +1,101 @@
> +#include <linux/kvm_host.h>
> +#include <linux/slab.h>
> +
> +#include "x86.h"
> +#include "vrtc.h"
> +
> +static inline struct kvm_vrtc *dev_to_vrtc(struct kvm_io_device *dev)
> +{
> + return container_of(dev, struct kvm_vrtc, dev);
> +}
> +
> +static int vrtc_ioport_read(struct kvm_vcpu *vcpu,
> + struct kvm_io_device *dev,
> + gpa_t addr, int len, void *data)
> +{
> + return 0xff;
> +}
> +
> +static int vrtc_ioport_write(struct kvm_vcpu *vcpu,
> + struct kvm_io_device *dev,
> + gpa_t addr, int len, const void *val)
> +{
> + struct kvm_vrtc *vrtc = dev_to_vrtc(dev);
> + unsigned char data = *(unsigned char *)val;
> + int ret = -EOPNOTSUPP;
> +
> + if (addr != KVM_VRTC_BASE_ADDRESS)
> + return -EOPNOTSUPP;
> +
> + spin_lock(&vrtc->lock);
> + if (vrtc->prev_reg_index == 0xff) {
> + vrtc->prev_reg_index = data;
> + goto out;
> + }
> +
> + if (data == vrtc->prev_reg_index) {
> + ret = 0;
> + goto out;
> + } else {
> + vrtc->prev_reg_index = data;
> + goto out;
> + }
> +
> +out:
> + spin_unlock(&vrtc->lock);
> + return ret;
> +}
> +
> +static const struct kvm_io_device_ops vrtc_dev_ops = {
> + .read = vrtc_ioport_read,
> + .write = vrtc_ioport_write,
> +};
> +
> +struct kvm_vrtc *kvm_create_vrtc(struct kvm *kvm)
> +{
> + struct kvm_vrtc *vrtc;
> + int ret;
> +
> + vrtc = kzalloc(sizeof(struct kvm_vrtc), GFP_KERNEL);
> + if (!vrtc)
> + return NULL;
> +
> + spin_lock_init(&vrtc->lock);
> + vrtc->kvm = kvm;
> + kvm_vrtc_reset(vrtc);
> + mutex_lock(&kvm->slots_lock);
> +
> + kvm_iodevice_init(&vrtc->dev, &vrtc_dev_ops);
> + ret = kvm_io_bus_register_dev(kvm, KVM_PIO_BUS, KVM_VRTC_BASE_ADDRESS,
> + 1, &vrtc->dev);
> + if (ret < 0)
> + goto fail_register_vrtc;
> +
> + mutex_unlock(&kvm->slots_lock);
> + return vrtc;
> +
> +fail_register_vrtc:
> + mutex_unlock(&kvm->slots_lock);
> + kfree(vrtc);
> + return NULL;
> +}
> +
> +void kvm_vrtc_reset(struct kvm_vrtc *vrtc)
> +{
> + vrtc->prev_reg_index = 0xff;
> +}
> +
> +void kvm_vrtc_destroy(struct kvm *kvm)
> +{
> + struct kvm_vrtc *vrtc = kvm->arch.vrtc;
> +
> + if (!vrtc)
> + return;
> + mutex_lock(&kvm->slots_lock);
> + kvm_io_bus_unregister_dev(kvm, KVM_PIO_BUS, &vrtc->dev);
> + mutex_unlock(&kvm->slots_lock);
> +
> + kvm->arch.vrtc = NULL;
> + kfree(vrtc);
> +}
> +
> diff --git a/arch/x86/kvm/vrtc.h b/arch/x86/kvm/vrtc.h
> new file mode 100644
> index 0000000..ea434ce
> --- /dev/null
> +++ b/arch/x86/kvm/vrtc.h
> @@ -0,0 +1,19 @@
> +#ifndef __VRTC_H
> +#define __VRTC_H
> +
> +#include <kvm/iodev.h>
> +#include <linux/spinlock.h>
> +
> +struct kvm_vrtc {
> + spinlock_t lock;
> + struct kvm_io_device dev;
> + struct kvm *kvm;
> + unsigned char prev_reg_index;
> +};
> +
> +
> +#define KVM_VRTC_BASE_ADDRESS 0x70
> +struct kvm_vrtc *kvm_create_vrtc(struct kvm *kvm);
> +void kvm_vrtc_destroy(struct kvm *kvm);
> +void kvm_vrtc_reset(struct kvm_vrtc *vrtc);
> +#endif
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6c7266f..426cf82 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -29,6 +29,7 @@
> #include "cpuid.h"
> #include "pmu.h"
> #include "hyperv.h"
> +#include "vrtc.h"
>
> #include <linux/clocksource.h>
> #include <linux/interrupt.h>
> @@ -4033,6 +4034,19 @@ long kvm_arch_vm_ioctl(struct file *filp,
> mutex_unlock(&kvm->lock);
> break;
> }
> + case KVM_CREATE_VRTC:
> + mutex_lock(&kvm->lock);
> + r = -EEXIST;
> + if (kvm->arch.vrtc)
> + goto create_vrtc_unlock;
> +
> + r = -ENOMEM;
> + kvm->arch.vrtc = kvm_create_vrtc(kvm);
> + if (kvm->arch.vrtc)
> + r = 0;
> +create_vrtc_unlock:
> + mutex_unlock(&kvm->lock);
> + break;
> case KVM_CREATE_PIT:
> u.pit_config.flags = KVM_PIT_SPEAKER_DUMMY;
> goto create_pit;
> @@ -8168,6 +8182,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
> if (kvm_x86_ops->vm_destroy)
> kvm_x86_ops->vm_destroy(kvm);
> kvm_pic_destroy(kvm);
> + kvm_vrtc_destroy(kvm);
> kvm_ioapic_destroy(kvm);
> kvm_free_vcpus(kvm);
> kvfree(rcu_dereference_check(kvm->arch.apic_map, 1));
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index c0b6dfe..ce503f35 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -927,6 +927,7 @@ struct kvm_ppc_resize_hpt {
> #define KVM_CAP_S390_CMMA_MIGRATION 145
> #define KVM_CAP_PPC_FWNMI 146
> #define KVM_CAP_PPC_SMT_POSSIBLE 147
> +#define KVM_CAP_VRTC 150
>
> #ifdef KVM_CAP_IRQ_ROUTING
>
> @@ -1256,6 +1257,7 @@ struct kvm_s390_ucas_mapping {
> #define KVM_PPC_CONFIGURE_V3_MMU _IOW(KVMIO, 0xaf, struct kvm_ppc_mmuv3_cfg)
> /* Available with KVM_CAP_PPC_RADIX_MMU */
> #define KVM_PPC_GET_RMMU_INFO _IOW(KVMIO, 0xb0, struct kvm_ppc_rmmu_info)
> +#define KVM_CREATE_VRTC _IO(KVMIO, 0xba)
>
> /* ioctl for vm fd */
> #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
>
Powered by blists - more mailing lists