Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Message-ID: <ccabd500-6b0a-4663-bf63-715bd28aff76@intel.com>
Date:   Tue, 12 Dec 2023 11:26:25 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/3] vfio: Report PASID capability via VFIO_DEVICE_FEATURE
 ioctl
Content-Language: en-US
To:     "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>,
        "baolu.lu@linux.intel.com" <baolu.lu@linux.intel.com>
CC:     "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>,
        "joao.m.martins@oracle.com" <joao.m.martins@oracle.com>,
        "Zeng, Xin" <xin.zeng@intel.com>,
        "Zhao, Yan Y" <yan.y.zhao@intel.com>
References: <20231127063909.129153-1-yi.l.liu@intel.com>
 <20231127063909.129153-4-yi.l.liu@intel.com>
 <BN9PR11MB527639DBE4C433542F351F6D8C8BA@BN9PR11MB5276.namprd11.prod.outlook.com>
 <0bdae2ca-a200-4db1-a016-059730d1545e@intel.com>
 <BN9PR11MB52763C75E3D638B722CE63A78C8EA@BN9PR11MB5276.namprd11.prod.outlook.com>
From:   Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <BN9PR11MB52763C75E3D638B722CE63A78C8EA@BN9PR11MB5276.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M0lPMk9ialpXZ21BaTF6aEp0SWxUb3YyckViK1BQaGFtcmkreGlyU2N1MWdo?=
 =?utf-8?B?TmI0TEJYekt6Nzg4UkZWRXZyekt2d1BtSllMbFFHdkppMWo0djZQVHZpaSth?=
 =?utf-8?B?Z3BwYXdxN25IRVIyOEdnQ2dxSkFla1V1dXZsS29HMjJMcndYQnltRDV3bWJl?=
 =?utf-8?B?OEJDNFVsZzJmOWliUDdONFM2aGgxbHhONDN3S2tYMExkY0xCWTlLSkhTSG1Z?=
 =?utf-8?B?S01Jak1veXE0Q2lOZ1pjcDlQSFVyOEJIa3dXb0FyOTNvSWNLK1FiaFpuYkNn?=
 =?utf-8?B?TkdlQ3A2U3dFb2xaN1RvNjlyK1RVUVZQQlkzQVVvZVQ4NXdMMHNjTmVFdkZB?=
 =?utf-8?B?MUlQWUJCZHFDVXU2VzJ1eEh1V1c0MjZUZU1CWTdZdklQWEI4QmIxOGNJYjJH?=
 =?utf-8?B?Y0ZmZTExVHJ1bExVeUZaTjVINTJnV2hwY2cxOG95NEM2bFRhOXJqbVRnangx?=
 =?utf-8?B?am9INzcydWRDZm9jRkxpVzdsTDdIREFKNk5NeFVwTnU2SDBuTnkvK250MGNt?=
 =?utf-8?B?TTFKdEtFSGpoY1Q4U1BaMkc1dVJxUGd6RWVYOW9iQ0ZNTjRrOS96ZkpydEFl?=
 =?utf-8?B?KzNHd1Rva1QyUHlIbTlaY1hyQUUxem9lcnNXcURCdnB4RWFFazNYemdmNFo1?=
 =?utf-8?B?MHI0bzl6SFBEOFZjYVBQWVlwSUhSK291bkFFU3drUmVQV1pDVmg3dWd6elFj?=
 =?utf-8?B?cVFYK2YxUzlDMmJCOXJMZ2pzS1E3VmV4Y3hYNnlOa3U1bVV1RWVzTEhSVi95?=
 =?utf-8?B?T1FhY1NVM2FLN2FpdURYWEtGZTljUjVrWURtb2drL0xFTitVajlzWjZUU3Uv?=
 =?utf-8?B?d1ByM2hhTkQyd3pBc0pLSU9OY1JVQzV5eGZLeENPb2QwOXBja3NPZGdIcG13?=
 =?utf-8?B?Vm5ldkxTSGRMdnNOS0JZdnhIRkdHTTREaUVzKzc2R29TWjVJWWxwdllqNkJV?=
 =?utf-8?B?anFKQWhJK1hpT1U4QkYzMHI2bnZmLzdQYjJBb1NTWGpMSkRhQnZwUVRxdVk1?=
 =?utf-8?B?WFFEWnFsTHhkWlN6bEh0amlLWkp1OTV1cHVYdHVxRHRpcUxxeXFGS2xzOWZx?=
 =?utf-8?B?YmhVckQ4cm83TVVJdTZyZm9qVFRIeW4rSlVtNzZvSXAwK3doTXVuNEEzbEJp?=
 =?utf-8?B?STZ5WEptUEQvTVhrYjd6Nm9jQlR0TkRRMXhnSkZPT2tpRDJ3NVdsVG9rR3pP?=
 =?utf-8?B?b2hrMHZ2ZGluWkEvTW44V2kvV2UxM3ZveGhDYzlMaEJOR2NaT2kzbEtNdzc2?=
 =?utf-8?B?aThRV21SSGZzRXdMY3A3c0dBSjF0a0lNS2U2QmZsOElSVy9ibFhxa1ZOS0RH?=
 =?utf-8?B?TllqY2FiR2htWFo0MnZGZytobERyeUVlbG43aUk4QmNkUFN0UzgzRC9weVU3?=
 =?utf-8?B?ZUlCV3lKTWVVNFNENDE4dHV1VW9xbjZNQ2VZcmFPWXVQd1NPakJrYmp6NVlp?=
 =?utf-8?B?Wk5ONENRT2QyUHBmMHQ5czByQm90dUJDTm1HeEhRc2tvSjJ5TTZFSjdZOW9L?=
 =?utf-8?B?RmRadkxtdlZ1Vk9vUzhNRVN0WFl4RngzeGVoTkg2cGh0ajlVTE4vRVEyMXY1?=
 =?utf-8?B?TTVhbldMTXBGNDdNVU5XRytyN3dtek1GRlV0S2NtMURxUFpvL01aZWoya2lt?=
 =?utf-8?B?UzIzanFkd3F1UmhnOG1RalpIZ0FSU1V5SUdXNTY2QUxOOEc4emdtVFpRaHhH?=
 =?utf-8?B?Tjk5TThHNGxMOGMvZ0FlWkxlOXdwcm9NWUJyY3lSYXlKcmJXUldoTXQ1alV1?=
 =?utf-8?B?Ums1Vi80RlZMMURXaTB3Z3RKNzF1RVRFTG01MzFxcklCWjJyMkwvQ1ZndVF2?=
 =?utf-8?B?a1RFeGFWVTlMMzE3eXdtLzFzNEUrZVh3c1c3TzhjRGYzU2NnNW9LamRyMTU2?=
 =?utf-8?B?WWZGYmNsVWNEejZFS3R3Z2NBRGRPWWVWRi9ndG54Vjd0cDFSSkQ1aVpjV3Zi?=
 =?utf-8?B?T3VEMGcvTkhWQ08weEg3eW4vMU1rUkZjLzJ5RnNtazA0cUp4emhYTVcwQ2E5?=
 =?utf-8?B?UE1oVXZCWGZEZXF0bGhERU16Y1VRMkZab0NYSkp6UkZSSnJUQjA2K3JGZXFQ?=
 =?utf-8?B?cXdWNGg4MHdJK0ErODhPNzF2bjl5c2d1YlAxeWxMUUpBN1BaZWZCa1JHK1JB?=
 =?utf-8?Q?SBMBZXp/lEq34sMENRifGLuSU?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8ce4ce09-e7bb-4a49-dbf4-08dbfac1c3a2
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Dec 2023 03:23:52.4242
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8ROZ/WaQDAHZ/jgsn5fGI8HEbCeFQruo7o0WlCmWVXJT6ho7g/p71bj7JTKsKquIxQbRPyuTg9Dp0Kdh/V79uA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB4901
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Mon, 11 Dec 2023 19:24:09 -0800 (PST)

On 2023/12/12 10:20, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Monday, December 11, 2023 4:08 PM
>>
>> On 2023/12/7 16:47, Tian, Kevin wrote:
>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>> Sent: Monday, November 27, 2023 2:39 PM
>>>>
>>>> +static int vfio_pci_core_feature_pasid(struct vfio_device *device, u32
>> flags,
>>>> +				       struct vfio_device_feature_pasid __user
>>>> *arg,
>>>> +				       size_t argsz)
>>>> +{
>>>> +	struct vfio_pci_core_device *vdev =
>>>> +		container_of(device, struct vfio_pci_core_device, vdev);
>>>> +	struct vfio_device_feature_pasid pasid = { 0 };
>>>> +	struct pci_dev *pdev = vdev->pdev;
>>>> +	u32 capabilities = 0;
>>>> +	int ret;
>>>> +
>>>> +	/* We do not support SET of the PASID capability */
>>>
>>> this line alone is meaningless. Please explain the reason e.g. due to
>>> no PASID capability per VF...
>>
>> sure. I think the major reason is we don't allow userspace to change the
>> PASID configuration. is it?
> 
> if only PF it's still possible to develop a model allowing userspace to
> change.
> 
> but with VF this is not possible in concept.

got it.

> 
>>>> +	if (pdev->is_virtfn)
>>>> +		pdev = pci_physfn(pdev);
>>>> +
>>>> +	if (!pdev->pasid_enabled)
>>>> +		goto out;
>>>> +
>>>> +#ifdef CONFIG_PCI_PASID
>>>> +	pci_read_config_dword(pdev, pdev->pasid_cap + PCI_PASID_CAP,
>>>> +			      &capabilities);
>>>> +#endif
>>>
>>> #ifdef is unnecessary. If CONFIG_PCI_PASID is false pdev->pasid_enabled
>>> won't be set anyway.
>>
>> it's sad that the pdev->pasid_cap is defined under #if CONFIG_PCI_PASID.
>> Perhaps we can have a wrapper for it.
> 
> oh I didn't note it.

If Alex feels better to have a wrapper, we may have one.

>>
>>> and it should read from PCI_PASID_CTRL which indicates whether a
>>> capability is actually enabled.
>>
>> yes, for the EXEC and PRIV capability, needs to check if it's enabled or
>> not before reporting.
>>
>>>
>>>> +/**
>>>> + * Upon VFIO_DEVICE_FEATURE_GET, return the PASID capability for the
>>>> device.
>>>> + * Zero width means no support for PASID.
>>>
>>> also mention the encoding of this field according to PCIe spec.
>>
>> yes.
>>
>>> or turn it to a plain number field.
>>
>> It is not exact the same as the spec since bit0 is reserved. But
>> here bit0 is used as well.
>>
> 
> what is bit0 used for?

it's just been reserved. No usage is mentioned in the latest spec. I don't
know the background neither.

-- 
Regards,
Yi Liu