From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suravee Suthikulanit Subject: Re: [PATCH 5/6] x86: enable multi-vector MSI Date: Fri, 26 Apr 2013 16:16:58 -0500 Message-ID: <517AEECA.7020907@amd.com> References: <51713D8F02000078000CEEE6@nat28.tlf.novell.com> <51713FCF02000078000CEF27@nat28.tlf.novell.com> <5175DC09.8090707@amd.com> <517645D202000078000CFC3C@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: <517645D202000078000CFC3C@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: Konrad Rzeszutek Wilk , Jacob Shin , xiantao.zhang@intel.com, xen-devel List-Id: xen-devel@lists.xenproject.org On 4/23/2013 1:26 AM, Jan Beulich wrote: >>>> On 23.04.13 at 02:55, Suravee Suthikulanit > wrote: >> On 4/19/2013 5:59 AM, Jan Beulich wrote: >>> --- a/xen/arch/x86/msi.c >>> +++ b/xen/arch/x86/msi.c >>> @@ -238,6 +238,11 @@ static int write_msi_msg(struct msi_desc >>> u8 bus = dev->bus; >>> u8 slot = PCI_SLOT(dev->devfn); >>> u8 func = PCI_FUNC(dev->devfn); >>> + int nr = entry->msi_attrib.entry_nr; >>> + >>> + ASSERT((msg->data & (entry[-nr].msi.nvec - 1)) == nr); >>> + if ( nr ) >>> + return 0; >> This logic seems incorrect. Do you meant to write --nr? > No, this indeed has to be -nr (i.e. the "masterkj" entry, which is the > first on in the array. > >> This causes assertion here. Also, investigation showing the >> value of nr is 0 here. > nr being 0 here is perfectly fine, meaning this is the first ("master") > entry of a multi-vector device (it can't be a single-vector one, as in > that case entry[0].msi.nvec == 1, i.e. the & yields zero regardless > of msg->data). > > And the assertion should hold, due to > > *data = (msg->data & ~(INTREMAP_ENTRIES - 1)) | offset; > > in update_intremap_entry_from_msi_msg(), and > alloc_intremap_entry() returning only aligned blocks. > > So the question isn't just what value nr there has, but also what > the other involved values are. > > Jan Ok, thanks for explanation. Do you think you could add comment in the code?kkk It was not quite clear at the beginning why we need this assertion. The problem occurs when the function "xen/driver/passthrough/amd/iommu_init.c: enable_iommu()" trying to initialize IOMMU and calling the "set_msi_affinity", which in turn calling "write_msi_msg". At this point, "nvec" is still zero. So, the following code should fix it. unsigned int nvec = entry[-nr].msi.nvec; if ( nvec > 0 ) ASSERT((msg->data & (nvec - 1)) == nr); Suravee