From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xiao Guangrong Subject: Re: [PATCH v2] KVM: IOAPIC: only access APIC registers one dword at a time Date: Mon, 05 Jul 2010 11:47:06 +0800 Message-ID: <4C3155BA.4070903@cn.fujitsu.com> References: <4C2D6D4B.5060309@cn.fujitsu.com> <4C2D95EA.6080209@np.css.fujitsu.com> <4C2D975A.2050704@cn.fujitsu.com> <4C2D9C8C.8060401@cn.fujitsu.com> <4C2F0CC1.6020808@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Marcelo Tosatti , LKML , KVM list , Jin Dongming To: Avi Kivity Return-path: Received: from cn.fujitsu.com ([222.73.24.84]:62835 "EHLO song.cn.fujitsu.com" rhost-flags-OK-FAIL-OK-OK) by vger.kernel.org with ESMTP id S1758098Ab0GEDu6 convert rfc822-to-8bit (ORCPT ); Sun, 4 Jul 2010 23:50:58 -0400 In-Reply-To: <4C2F0CC1.6020808@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: Avi Kivity wrote: > On 07/02/2010 11:00 AM, Xiao Guangrong wrote: >> The IOAPIC spec says: >> >> When accessing these registers, accesses must be done one dword at a >> time. >> For example, software should never access byte 2 from the Data >> register before >> accessing bytes 0 and 1. The hardware will not attempt to recover fr= om >> a bad >> programming model in this case. >> >> So, this patch removes other width access >> >> =20 >=20 > The ioapic code also implements the ia64 iosapic. I'm guessing that > does support 64-bit accesses. Please check the iosapic documentation= =2E >=20 The iosapic also using 32-bit to access registers: All registers are accessed using 32-bit uncacheable loads and stores to= a reserved memory location in system memory. This implies that to modify a field (e.g., a bit or a= byte) in any register, the whole 32-bit register must be read, the field modified, and the 32 bits= written back. Partial register access, or non-aligned register access, are implementation-defined by t= he I/O xAPIC and will not be compatible across different implementations. Also, registers that ar= e described as 64 bits wide are accessed as multiple independent 32-bit registers. [ From << Intel=C2=AE Itanium=C2=AE Processor Family Interrupt Architec= ture Guide >>, P2-6 ] > There might be guests that use incorrect access despite the > documentation; if real hardware supports it, it should work. So we n= eed > to start with just a warning, and allow the access. Later we can dro= p > the invalid access. If the OS contravene the spec, i thinks it's the OS's bug, also, i have= tested some versions windows/linux guests, it's no broken, can we directly drop the other wi= de access? >=20 >> @@ -288,6 +288,11 @@ static int ioapic_mmio_read(struct kvm_io_devic= e >> *this, gpa_t addr, int len, >> ioapic_debug("addr %lx\n", (unsigned long)addr); >> ASSERT(!(addr& 0xf)); /* check alignment */ >> >> + if (len !=3D 4) { >> + printk(KERN_WARNING "ioapic: wrong length %d\n", len); >> + return 0; >> + } >> + >> =20 >=20 > Guest triggered, so needs to be rate limited. Yeah, will using printk_ratelimit cooperate with it.