Re: [Xen-devel] [PATCH 1/2] amd-iommu: replace occurrences of bool_t with bool

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 26 November 2018 09:39
> To: Paul Durrant <Paul.Durrant@xxxxxxxxxx>
> Cc: Brian Woods <brian.woods@xxxxxxx>; Suravee Suthikulpanit
> <suravee.suthikulpanit@xxxxxxx>; xen-devel <xen-
> devel@xxxxxxxxxxxxxxxxxxxx>
> Subject: Re: [Xen-devel] [PATCH 1/2] amd-iommu: replace occurrences of
> bool_t with bool
> 
> >>> On 26.11.18 at 10:05, <paul.durrant@xxxxxxxxxx> wrote:
> > @@ -123,13 +123,13 @@ static bool_t set_iommu_pde_present(u32 *pde,
> unsigned long next_mfn,
> >      return need_flush;
> >  }
> >
> > -static bool_t set_iommu_pte_present(unsigned long pt_mfn, unsigned long
> dfn,
> > -                                    unsigned long next_mfn, int
> pde_level,
> > -                                    bool_t iw, bool_t ir)
> > +static bool set_iommu_pte_present(unsigned long pt_mfn, unsigned long
> dfn,
> > +                                  unsigned long next_mfn, int
> pde_level,
> > +                                  bool iw, bool ir)
> >  {
> >      u64 *table;
> >      u32 *pde;
> > -    bool_t need_flush = 0;
> > +    bool need_flush;
> 
> You validly drop the initializer here (even if this makes the "no
> functional change" assertion un-obvious without looking at the
> entire function), but you don't do so in update_paging_mode().
> Is there any particular reason?

No, I just missed it.

> 
> > @@ -347,16 +347,16 @@ static void set_pde_count(u64 *pde, unsigned int
> count)
> >  /* Return 1, if pages are suitable for merging at merge_level.
> >   * otherwise increase pde count if mfn is contigous with mfn - 1
> >   */
> > -static int iommu_update_pde_count(struct domain *d, unsigned long
> pt_mfn,
> > -                                  unsigned long dfn, unsigned long mfn,
> > -                                  unsigned int merge_level)
> > +static bool iommu_update_pde_count(struct domain *d, unsigned long
> pt_mfn,
> > +                                   unsigned long dfn, unsigned long
> mfn,
> > +                                   unsigned int merge_level)
> >  {
> >      unsigned int pde_count, next_level;
> >      unsigned long first_mfn;
> >      u64 *table, *pde, *ntable;
> >      u64 ntable_maddr, mask;
> >      struct domain_iommu *hd = dom_iommu(d);
> > -    bool_t ok = 0;
> > +    bool ok = false;
> 
> There's "ok = 1" downwards in this function, which I think you
> should adjust as well.

Yes, indeed.

> 
> With these adjustments
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 

Thanks. Are you happy to fix up on commit or would you like a v2?

  Paul

> Jan
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel