On 10/26/23 12:30, Zhenzhong Duan wrote:
> IOMMUFD supports auto allocated hwpt and manually allocated one.
> Manually allocated hwpt has benefit that its life cycle is under
> user's control, so it could be used as stage 2 page table by nested
> feature in the future.
Would an option be useful to switch from one mode to another ?
>
> Introduce two helpers __vfio_device_attach/detach_hwpt to facilitate
> this change.
I think merging this change with the previous patch makes sense.
It doesn't add much to keep it as a standalone patch unless we
want a feature toggle.
Thanks,
C.
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
> hw/vfio/iommufd.c | 89 +++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 70 insertions(+), 19 deletions(-)
>
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index aee64d63f3..c1daaf1c39 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -217,38 +217,91 @@ static VFIOIOASHwpt *vfio_container_get_hwpt(VFIOIOMMUFDContainer *container,
> static void vfio_container_put_hwpt(IOMMUFDBackend *be, VFIOIOASHwpt *hwpt)
> {
> QLIST_REMOVE(hwpt, next);
> + iommufd_backend_free_id(be->fd, hwpt->hwpt_id);
> g_free(hwpt);
> }
>
> -static int vfio_device_attach_container(VFIODevice *vbasedev,
> - VFIOIOMMUFDContainer *container,
> - Error **errp)
> +static int __vfio_device_attach_hwpt(VFIODevice *vbasedev, uint32_t hwpt_id,
> + Error **errp)
> {
> - int ret, iommufd = vbasedev->iommufd->fd;
> - VFIOIOASHwpt *hwpt;
> struct vfio_device_attach_iommufd_pt attach_data = {
> .argsz = sizeof(attach_data),
> .flags = 0,
> - .pt_id = container->ioas_id,
> + .pt_id = hwpt_id,
> };
> + int ret;
>
> - /* Attach device to an ioas within iommufd */
> ret = ioctl(vbasedev->fd, VFIO_DEVICE_ATTACH_IOMMUFD_PT, &attach_data);
> if (ret) {
> error_setg_errno(errp, errno,
> - "[iommufd=%d] error attach %s (%d) to ioasid=%d",
> - container->be->fd, vbasedev->name, vbasedev->fd,
> - attach_data.pt_id);
> + "[iommufd=%d] error attach %s (%d) to hwpt_id=%d",
> + vbasedev->iommufd->fd, vbasedev->name, vbasedev->fd,
> + hwpt_id);
> + }
> + return ret;
> +}
> +
> +static int __vfio_device_detach_hwpt(VFIODevice *vbasedev, Error **errp)
> +{
> + struct vfio_device_detach_iommufd_pt detach_data = {
> + .argsz = sizeof(detach_data),
> + .flags = 0,
> + };
> + int ret;
> +
> + ret = ioctl(vbasedev->fd, VFIO_DEVICE_DETACH_IOMMUFD_PT, &detach_data);
> + if (ret) {
> + error_setg_errno(errp, errno, "detach %s from ioas failed",
> + vbasedev->name);
> + }
> + return ret;
> +}
> +
> +static int vfio_device_attach_container(VFIODevice *vbasedev,
> + VFIOIOMMUFDContainer *container,
> + Error **errp)
> +{
> + int ret, iommufd = vbasedev->iommufd->fd;
> + VFIOIOASHwpt *hwpt;
> + uint32_t hwpt_id;
> + Error *err = NULL;
> +
> + /* try to attach to an existing hwpt in this container */
> + QLIST_FOREACH(hwpt, &container->hwpt_list, next) {
> + ret = __vfio_device_attach_hwpt(vbasedev, hwpt->hwpt_id, &err);
> + if (ret) {
> + const char *msg = error_get_pretty(err);
> +
> + trace_vfio_iommufd_fail_attach_existing_hwpt(msg);
> + error_free(err);
> + err = NULL;
> + } else {
> + goto found_hwpt;
> + }
> + }
> +
> + ret = iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
> + container->ioas_id, &hwpt_id);
> +
> + if (ret) {
> + error_setg_errno(errp, errno, "error alloc shadow hwpt");
> return ret;
> + }
>
> + /* Attach device to an hwpt within iommufd */
> + ret = __vfio_device_attach_hwpt(vbasedev, hwpt_id, errp);
> + if (ret) {
> + iommufd_backend_free_id(iommufd, hwpt_id);
> + return ret;
> }
> - hwpt = vfio_container_get_hwpt(container, attach_data.pt_id);
>
> + hwpt = vfio_container_get_hwpt(container, hwpt_id);
> +found_hwpt:
> QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, next);
> vbasedev->hwpt = hwpt;
>
> trace_vfio_iommufd_attach_device(iommufd, vbasedev->name, vbasedev->fd,
> - container->ioas_id, attach_data.pt_id);
> + container->ioas_id, hwpt->hwpt_id);
> return ret;
> }
>
> @@ -256,14 +309,12 @@ static void vfio_device_detach_container(VFIODevice *vbasedev,
> VFIOIOMMUFDContainer *container)
> {
> VFIOIOASHwpt *hwpt = vbasedev->hwpt;
> - struct vfio_device_attach_iommufd_pt detach_data = {
> - .argsz = sizeof(detach_data),
> - .flags = 0,
> - };
> + Error *err = NULL;
> + int ret;
>
> - if (ioctl(vbasedev->fd, VFIO_DEVICE_DETACH_IOMMUFD_PT, &detach_data)) {
> - error_report("detach %s from ioas id=%d failed: %m", vbasedev->name,
> - container->ioas_id);
> + ret = __vfio_device_detach_hwpt(vbasedev, &err);
> + if (ret) {
> + error_report_err(err);
> }
>
> QLIST_REMOVE(vbasedev, next);