On 2023/12/12 10:20, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Monday, December 11, 2023 4:08 PM
On 2023/12/7 16:47, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
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
