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

[Yi Liu <yi.l.liu@intel.com>]

On 2024/11/7 10:57, Baolu Lu wrote:
> 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);
> 

yes. will fix it.

-- 
Regards,
Yi Liu