From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH v3 0/3] x86/IOMMU: multi-vector MSI prerequisites Date: Mon, 22 Apr 2013 18:58:18 -0500 Message-ID: <5175CE9A.3020900@amd.com> References: <5167FBAE02000078000CCB19@nat28.tlf.novell.com> <5168B200.8020906@amd.com> <516C2E2A02000078000CD423@nat28.tlf.novell.com> <516C277A.8030308@amd.com> <516D0F1702000078000CD71E@nat28.tlf.novell.com> <51702C9B.6000600@amd.com> <51710E9502000078000CED3E@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51710E9502000078000CED3E@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: Jacob Shin , xiantao.zhang@intel.com, xen-devel List-Id: xen-devel@lists.xenproject.org On 4/19/2013 2:29 AM, Jan Beulich wrote: >>>> On 18.04.13 at 19:25, Suravee Suthikulanit > wrote: >> On 4/16/2013 1:43 AM, Jan Beulich wrote: >>>> On another topic, in arch/x86/msi.c, in the function >>>>> "setup_msi_affinity()", the code does: >>>>> 1. "read_msi_msg" >>>>> 2. Modify the affitity mask >>>>> 3. "write_msi_msg" back the register value. >>>>> >>>>> In read, if the interrupt remapping is enabled, from the patch, the >>>>> function returns the MSI data with remapped information from IOMMU. Then >>>>> in write, if the interrupt remapping is enabled, the function will >>>>> update the IOMMU interrupt remapping entries with the already "remapped" >>>>> vector. In this case, you would be updating the incorrect IOMMU IRTE. >>> Where did you spot that? >> This is in xen/arch/x86/msi.c > That's not precise enough, the more that the same model has > been working for VT-d for a long time. > > Are you perhaps getting confused by the slightly odd way > things get stored/passed: write_msi_msg() specifically asserts > that "msg" doesn't point to the stored version (entry->msg), i.e. > the modification done to *msg by iommu_update_ire_from_msi() > won't be used as input on a subsequent invocation. > >>> To prevent this from happening is exactly >>> why amd_iommu_read_msi_from_ire() isn't empty anymore (this is >>> where the original MSI message information gets reconstructed - or >>> at least is intended to be). The only modification done by >>> update_intremap_entry_from_msi_msg() are the low 11 data bits, >>> and that's what gets overwritten upon read. >> Sorry, I am not quite following this. Why do we need to reconstruct MSI >> message? Why was not it required in the past? > Previously the write path didn't modify the message, and hence > the read path didn't need to reconstruct the original. With the > switch to allocating IRTEs (rather than calculating the used one > from vector and delivery mode), the model now matches VT-d's, > and hence the behavior also needs to be adjusted accordingly. Thanks for clarification. I misunderstood the code that manage the interrupt remapping entry. Sorry for confusion. > > Anyway - did you make any progress towards identifying the > problems with the USB controller? I didn't see any more complete > output, so I have nothing to work with to find where the > problem is. In any case I'm going to post the full multi-vector > MSI series later today, as it's working fine for me on VT-d. > > Jan Some how I could no longer reproduce this case. I'll keep an eye on this for the future. Suravee > >