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

Re: [Xen-devel] [v2 4/6] xen/iommu: smmu-v3: Add Xen specific code to enable the ported driver





On 06/02/2018 12:28 AM, Sameer Goel wrote:

On 5/31/2018 11:16 PM, Manish Jaggi wrote:

On 05/31/2018 09:27 PM, Sameer Goel wrote:
On 5/30/2018 10:13 PM, Manish Jaggi wrote:
On 05/31/2018 04:31 AM, Sameer Goel wrote:
+
+static int arm_smmu_iommu_domain_init(struct domain *d)
Where is iommu_domain initialized?
The function does not use a iommu_domain * variable
Please check iommu.c 2 levels up.
In this function do you see iommu_domain getting allocated or initialized?
As per the name of function arm_smmu iommu_domain_init.
Where is init of iommu_domain in this function?
Well without the xen_domain the iommu_domain is not initialized. It is just the 
default value. This generic iommu code makes an .init call to our code for 
whatever initialization is needed. So the name here seemed absolutely fine to 
me.

Initialization does not always refer to allocation. In this case this is driver 
specific initialization. Since, the iommu code is making an init call to the 
smmu code hence the name arm_smmu_iommu_domain_init. So, again I agree with 
your comments on the domain variable names and I'm making these changes as they 
would make the code cleaner. This function name change probably will not do 
much but the move along the discussion, let me know what you were thinking.
Sameer, few points
a. all the functions are prefixed with arm_smmu_ , so what the function is 
doing can be understood by the rest part of the name
In this case it is iommu_domain_init.

b. By the name it seems to suggest that you are  doing some kind of init for 
iommu_domain

c. But in this complete function, iommu_domain pointer is never used.

If I take your point, the appropriate name of the function should be 
arm_smmu_xen_domain_init().
Its not the iommu domain defined within the current driver.  I'll change the 
name to arm_smmu_xen_iommu_init().

arm_smmu for just keeping the prefixes as needed. Sounds good?
yes. thanks.

Thanks,
Sameer
-Manish
+static int arm_smmu_iommu_domain_init(struct domain *d)
+{
+    struct arm_smmu_xen_domain *xen_domain;
+
+    xen_domain = xzalloc(struct arm_smmu_xen_domain);
+    if (!xen_domain)
+        return -ENOMEM;
+
+    spin_lock_init(&xen_domain->lock);
+    INIT_LIST_HEAD(&xen_domain->contexts);
+
+    dom_iommu(d)->arch.priv = xen_domain;
+
+    return 0;
+}
+



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

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

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


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

 


Rackspace

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