Subject: Re: [PATCH] KVM: x86: ioapic: Clear IRR for rtc bit when rtc EOI
 gotten
To: Liran Alon <LIRAN.ALON@ORACLE.COM>, Gonglei <arei.gonglei@huawei.com>,
        rkrcmar@redhat.com
Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        weidong.huang@huawei.com
References: <1513254206-25344-1-git-send-email-arei.gonglei@huawei.com>
 <5A327636.3050307@ORACLE.COM>
From: Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <c717aa9a-390c-4f8b-76f0-6754a02a7176@redhat.com>
Date: Thu, 14 Dec 2017 14:18:32 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101
 Thunderbird/52.5.0
MIME-Version: 1.0
In-Reply-To: <5A327636.3050307@ORACLE.COM>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1391
Lines: 30

On 14/12/2017 14:01, Liran Alon wrote:
>> But in our test, we found that there is a possible situation that Vcpu
>> fails to read
>> RTC_REG_C to clear irq, This could happens while two VCpus are
>> writing/reading
>> registers at the same time, for example, vcpu 0 is trying to read
>> RTC_REG_C,
>> so it write RTC_REG_C first, where the s->cmos_index will be RTC_REG_C,
>> but before it tries to read register C, another vcpu1 is going to read
>> RTC_YEAR,
>> it changes s->cmos_index to RTC_YEAR by a writing action.
>> The next operation of vcpu0 will be lead to read RTC_YEAR, In this
>> case, we will miss
>> calling qemu_irq_lower(s->irq) to clear the irq. After this, kvm will
>> never inject RTC irq,
>> and Windows VM will hang.
> 
> If I understood correctly, this looks to me like a race-condition bug in
> the Windows guest kernel. In real-hardware this race-condition will also
> cause the RTC_YEAR to be read instead of RTC_REG_C.
> Guest kernel should make sure that 2 CPUs does not attempt to read a
> CMOS register in parallel as they can override each other's cmos_index.
> 
> See for example how Linux kernel makes sure to avoid such kind of issues
> in rtc_cmos_read() (arch/x86/kernel/rtc.c) by grabbing a cmos_lock.

Lei and I looked at it further, and the root cause is not the missed EOI
in QEMU.  Rather it's a bug in ioapic.c's tracking of RTC interrupts.

Paolo