[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On 01/16/2018 06:44 PM, Julien Grall wrote:

On 16/01/18 12:40, Manish Jaggi wrote:
Hi Julien,


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. |


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.