Re: [PATCH v2 01/11] vt-d: fix the IOMMU flush issue

From: "Xu, Quan" <quan.xu@intel.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: "Tian, Kevin" <kevin.tian@intel.com>, Keir Fraser <keir@xen.org>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	"dario.faggioli@citrix.com" <dario.faggioli@citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	"Wu, Feng" <feng.wu@intel.com>
Subject: Re: [PATCH v2 01/11] vt-d: fix the IOMMU flush issue
Date: Tue, 26 Apr 2016 10:58:06 +0000	[thread overview]
Message-ID: <945CA011AD5F084CBEA3E851C0AB28894B89474A@SHSMSX101.ccr.corp.intel.com> (raw)
In-Reply-To: <571F649B02000078000E5C5B@prv-mh.provo.novell.com>

On April 26, 2016 6:53 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >>> On 26.04.16 at 12:15, <quan.xu@intel.com> wrote:
> > On April 26, 2016 5:11 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >> >>> On 26.04.16 at 04:18, <quan.xu@intel.com> wrote:
> >> > On April 25, 2016 5:22 PM, Jan Beulich <JBeulich@suse.com> wrote:
> >> >> >>> On 18.04.16 at 16:00, <quan.xu@intel.com> wrote:
> >> >> > --- a/xen/drivers/passthrough/vtd/iommu.c
> >> >> > +++ b/xen/drivers/passthrough/vtd/iommu.c
> >> >> > @@ -558,14 +558,16 @@ static void iommu_flush_all(void)
> >> >> >      }
> >> >> >  }
> >> >> >
> >> >> > -static void __intel_iommu_iotlb_flush(struct domain *d,
> >> >> > unsigned long
> >> gfn,
> >> >> > -        int dma_old_pte_present, unsigned int page_count)
> >> >> > +static int iommu_flush_iotlb(struct domain *d, unsigned long gfn,
> >> >> > +                             int dma_old_pte_present,
> >> >> > +                             unsigned int page_count)
> >> >> >  {
> >> >> >      struct hvm_iommu *hd = domain_hvm_iommu(d);
> >> >> >      struct acpi_drhd_unit *drhd;
> >> >> >      struct iommu *iommu;
> >> >> >      int flush_dev_iotlb;
> >> >> >      int iommu_domid;
> >> >> > +    int rc = 0;
> >> >>
> >> >> Pointless initializer.
> >> >
> >> > I am afraid not.
> >> > In case I don't initialize 'rc', the compiler prints out  " error:
> >> > 'rc' may be used uninitialized in this function
> > [-Werror=maybe-uninitialized]".
> >>
> >> Looking at the patch again I can't see why that would be the case.
> >> Are you certain this isn't a result of subsequent patches, IOW did
> >> you try
> > this
> >> with just this one patch applied?
> >
> > Yes, I test it with this only patch again.
> >
> > The same result.
> >
> >> rc gets initialized in both the if() and the else branches, and
> >> there's no label allowing that initialization to be
> > bypassed...
> >
> > look at this function again,
> >
> >>> func
> > iommu_flush_iotlb()
> > {
> >     int rc;
> >
> >     for_each_drhd_unit ( drhd )
> >     {
> >         if ()
> >             rc ..
> >         else
> >             rc ..
> >     }
> >
> >     return rc;
> > }
> 
> Ah, I'm sorry - the patch context didn't make it obvious this is in a loop.
> 

Never mind. It is _not_ quite as obvious, but compiler helps us.
Quan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel