Hi,
On 2018/5/29 19:55, Jean-Philippe Brucker wrote:
(If possible, please reply in plain text to the list. Reading this in a
text-only reader is confusing, because all quotes have the same level)
Sorry for that， I have reset the thunderbird, :) thanks.
On 26/05/18 04:53, Xu Zaibo wrote:
I guess there may be some misunderstandings :).
From the current patches, 'iommu_sva_device_shutdown' is called by 'vfio_iommu_sva_shutdown', which
is mainly used by 'vfio_iommu_type1_detach_group' that is finally called by processes' release of vfio facilitiy
automatically or called by 'VFIO_GROUP_UNSET_CONTAINER' manually in the processes.
So, just image that 2 processes is working on the device with IOPF feature, and the 2 do the following to enable SVA:
1.open("/dev/vfio/vfio") ;
2.open the group of the devcie by calling open("/dev/vfio/x"), but now,
I think the second processes will fail to open the group because current VFIO thinks that one group only can be used by one process/vm,
at present, I use mediated device to create more groups on the parent device to support multiple processes;
3.VFIO_GROUP_SET_CONTAINER;
4.VFIO_SET_IOMMU;
5.VFIO_IOMMU_BIND;
I have a concern regarding your driver. With mdev you can't allow
processes to program the PASID themselves, since the parent device has a
single PASID table. You lose all isolation since processes could write
any value in the PASID field and access address spaces of other
processes bound to this parent device (even if the BIND call was for
other mdevs).
Yes, if wrapdrive do nothing on this PASID setting procedure in kernel
space, I think
there definitely exists this security risk.
The wrap driver has to mediate calls to bind(), and either program the
PASID into the device itself, or at least check that, when receiving a
SET_PASID ioctl from a process, the given PASID was actually allocated
to the process.
Yes, good advice, thanks.
6.Do some works with the hardware working unit filled by PASID on the device;
7.VFIO_IOMMU_UNBIND;
8.VFIO_GROUP_UNSET_CONTAINER;---here, have to sleep to wait another process to finish works of the step 6;
9. close(group); close(containner);
So, my idea is: If it is possible to just release the params or facilities that only belong to the current process while the process shutdown the device,
and while the last process releases them all. Then, as in the above step 8, we
don't need to wait, or maybe wait for a very long time if the other process is doing lots of work on the device.
Given that you need to notify the mediating driver of IOMMU_BIND calls
as explained above, you could do something similar for shutdown: from
the mdev driver, call iommu_sva_shutdown_device() only for the last mdev.
Currently, I add an API to check if it is the last mdev in wrapdrive, as
vfio shutdowns the device,
it call the API to do the check at first.
Thanks
Zaibo
