From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753467AbcCILKj (ORCPT ); Wed, 9 Mar 2016 06:10:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49673 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753156AbcCILKc (ORCPT ); Wed, 9 Mar 2016 06:10:32 -0500 Subject: Re: [PART1 RFC v2 06/10] svm: Add interrupt injection via AVIC To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <1457124368-2025-1-git-send-email-Suravee.Suthikulpanit@amd.com> <1457124368-2025-7-git-send-email-Suravee.Suthikulpanit@amd.com> <56DD9FF5.7010201@redhat.com> <20160308215424.GA31328@potion.brq.redhat.com> Cc: Suravee Suthikulpanit , joro@8bytes.org, bp@alien8.de, gleb@kernel.org, alex.williamson@redhat.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, wei@redhat.com, sherry.hurwitz@amd.com From: Paolo Bonzini Message-ID: <56E004A2.70702@redhat.com> Date: Wed, 9 Mar 2016 12:10:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160308215424.GA31328@potion.brq.redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/03/2016 22:54, Radim Krčmář wrote: > 2016-03-07 16:36+0100, Paolo Bonzini: >> On 04/03/2016 21:46, Suravee Suthikulpanit wrote: >>> +static void svm_deliver_avic_intr(struct kvm_vcpu *vcpu, int vec) >>> +{ >>> + struct vcpu_svm *svm = to_svm(vcpu); >>> + >>> + kvm_lapic_set_vector(vec, avic_get_bk_page_entry(svm, APIC_IRR)); > > (I think that smp_mb here would make sense, even though we're fine now > thanks to re-checking vcpu->mode in kvm_vcpu_kick. Right, though only a smp_mb__after_atomic() is required (which is a compiler barrier). It is similarly required in vmx. Offtopic remark, kvm_make_request() and kvm_check_request() should probably have a smp_wmb() and a smp_rmb(). >>> + >>> + if (vcpu->mode == IN_GUEST_MODE) { >>> + wrmsrl(SVM_AVIC_DOORBELL, >>> + __default_cpu_present_to_apicid(vcpu->cpu)); >>> + } else { >>> + kvm_vcpu_kick(vcpu); >>> + } >> >> You also need to add >> >> kvm_make_request(KVM_REQ_EVENT, vcpu); >> >> before the "if", similar to vmx_deliver_posted_interrupt. > > KVM won't do anything in KVM_REQ_EVENT and I think that the request can > be avoided because KVM already has to handle IRR writes from AVIC. Doh, I've made the same remark already and you've given the same answer. :) > And what about > [...] > else if (!vcpu->...->is_running) > kvm_vcpu_kick(vcpu); > > ? > The kick isn't needed unless the VCPU is scheduled out. > Or maybe just > if (vcpu->...->is_running) > wrmsrl() > else > kvm_vcpu_kick(); > ? > Which doesn't use the information we have on top AVIC, making our logic > a bit simpler. Yes, both of this should work. I like the latter. In any case, the smp_mb__after_atomic() is necessary. Paolo