Re: [Xen-devel] [RFC v4 8/8] drivers/passthrough/arm: Refactor code for arm smmu drivers

[Manish Jaggi <mjaggi@xxxxxxxxxxxxxxxxxx>]

On 01/16/2018 06:44 PM, Julien Grall wrote:
> On 16/01/18 12:40, Manish Jaggi wrote:
> > Hi Julien,
> Hi,
>
> > On 01/16/2018 02:11 AM, Julien Grall wrote:
> > > On 01/03/2018 05:34 AM, Manish Jaggi wrote:
> > > > Hi Sameer,
> > > Hi Manish,
> > >
> > > > > +    unsigned int            type;
> > > > > +
> > > > > +    /* Dummy compatibility defines */
> > > > > +    unsigned long pgsize_bitmap;
> > > > > +    struct iommu_domain_geometry geometry;
> > > > > +
> > > > > +    atomic_t ref;
> > > > > +    /* Used to link iommu_domain contexts for a same domain.
> > > > > +     * There is at least one per-SMMU to used by the domain.
> > > > > +     */
> > > > > +    struct list_head        list;
> > > > > +};
> > > > > +
> > > > > +/* Xen: Describes information required for a Xen domain */
> > > > > +struct arm_smmu_xen_domain {
> > > > > +    spinlock_t          lock;
> > > > > +    /* List of iommu domains associated to this domain */
> > > > > +    struct list_head        contexts;
> > > > Could we use a more verbose name, How about 
> > > > %s/contexts/iommu_domain_contexts/g ?
> > > How about a much more verbose name...? This name is 21 letters and 
> > > that's only for the field. Just imagine with the variable name 
> > > before and a couple of indentation.
> > How about io_context? anything which makes it more verbose is ok with 
> > me.
> I much prefer to stick with "contexts".
I am not able to understand why to use contexts for a list which has 
iommu_domain pointers.
If you are ok with this logic, we can rename all iommu_domain pointer 
variables in this file as context.
It is not intuitive to use context for iommu_domain in the same file it 
is confusing.
smmu code in xen is not easy to read.
There are other places which confuse...

|struct arm_smmu_xen_domain *xen_domain = dom_iommu(d)->arch.priv; What 
is the first impression from this variable xen_domain, does it refer to 
a VM ? It is confusing. |
> Cheers,

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