| 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
| ||
|
Date: Mon, 7 Aug 2017 10:55:31 +0200
From: David Hildenbrand <david@...hat.com>
To: "Longpeng(Mike)" <longpeng2@...wei.com>, pbonzini@...hat.com,
rkrcmar@...hat.com
Cc: agraf@...e.com, borntraeger@...ibm.com, cohuck@...hat.com,
christoffer.dall@...aro.org, marc.zyngier@....com,
james.hogan@...tec.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org, weidong.huang@...wei.com,
arei.gonglei@...wei.com, wangxinxin.wang@...wei.com,
longpeng.mike@...il.com
Subject: Re: [PATCH 1/3] KVM: add spinlock-exiting optimize framework
On 07.08.2017 10:44, Longpeng(Mike) wrote:
> If the vcpu(me) exit due to request a usermode spinlock, then
> the spinlock-holder may be preempted in usermode or kernmode.
>
> But if the vcpu(me) is in kernmode, then the holder must be
> preempted in kernmode, so we should choose a vcpu in kernmode
> as the most eligible candidate.
>
> For some architecture(e.g. arm/s390), spin/preempt_in_kernel()
> are the same, but they are different for X86.
>
> Signed-off-by: Longpeng(Mike) <longpeng2@...wei.com>
> ---
> arch/mips/kvm/mips.c | 10 ++++++++++
> arch/powerpc/kvm/powerpc.c | 10 ++++++++++
> arch/s390/kvm/kvm-s390.c | 10 ++++++++++
> arch/x86/kvm/x86.c | 10 ++++++++++
> include/linux/kvm_host.h | 2 ++
> virt/kvm/arm/arm.c | 10 ++++++++++
> virt/kvm/kvm_main.c | 4 ++++
> 7 files changed, 56 insertions(+)
>
> diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
> index d4b2ad1..e04e6b3 100644
> --- a/arch/mips/kvm/mips.c
> +++ b/arch/mips/kvm/mips.c
> @@ -98,6 +98,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> return !!(vcpu->arch.pending_exceptions);
> }
>
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
> {
> return 1;
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 1a75c0b..c573ddd 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -58,6 +58,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
> return !!(v->arch.pending_exceptions) || kvm_request_pending(v);
> }
>
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
> {
> return 1;
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index af09d34..f78cdc2 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2447,6 +2447,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> return kvm_s390_vcpu_has_irq(vcpu, 0);
> }
>
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> void kvm_s390_vcpu_block(struct kvm_vcpu *vcpu)
> {
> atomic_or(PROG_BLOCK_SIE, &vcpu->arch.sie_block->prog20);
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 6c97c82..04c6a1f 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8435,6 +8435,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu)
> return kvm_vcpu_running(vcpu) || kvm_vcpu_has_events(vcpu);
> }
>
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu)
> {
> return kvm_vcpu_exiting_guest_mode(vcpu) == IN_GUEST_MODE;
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 890b706..9613620 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -798,6 +798,8 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
> void kvm_arch_hardware_unsetup(void);
> void kvm_arch_check_processor_compat(void *rtn);
> int kvm_arch_vcpu_runnable(struct kvm_vcpu *vcpu);
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu);
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu);
> int kvm_arch_vcpu_should_kick(struct kvm_vcpu *vcpu);
>
> #ifndef __KVM_HAVE_ARCH_VM_ALLOC
> diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c
> index a39a1e1..e45f780 100644
> --- a/virt/kvm/arm/arm.c
> +++ b/virt/kvm/arm/arm.c
> @@ -416,6 +416,16 @@ int kvm_arch_vcpu_runnable(struct kvm_vcpu *v)
> && !v->arch.power_off && !v->arch.pause);
> }
>
> +bool kvm_arch_vcpu_spin_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
> +
> +bool kvm_arch_vcpu_preempt_in_kernel(struct kvm_vcpu *vcpu)
> +{
> + return false;
> +}
Is the differentiation really necessary?
Can't you cache for x86 in all scenarios and simply introduce
kvm_arch_vcpu_in_kernel() ?
Otherwise, we have complexity that might just be avoided (e.g.
kvm_arch_vcpu_spin_in_kernel must only be called on the loaded VCPU)
> +
> /* Just ensure a guest exit from a particular CPU */
> static void exit_vm_noop(void *info)
> {
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index f3f7427..0d0527b 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2324,12 +2324,14 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
> {
> struct kvm *kvm = me->kvm;
> struct kvm_vcpu *vcpu;
> + bool in_kern;
> int last_boosted_vcpu = me->kvm->last_boosted_vcpu;
> int yielded = 0;
> int try = 3;
> int pass;
> int i;
>
> + in_kern = kvm_arch_vcpu_spin_in_kernel(me);
> kvm_vcpu_set_in_spin_loop(me, true);
> /*
> * We boost the priority of a VCPU that is runnable but not
> @@ -2351,6 +2353,8 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
> continue;
> if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
> continue;
> + if (in_kern && !kvm_arch_vcpu_preempt_in_kernel(vcpu))
> + continue;
> if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
> continue;
>
>
--
Thanks,
David
Powered by blists - more mailing lists