From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH] KVM: x86: sync old tmr with ioapic to update
Date: Tue, 26 Aug 2014 12:58:56 +0200
Message-ID: <53FC6870.3050303@redhat.com>
References: <1409070441-31239-1-git-send-email-wei.w.wang@intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Cc: alex.williamson@redhat.com, Yang Zhang <yang.z.zhang@intel.com>
To: Wei Wang <wei.w.wang@intel.com>, kvm@vger.kernel.org
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:7763 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1754305AbaHZK7A (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 26 Aug 2014 06:59:00 -0400
In-Reply-To: <1409070441-31239-1-git-send-email-wei.w.wang@intel.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

Il 26/08/2014 18:27, Wei Wang ha scritto:
> kvm_ioapic_scan_entry() needs to update tmr. The previous lapic tmr value
> (old_tmr) needs to sync with ioapic to get an accurate updated tmr
> value before the updating work.

> +	u32 irr[8];
> +	u32 isr[8];
> +	u32 old_tmr[8];

These do not need to be arrays.  You can use a single variable for each
of irr, isr and old_tmr.

>  	int i;
>  
> -	for (i = 0; i < 8; i++)
> +	for (i = 0; i < 8; i++) {
> +		irr[i] = kvm_apic_get_reg(apic, APIC_IRR + 0x10 * i);
> +		isr[i] = kvm_apic_get_reg(apic, APIC_ISR + 0x10 * i);
> +		old_tmr[i] = kvm_apic_get_reg(apic, APIC_TMR + 0x10 * i);
> +		tmr[i] = (~(irr[i] | isr[i]) & tmr[i])
> +				| ((irr[i] | isr[i]) & old_tmr[i]);

Please do not modify the tmr array.  Instead, use kvm_apic_get_reg in
the next patch to read the TMR.

Also, please add a comment about how the code works and why the old TMR
is used if IRR=1 || ISR=1.

Paolo

>  		apic_set_reg(apic, APIC_TMR + 0x10 * i, tmr[i]);