Re: [PATCH v5 04/13] iommu/vt-d: Add pasid replace helpers

[Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>]

On 11/6/24 23:45, Yi Liu wrote:
> +int intel_pasid_replace_second_level(struct intel_iommu *iommu,
> +				     struct dmar_domain *domain,
> +				     struct device *dev, u16 old_did,
> +				     u32 pasid)
> +{
> +	struct pasid_entry *pte;
> +	struct dma_pte *pgd;
> +	u64 pgd_val;
> +	int agaw;
> +	u16 did;
> +
> +	/*
> +	 * If hardware advertises no support for second level
> +	 * translation, return directly.
> +	 */
> +	if (!ecap_slts(iommu->ecap)) {
> +		pr_err("No second level translation support on %s\n",
> +		       iommu->name);
> +		return -EINVAL;
> +	}
> +
> +	pgd = domain->pgd;
> +	pgd_val = virt_to_phys(pgd);
> +	did = domain_id_iommu(domain, iommu);
> +
> +	spin_lock(&iommu->lock);
> +	pte = intel_pasid_get_entry(dev, pasid);
> +	if (!pte) {
> +		spin_unlock(&iommu->lock);
> +		return -ENODEV;
> +	}
> +
> +	if (!pasid_pte_is_present(pte)) {
> +		spin_unlock(&iommu->lock);
> +		return -EINVAL;
> +	}
> +
> +	WARN_ON(old_did != pasid_get_domain_id(pte));
> +
> +	pasid_pte_config_second_level(iommu, pte, pgd_val, agaw,
> +				      did, domain->dirty_tracking);
> +	spin_unlock(&iommu->lock);
> +
> +	intel_pasid_flush_present(iommu, dev, pasid, old_did, pte);
> +	intel_iommu_drain_pasid_prq(dev, pasid);
> +
> +	return 0;
> +}

0day robot complains:

>> drivers/iommu/intel/pasid.c:540:53: warning: variable 'agaw' is 
uninitialized when used here [-Wuninitialized]
     540 |         pasid_pte_config_second_level(iommu, pte, pgd_val, agaw,
         |                                                            ^~~~
   drivers/iommu/intel/pasid.c:509:10: note: initialize the variable 
'agaw' to silence this warning
     509 |         int agaw;
         |                 ^
         |                  = 0

The right fix could be like this:

diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 777e70b539b1..69f12b1b8a2b 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -506,7 +506,6 @@ int intel_pasid_replace_second_level(struct 
intel_iommu *iommu,
        struct pasid_entry *pte;
        struct dma_pte *pgd;
        u64 pgd_val;
-       int agaw;
        u16 did;

        /*
@@ -537,7 +536,7 @@ int intel_pasid_replace_second_level(struct 
intel_iommu *iommu,

        WARN_ON(old_did != pasid_get_domain_id(pte));

-       pasid_pte_config_second_level(iommu, pte, pgd_val, agaw,
+       pasid_pte_config_second_level(iommu, pte, pgd_val, domain->agaw,
                                      did, domain->dirty_tracking);
        spin_unlock(&iommu->lock);

--
baolu