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

["Xu, Quan" <quan.xu@xxxxxxxxx>]

On April 26, 2016 6:53 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >>> On 26.04.16 at 12:15, <quan.xu@xxxxxxxxx> wrote:
> > On April 26, 2016 5:11 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> >>> On 26.04.16 at 04:18, <quan.xu@xxxxxxxxx> wrote:
> >> > On April 25, 2016 5:22 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
> >> >> >>> On 18.04.16 at 16:00, <quan.xu@xxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel