Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757696AbcLOObA (ORCPT ); Thu, 15 Dec 2016 09:31:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18103 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753550AbcLOOa6 (ORCPT ); Thu, 15 Dec 2016 09:30:58 -0500 Date: Thu, 15 Dec 2016 15:30:54 +0100 From: Radim =?utf-8?B?S3LEjW3DocWZ?= To: Roman Kagan , Paolo Bonzini , Denis Plotnikov , den@virtuozzo.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] KVM: x86: avoid redundant REQ_EVENT Message-ID: <20161215143054.GC6667@potion> References: <20161214105908.322638-1-dplotnikov@virtuozzo.com> <20161215071840.GB7704@rkaganb.sw.ru> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161215071840.GB7704@rkaganb.sw.ru> X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 15 Dec 2016 14:30:58 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1971 Lines: 43 2016-12-15 10:18+0300, Roman Kagan: > On Wed, Dec 14, 2016 at 11:29:33PM +0100, Paolo Bonzini wrote: >> On 14/12/2016 11:59, Denis Plotnikov wrote: >> > >> > if ((exit_reason == EXIT_REASON_EXTERNAL_INTERRUPT) >> > && nested_exit_intr_ack_set(vcpu)) { >> > - int irq = kvm_cpu_get_interrupt(vcpu); >> > + int irq = kvm_cpu_get_interrupt(vcpu, true); >> > WARN_ON(irq < 0); >> >> I think this is not needed, because all nested vmexits end with a KVM_REQ_EVENT: I also think that it can safely be false and we could drop the parameter from kvm_cpu_get_interrupt(). (We have injected the highest priority interrupt and put it into ISR, raising PPR again to its level, so there should be nothing to do in KVM_REQ_EVENT due to any TPR changes.) >> /* >> * the KVM_REQ_EVENT optimization bit is only on for one entry, and if >> * we did not inject a still-pending event to L1 now because of >> * nested_run_pending, we need to re-enable this bit. >> */ >> if (vmx->nested.nested_run_pending) >> kvm_make_request(KVM_REQ_EVENT, vcpu); > > IIRC .nested_run_pending indicates we're emulating vmlaunch/vmresume and > should not vmexit to L1, so this is not exactly "all nested vmexits"... > >> This would allow you to always pass false from kvm_cpu_get_interrupt to >> kvm_get_apic_interrupt. Not sure if the additional complication in vmx.c >> is worth the simplification in lapic.c. Radim, second opinion? :) This patch goes for a minimal change in the non-nested case, so I would leave nVMX optimizations for another patch. One useless round of KVM_REQ_EVENT is not going change nested performance by much and it is not the only thing we could improve wrt. TPR ... I would just leave it for now and take care of it when we * don't to update PPR at all with APICv -- it is already correct * drop the KVM_REQ_EVENT with flex priority, because lower TPR cannot unmask an interrupt