From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH] AMD IOMMU: Fix an interrupt remapping issue Date: Fri, 08 Apr 2011 14:43:57 +0100 Message-ID: <4D9F2D3D020000780003A9AD@vpn.id2.novell.com> References: <201104081335.36718.wei.wang2@amd.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <201104081335.36718.wei.wang2@amd.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Wei Wang2 Cc: Boris Ostrovsky , Wei Huang2 , "xen-devel@lists.xensource.com" List-Id: xen-devel@lists.xenproject.org >>> On 08.04.11 at 13:35, Wei Wang2 wrote: > Some device could generate bogus interrupts if an IO-APIC RTE and an = iommu=20 > interrupt remapping entry are not consistent during 2 adjacent 64bits = IO-APIC=20 > RTE updates. For example, if the 2nd operation updates destination bits = in=20 > RTE for SATA device and unmask it, in some case, SATA device will = assert=20 > ioapic pin to generate interrupt immediately using new destination but = iommu=20 > could still translate it into the old destination, then dom0 would be=20 > confused. To fix that, we sync up interrupt remapping entry with IO-APIC = IRE=20 > on every 32 bits operation and foward IOAPIC RTE updates after = interrupt=20 > remapping table has been changed.=20 I don't think this is correct: Without the patch, the filling of ioapic_rte= takes into account the value already written. Now that you only write the value at the end of the function, you should overwrite the affected half with "value" immediately before calling update_intremap_entry_from_ioapic(). (Without knowing which half gets written, passing "value" to update_intremap_entry_from_ioapic() is pointless, and indeed the function doesn't use that parameter.) Eliminating the double write if reg =3D=3D rte_lo would also seem = desirable (and in no case should you write back the old value after having called update_intremap_entry_from_ioapic()). Jan