Skip to contentSkip to conversation

xen/arm: smmu: Renaming arm_smmu_xen_domain with, domain_iommu_info

PATCH NAK Manish Jaggi 3 participants 5 messages Mar 27, 2015

lore
Share & tools
lore
#0 lore raw

PATCH 2/3 xen/arm: smmu: Renaming arm_smmu_xen_domain with, domain_iommu_info

The name arm_smmu_xen_domain was mapped to domain_hvm_iommu(d)->arch.priv. Also there are a lot of datastructre in smmu.c with name ending in domain it is not intuitive and code is hard to understand.

domain_iommu_info is easy to understand that it refers to domain_hvm_iommu(d)->arch.priv.

The instances of arm_smmu_xen_domain were name smmu_domain or xen_domain in different functions which is also cleaned up in this patch.

---
 xen/drivers/passthrough/arm/smmu.c | 46 +++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 23 deletions(-)
xen/drivers/passthrough/arm/smmu.c +23 -23
index ab4f7a4..fe0549e 100644
@@ -230,7 +230,7 @@ struct iommu_domain
 };
 
 /* Xen: Describes informations required for a Xen domain */
-struct arm_smmu_xen_domain {
+struct domain_iommu_info {
 	spinlock_t			lock;
 	/* List of context (i.e iommu_domain) associated to this domain */
 	struct list_head		contexts;
@@ -2540,11 +2540,11 @@ static u32 platform_features = ARM_SMMU_FEAT_COHERENT_WALK;
 
 static void arm_smmu_iotlb_flush_all(struct domain *d)
 {
-	struct arm_smmu_xen_domain *smmu_domain = domain_hvm_iommu(d)->arch.priv;
+	struct domain_iommu_info *domain_iommu_info = domain_hvm_iommu(d)->arch.priv;
 	struct iommu_domain *cfg;
 
-	spin_lock(&smmu_domain->lock);
-	list_for_each_entry(cfg, &smmu_domain->contexts, list) {
+	spin_lock(&domain_iommu_info->lock);
+	list_for_each_entry(cfg, &domain_iommu_info->contexts, list) {
 		/*
 		 * Only invalidate the context when SMMU is present.
 		 * This is because the context initialization is delayed
@@ -2554,7 +2554,7 @@ static void arm_smmu_iotlb_flush_all(struct domain *d)
 			continue;
 		arm_smmu_tlb_inv_context(cfg->priv);
 	}
-	spin_unlock(&smmu_domain->lock);
+	spin_unlock(&domain_iommu_info->lock);
 }
 
 static void arm_smmu_iotlb_flush(struct domain *d, unsigned long gfn,
@@ -2568,10 +2568,10 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
 			       struct device *dev)
 {
 	struct iommu_domain *domain;
-	struct arm_smmu_xen_domain *xen_domain;
+	struct domain_iommu_info *domain_iommu_info;
 	int ret;
 
-	xen_domain = domain_hvm_iommu(d)->arch.priv;
+	domain_iommu_info = domain_hvm_iommu(d)->arch.priv;
 
 	if (!dev->archdata.iommu) {
 		dev->archdata.iommu = xzalloc(struct device_iommu_info);
@@ -2604,10 +2604,10 @@ static int arm_smmu_assign_dev(struct domain *d, u8 devfn,
 	if (ret)
 		goto err_attach_dev;
 
-	spin_lock(&xen_domain->lock);
+	spin_lock(&domain_iommu_info->lock);
 	/* Chain the new context to the domain */
-	list_add(&domain->list, &xen_domain->contexts);
-	spin_unlock(&xen_domain->lock);
+	list_add(&domain->list, &domain_iommu_info->contexts);
+	spin_unlock(&domain_iommu_info->lock);
 
 	return 0;
 
@@ -2622,9 +2622,9 @@ err_dom_init:
 static int arm_smmu_deassign_dev(struct domain *d, struct device *dev)
 {
 	struct iommu_domain *domain = dev_iommu_domain(dev);
-	struct arm_smmu_xen_domain *xen_domain;
+	struct domain_iommu_info *domain_iommu_info;
 
-	xen_domain = domain_hvm_iommu(d)->arch.priv;
+	domain_iommu_info = domain_hvm_iommu(d)->arch.priv;
 
 	if (!domain || domain->priv->cfg.domain != d) {
 		dev_err(dev, " not attached to domain %d\n", d->domain_id);
@@ -2633,9 +2633,9 @@ static int arm_smmu_deassign_dev(struct domain *d, struct device *dev)
 
 	arm_smmu_detach_dev(domain, dev);
 
-	spin_lock(&xen_domain->lock);
+	spin_lock(&domain_iommu_info->lock);
 	list_del(&domain->list);
-	spin_unlock(&xen_domain->lock);
+	spin_unlock(&domain_iommu_info->lock);
 
 	arm_smmu_domain_destroy(domain);
 	xfree(domain);
@@ -2664,16 +2664,16 @@ static int arm_smmu_reassign_dev(struct domain *s, struct domain *t,
 
 static int arm_smmu_iommu_domain_init(struct domain *d)
 {
-	struct arm_smmu_xen_domain *xen_domain;
+	struct domain_iommu_info *domain_iommu_info;
 
-	xen_domain = xzalloc(struct arm_smmu_xen_domain);
-	if ( !xen_domain )
+	domain_iommu_info = xzalloc(struct domain_iommu_info);
+	if ( !domain_iommu_info )
 		return -ENOMEM;
 
-	spin_lock_init(&xen_domain->lock);
-	INIT_LIST_HEAD(&xen_domain->contexts);
+	spin_lock_init(&domain_iommu_info->lock);
+	INIT_LIST_HEAD(&domain_iommu_info->contexts);
 
-	domain_hvm_iommu(d)->arch.priv = xen_domain;
+	domain_hvm_iommu(d)->arch.priv = domain_iommu_info;
 
 	/* Coherent walk can be enabled only when all SMMUs support it. */
 	if (platform_features & ARM_SMMU_FEAT_COHERENT_WALK)
@@ -2688,10 +2688,10 @@ static void __hwdom_init arm_smmu_iommu_hwdom_init(struct domain *d)
 
 static void arm_smmu_iommu_domain_teardown(struct domain *d)
 {
-	struct arm_smmu_xen_domain *xen_domain = domain_hvm_iommu(d)->arch.priv;
+	struct domain_iommu_info *domain_iommu_info = domain_hvm_iommu(d)->arch.priv;
 
-	ASSERT(list_empty(&xen_domain->contexts));
-	xfree(xen_domain);
+	ASSERT(list_empty(&domain_iommu_info->contexts));
+	xfree(domain_iommu_info);
 }
 
 static int arm_smmu_map_page(struct domain *d, unsigned long gfn,
Signature · 1 line
1.9.1
#1 lore raw

Hi Manish,

Quoted 2 lines · Manish Jaggi

The name arm_smmu_xen_domain was mapped to domain_hvm_iommu(d)->arch.priv. Also there are a lot of datastructre in smmu.c with name ending in domain

data structure.

Quoted 1 line

it is not intuitive and code is hard to understand.

Now it's more difficult to differentiate domain_iommu_info and iommu_domain.

Regards,

Regards,

Signature · 1 line
Julien Grall
#2 lore raw

Regards,
Manish Jaggi

________________________________________
From: Julien Grall <[email protected]>
Sent: Friday, March 27, 2015 6:32 PM
To: Jaggi, Manish; Xen Devel; [email protected]; Kumar, Vijaya; Ian Campbell; Stefano Stabellini
Subject: Re: [PATCH v1 2/3] xen/arm: smmu: Renaming arm_smmu_xen_domain with, domain_iommu_info

Hi Manish,

Quoted 2 lines · Manish Jaggi

The name arm_smmu_xen_domain was mapped to domain_hvm_iommu(d)->arch.priv. Also there are a lot of datastructre in smmu.c with name ending in domain

data structure.

Quoted 1 line

it is not intuitive and code is hard to understand.

Now it's more difficult to differentiate domain_iommu_info and iommu_domain.

[manish] Please see my previous mail. Also please pick any other name there are so many data structures which end up in domain. What is the difference between arm_smmu_domain and arm_smmu_xen_domain, I cannot find it based on name.

Regards,

Regards,

--
Julien Grall

#3 lore raw
Quoted 3 lines · Manish

Now it's more difficult to differentiate domain_iommu_info and iommu_domain.

[manish] Please see my previous mail. Also please pick any other name there are so many data structures which end up in domain. What is the difference between arm_smmu_domain and arm_smmu_xen_domain, I cannot find it based on name.

Again, did you read the common above the structure?

How do you different domain_iommu_info vs iommu_domain?

Regards,

Signature · 1 line
Julien Grall
#4 lore raw

Regards,
Manish Jaggi

________________________________________
From: Julien Grall <[email protected]>
Sent: Friday, March 27, 2015 7:06 PM
To: Jaggi, Manish; Xen Devel; [email protected]; Kumar, Vijaya; Ian Campbell; Stefano Stabellini
Subject: Re: [PATCH v1 2/3] xen/arm: smmu: Renaming arm_smmu_xen_domain with, domain_iommu_info

Quoted 3 lines · Manish

Now it's more difficult to differentiate domain_iommu_info and iommu_domain.

[manish] Please see my previous mail. Also please pick any other name there are so many data structures which end up in domain. What is the difference between arm_smmu_domain and arm_smmu_xen_domain, I cannot find it based on name.

Again, did you read the common above the structure? [manish] yes but naming is a bit confusing and need to be fixed

How do you different domain_iommu_info vs iommu_domain?
[manish] ok lets change it to domain_iommu_priv. here domain refers to a xen VM domain. While iommu_domain refers to a iommu domain within a particular smmu.

Regards,

--
Julien Grall