On 6/30/22 4:28 PM, Tian, Kevin wrote:
From: Lu Baolu <baolu...@linux.intel.com>
Sent: Saturday, June 25, 2022 8:52 PM
+struct iommu_domain_info {
+ struct intel_iommu *iommu;
+ unsigned int refcnt;
+ u16 did;
+};
+
struct dmar_domain {
int nid; /* node id */
-
- unsigned int iommu_refcnt[DMAR_UNITS_SUPPORTED];
- /* Refcount of devices per iommu */
-
-
- u16 iommu_did[DMAR_UNITS_SUPPORTED];
- /* Domain ids per IOMMU. Use u16
since
- * domain ids are 16 bit wide
according
- * to VT-d spec, section 9.3 */
It'd make sense to keep the comments when moving above fields.
Sure. Updated.
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 56c3d1a9e155..fae45bbb0c7f 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -527,8 +527,10 @@ struct context_entry {
struct iommu_domain_info {
struct intel_iommu *iommu;
- unsigned int refcnt;
- u16 did;
+ unsigned int refcnt; /* Refcount of devices per iommu */
+ u16 did; /* Domain ids per IOMMU. Use u16
since
+ * domain ids are 16 bit wide
according
+ * to VT-d spec, section 9.3 */
};
+ spin_lock(&iommu->lock);
+ curr = xa_load(&domain->iommu_array, iommu->seq_id);
+ if (curr) {
+ curr->refcnt++;
+ kfree(info);
+ goto success;
Not a fan of adding a label for success. Just putting two lines (unlock+
return) here is clearer.
Updated.
+ ret = xa_err(xa_store(&domain->iommu_array, iommu->seq_id,
+ info, GFP_ATOMIC));
check xa_err in separate line.
Sure. Updated as below:
@@ -1778,13 +1780,14 @@ static int domain_attach_iommu(struct
dmar_domain *domain,
info->did = num;
info->iommu = iommu;
domain->nid = iommu->node;
- ret = xa_err(xa_store(&domain->iommu_array, iommu->seq_id,
- info, GFP_ATOMIC));
- if (ret)
+ curr = xa_cmpxchg(&domain->iommu_array, iommu->seq_id,
+ NULL, info, GFP_ATOMIC);
+ if (curr) {
+ ret = xa_err(curr) ? : -EBUSY;
goto err_clear;
+ }
domain_update_iommu_cap(domain);
static void domain_detach_iommu(struct dmar_domain *domain,
struct intel_iommu *iommu)
{
- int num;
+ struct iommu_domain_info *info;
spin_lock(&iommu->lock);
- domain->iommu_refcnt[iommu->seq_id] -= 1;
- if (domain->iommu_refcnt[iommu->seq_id] == 0) {
- num = domain->iommu_did[iommu->seq_id];
- clear_bit(num, iommu->domain_ids);
+ info = xa_load(&domain->iommu_array, iommu->seq_id);
+ if (--info->refcnt == 0) {
+ clear_bit(info->did, iommu->domain_ids);
+ xa_erase(&domain->iommu_array, iommu->seq_id);
domain_update_iommu_cap(domain);
- domain->iommu_did[iommu->seq_id] = 0;
+ kfree(info);
domain->nid may still point to the node of the removed iommu.
Good catch! I should assign it to NUMA_NO_NODE, so that it could be
updated in the next domain_update_iommu_cap(). Updated as below:
@@ -1806,6 +1809,7 @@ static void domain_detach_iommu(struct dmar_domain
*domain,
if (--info->refcnt == 0) {
clear_bit(info->did, iommu->domain_ids);
xa_erase(&domain->iommu_array, iommu->seq_id);
+ domain->nid = NUMA_NO_NODE;
domain_update_iommu_cap(domain);
kfree(info);
}
Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
