From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: [PATCH] kvm: x86: remove vmx_vm_has_apicv() outside of hwapic_isr_update() Date: Fri, 19 Dec 2014 12:12:26 +0100 Message-ID: <5494081A.7020601@redhat.com> References: <1417426124-9685-1-git-send-email-tiejun.chen@intel.com> <547C5477.7030101@redhat.com> <54938E4F.7060105@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Cc: kvm@vger.kernel.org To: "Chen, Tiejun" Return-path: Received: from mx1.redhat.com ([209.132.183.28]:59188 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752380AbaLSLMb (ORCPT ); Fri, 19 Dec 2014 06:12:31 -0500 In-Reply-To: <54938E4F.7060105@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: On 19/12/2014 03:32, Chen, Tiejun wrote: > > Are you saying something below? > > if (enable_apicv) > ... > else { > kvm_x86_ops->hwapic_irr_update = NULL; Yes. > But this means we have to revise hadware_setup() to get 'kvm' inside, This would not even be possible, since hardware_setup() is only called once. However, for the only caller of hwapic_isr_update (but presumably all of them, as is the case for hwapic_irr_update), you already know that irqchip_in_kernel(kvm) is true. You are in kvm_apic_post_state_restore, which takes a kvm_lapic_state, and no lapic exists if !irqchip_in_kernel(kvm). (Yes, irqchip_in_kernel) is a bit weird and tests pic_irqchip(kvm) instead, but it's the same. It tests pic_irqchip(kvm) only because the LAPIC is per-cpu and irqchip_in_kernel takes a struct kvm). So it's possible to NULL out hwapic_isr_update in hardware_setup. It simply shouldn't happen that you call hwapic_isr_update without the in-kernel irqchip. The kernel knows nothing about ISR/IRR without the in-kernel irqchip. Paolo > then rebase other callers to hwapic_isr_update(), is it really good?