From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH v3 3/4] KVM: x86: Add EOI exit bitmap inference
Date: Mon, 08 Jun 2015 16:15:03 +0200
Message-ID: <5575A367.1060803@redhat.com>
References: <1433289107-20638-1-git-send-email-srutherford@google.com> <1433289107-20638-3-git-send-email-srutherford@google.com> <BLU436-SMTP236C4B4413A76BF251B434380BF0@phx.gbl>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
To: Wanpeng Li <wanpeng.li@hotmail.com>,
	Steve Rutherford <srutherford@google.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-wi0-f182.google.com ([209.85.212.182]:35849 "EHLO
	mail-wi0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753109AbbFHOPG (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 8 Jun 2015 10:15:06 -0400
Received: by wigg3 with SMTP id g3so54040364wig.1
        for <kvm@vger.kernel.org>; Mon, 08 Jun 2015 07:15:05 -0700 (PDT)
In-Reply-To: <BLU436-SMTP236C4B4413A76BF251B434380BF0@phx.gbl>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 08/06/2015 12:33, Wanpeng Li wrote:
>> +        if (kvm_check_request(KVM_REQ_SCAN_IOAPIC, vcpu)) {
>> +            if (irqchip_split(vcpu->kvm)) {
>> +                memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
>> +                kvm_scan_ioapic_routes(
>> +                    vcpu, vcpu->arch.eoi_exit_bitmaps);
>> +                kvm_x86_ops->load_eoi_exitmap(
>> +                    vcpu, vcpu->arch.eoi_exit_bitmaps);
> 
> How about introduce a handler to fold above just like vcpu_scan_ioapic() ?

Or just move the "if" inside the existing vcpu_scan_ioapic().  This is a
good suggestion because it raises more question:

1) should the

        if (!kvm_apic_hw_enabled(vcpu->arch.apic))
                return;

of vcpu_scan_ioapic apply to the "split irqchip" case too?

2) the "non-split irqchip" can also use vcpu->arch.eoi_exit_bitmaps
instead of the local eoi_exit_bitmap variable of vcpu_scan_ioapic.

So in the end the patched vcpu_scan_ioapic becomes

	if (kvm_apic_hw_enabled(vcpu->arch.apic))
                return;

	memset(vcpu->arch.eoi_exit_bitmaps, 0, 32);
	if (irqchip_split)
		scan_ioapic_routes(...)
	else {
		memset(tmr, 0, 32)
		kvm_ioapic_scan_entry(...)
		kvm_apic_update_tmr(vcpu, tmr);
	}
	kvm_x86_ops->load_eoi_exitmap(vcpu,
				      vcpu->arch.eoi_exit_bitmaps);

A FIXME/TODO comment about TMR in the split-irqchip case is probably in
order too.

Paolo