Re: [Xen-devel] [PATCH V2 4/6] iommu/arm: Add lightweight iommu_fwspec support

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 8/13/19 4:17 PM, Oleksandr wrote:
> On 13.08.19 15:39, Julien Grall wrote:
> > xfree is able to deal with NULL pointer, so the check is not necessary.
> Yes, the reason I left this check is to not perform an extra operation 
> (dev_iommu_fwspec_set). Shall I drop this check anyway?
I can't see any issue to do the extra operation. This is not hotpath and 
it is harmless.

> > > diff --git a/xen/include/asm-arm/iommu.h b/xen/include/asm-arm/iommu.h
> > > index 20d865e..1853bd9 100644
> > > --- a/xen/include/asm-arm/iommu.h
> > > +++ b/xen/include/asm-arm/iommu.h
> > > @@ -14,6 +14,8 @@
> > >   #ifndef __ARCH_ARM_IOMMU_H__
> > >   #define __ARCH_ARM_IOMMU_H__
> > >   +#include <asm/iommu_fwspec.h>
> > iommu.h does not seem to use anything defined in iommu_fwspec.h. So 
> > why do you include it here?
> I was thinking that every source which includes iommu.h will get 
> iommu_fwspec.h included indirectly. No need to include iommu_fwspec.h in 
> many sources.
> This was a reason. Shall I included it directly where needed?
There are a few cases where iommu.h is required but not iommu_fwspec.h. 
In general, I would prefer if headers are only included when strictly 
necessary.
Cheers,

--
Julien Grall

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