Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:3 as permitted sender) client-ip=2620:137:e000::3:3;
Message-ID: <a4c2402f-fc81-ff54-7c5b-606fa14405e2@linux.intel.com>
Date:   Fri, 13 Oct 2023 20:48:19 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Cc:     baolu.lu@linux.intel.com, "cohuck@redhat.com" <cohuck@redhat.com>,
        "eric.auger@redhat.com" <eric.auger@redhat.com>,
        "nicolinc@nvidia.com" <nicolinc@nvidia.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
        "chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
        "yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
        "peterx@redhat.com" <peterx@redhat.com>,
        "jasowang@redhat.com" <jasowang@redhat.com>,
        "shameerali.kolothum.thodi@huawei.com" 
        <shameerali.kolothum.thodi@huawei.com>,
        "lulu@redhat.com" <lulu@redhat.com>,
        "suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
        "iommu@lists.linux.dev" <iommu@lists.linux.dev>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "Duan, Zhenzhong" <zhenzhong.duan@intel.com>,
        "Martins, Joao" <joao.m.martins@oracle.com>,
        Jacob Pan <jacob.jun.pan@linux.intel.com>
Subject: Re: [PATCH v5 04/11] iommu/vt-d: Add helper to setup pasid nested
 translation
To:     Yi Liu <yi.l.liu@intel.com>, "Tian, Kevin" <kevin.tian@intel.com>,
        "joro@8bytes.org" <joro@8bytes.org>,
        "alex.williamson@redhat.com" <alex.williamson@redhat.com>,
        "jgg@nvidia.com" <jgg@nvidia.com>,
        "robin.murphy@arm.com" <robin.murphy@arm.com>
References: <20230921075431.125239-1-yi.l.liu@intel.com>
 <20230921075431.125239-5-yi.l.liu@intel.com>
 <BN9PR11MB5276DD2D6E5690F8FC7B18378CC2A@BN9PR11MB5276.namprd11.prod.outlook.com>
 <8974035d-9f9c-e209-79d6-8e3c6402e516@intel.com>
Content-Language: en-US
From:   Baolu Lu <baolu.lu@linux.intel.com>
In-Reply-To: <8974035d-9f9c-e209-79d6-8e3c6402e516@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 2023/10/13 20:40, Yi Liu wrote:
>>
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if ((s1_cfg->flags & IOMMU_VTD_S1_SRE) && !ecap_srs(iommu-
>>>> ecap)) {
>>> +        pr_err_ratelimited("No supervisor request support on %s\n",
>>> +                   iommu->name);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    if ((s1_cfg->flags & IOMMU_VTD_S1_EAFE) && !ecap_eafs(iommu-
>>>> ecap)) {
>>> +        pr_err_ratelimited("No extended access flag support
>>> on %s\n",
>>> +                   iommu->name);
>>> +        return -EINVAL;
>>> +    }
>>> +
>>> +    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 -EBUSY;
>>> +    }
>>> +
>>> +    pasid_clear_entry(pte);
>>> +
>>> +    if (s1_cfg->addr_width == ADDR_WIDTH_5LEVEL)
>>> +        pasid_set_flpm(pte, 1);
>>> +
>>> +    pasid_set_flptr(pte, (uintptr_t)s1_gpgd);
>>> +
>>> +    if (s1_cfg->flags & IOMMU_VTD_S1_SRE) {
>>> +        pasid_set_sre(pte);
>>> +        if (s1_cfg->flags & IOMMU_VTD_S1_WPE)
>>> +            pasid_set_wpe(pte);
>>> +    }
>>> +
>>> +    if (s1_cfg->flags & IOMMU_VTD_S1_EAFE)
>>> +        pasid_set_eafe(pte);
>>> +
>>> +    if (s2_domain->force_snooping)
>>> +        pasid_set_pgsnp(pte);
>>> +
>>> +    pasid_set_slptr(pte, virt_to_phys(pgd));
>>> +    pasid_set_fault_enable(pte);
>>> +    pasid_set_domain_id(pte, did);
>>> +    pasid_set_address_width(pte, s2_domain->agaw);
>>> +    pasid_set_page_snoop(pte, !!ecap_smpwc(iommu->ecap));
>>> +    pasid_set_translation_type(pte, PASID_ENTRY_PGTT_NESTED);
>>> +    pasid_set_present(pte);
>>> +    spin_unlock(&iommu->lock);
>>
>> All changes within iommu->lock are specific to the device specific
>> PASID entry. Probably this is one potential cleanup TODO to
>> use a per-device lock instead.
> 
> yeah, a separate cleanup. is it, @Baolu?

Sure. I'd like to take some time to consider this further. Please keep
it as-is for the time being.

Best regards,
baolu