| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aeb88b6b-9989-4e1a-824c-757ffdbfbca7@amd.com>
Date: Thu, 7 Nov 2024 21:11:27 +1100
From: Alexey Kardashevskiy <aik@....com>
To: Nicolin Chen <nicolinc@...dia.com>, jgg@...dia.com, kevin.tian@...el.com,
corbet@....net
Cc: joro@...tes.org, suravee.suthikulpanit@....com, will@...nel.org,
robin.murphy@....com, dwmw2@...radead.org, shuah@...nel.org,
iommu@...ts.linux.dev, linux-doc@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
baolu.lu@...ux.intel.com, eric.auger@...hat.com, jean-philippe@...aro.org,
mdf@...nel.org, mshavit@...gle.com, shameerali.kolothum.thodi@...wei.com,
smostafa@...gle.com, yi.l.liu@...el.com, zhangfei.gao@...aro.org,
patches@...ts.linux.dev
Subject: Re: [PATCH v6 01/10] iommufd/viommu: Add IOMMUFD_OBJ_VDEVICE and
IOMMU_VDEVICE_ALLOC ioctl
On 31/10/24 08:35, Nicolin Chen wrote:
> Introduce a new IOMMUFD_OBJ_VDEVICE to represent a physical device (struct
> device) against a vIOMMU (struct iommufd_viommu) object in a VM.
>
> This vDEVICE object (and its structure) holds all the infos and attributes
> in the VM, regarding the device related to the vIOMMU.
>
> As an initial patch, add a per-vIOMMU virtual ID. This can be:
> - Virtual StreamID on a nested ARM SMMUv3, an index to a Stream Table
> - Virtual DeviceID on a nested AMD IOMMU, an index to a Device Table
> - Virtual RID on a nested Intel VT-D IOMMU, an index to a Context Table
> Potentially, this vDEVICE structure would hold some vData for Confidential
> Compute Architecture (CCA). Use this virtual ID to index an "vdevs" xarray
> that belongs to a vIOMMU object.
>
> Add a new ioctl for vDEVICE allocations. Since a vDEVICE is a connection
> of a device object and an iommufd_viommu object, take two refcounts in the
> ioctl handler.
>
> Signed-off-by: Nicolin Chen <nicolinc@...dia.com>
> ---
> drivers/iommu/iommufd/iommufd_private.h | 18 ++++++
> include/linux/iommufd.h | 4 ++
> include/uapi/linux/iommufd.h | 22 +++++++
> drivers/iommu/iommufd/main.c | 6 ++
> drivers/iommu/iommufd/viommu.c | 76 +++++++++++++++++++++++++
> 5 files changed, 126 insertions(+)
>
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index e8f5ef550cc9..062656c19a07 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -507,8 +507,26 @@ static inline int iommufd_hwpt_replace_device(struct iommufd_device *idev,
> return iommu_group_replace_domain(idev->igroup->group, hwpt->domain);
> }
>
> +static inline struct iommufd_viommu *
> +iommufd_get_viommu(struct iommufd_ucmd *ucmd, u32 id)
> +{
> + return container_of(iommufd_get_object(ucmd->ictx, id,
> + IOMMUFD_OBJ_VIOMMU),
> + struct iommufd_viommu, obj);
> +}
> +
> int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd);
> void iommufd_viommu_destroy(struct iommufd_object *obj);
> +int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd);
> +void iommufd_vdevice_destroy(struct iommufd_object *obj);
> +
> +struct iommufd_vdevice {
> + struct iommufd_object obj;
> + struct iommufd_ctx *ictx;
> + struct iommufd_viommu *viommu;
> + struct device *dev;
> + u64 id; /* per-vIOMMU virtual ID */
> +};
>
> #ifdef CONFIG_IOMMUFD_TEST
> int iommufd_test(struct iommufd_ucmd *ucmd);
> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index f03c75410938..ee58c5c573ec 100644
> --- a/include/linux/iommufd.h
> +++ b/include/linux/iommufd.h
> @@ -10,6 +10,7 @@
> #include <linux/errno.h>
> #include <linux/refcount.h>
> #include <linux/types.h>
> +#include <linux/xarray.h>
>
> struct device;
> struct file;
> @@ -31,6 +32,7 @@ enum iommufd_object_type {
> IOMMUFD_OBJ_ACCESS,
> IOMMUFD_OBJ_FAULT,
> IOMMUFD_OBJ_VIOMMU,
> + IOMMUFD_OBJ_VDEVICE,
> #ifdef CONFIG_IOMMUFD_TEST
> IOMMUFD_OBJ_SELFTEST,
> #endif
> @@ -89,6 +91,8 @@ struct iommufd_viommu {
>
> const struct iommufd_viommu_ops *ops;
>
> + struct xarray vdevs;
> +
> unsigned int type;
> };
>
> diff --git a/include/uapi/linux/iommufd.h b/include/uapi/linux/iommufd.h
> index a498d4838f9a..9b5236004b8e 100644
> --- a/include/uapi/linux/iommufd.h
> +++ b/include/uapi/linux/iommufd.h
> @@ -53,6 +53,7 @@ enum {
> IOMMUFD_CMD_FAULT_QUEUE_ALLOC = 0x8e,
> IOMMUFD_CMD_IOAS_MAP_FILE = 0x8f,
> IOMMUFD_CMD_VIOMMU_ALLOC = 0x90,
> + IOMMUFD_CMD_VDEVICE_ALLOC = 0x91,
> };
>
> /**
> @@ -864,4 +865,25 @@ struct iommu_viommu_alloc {
> __u32 out_viommu_id;
> };
> #define IOMMU_VIOMMU_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_ALLOC)
> +
> +/**
> + * struct iommu_vdevice_alloc - ioctl(IOMMU_VDEVICE_ALLOC)
> + * @size: sizeof(struct iommu_vdevice_alloc)
> + * @viommu_id: vIOMMU ID to associate with the virtual device
> + * @dev_id: The physical device to allocate a virtual instance on the vIOMMU
> + * @out_vdevice_id: Object handle for the vDevice. Pass to IOMMU_DESTORY
> + * @virt_id: Virtual device ID per vIOMMU, e.g. vSID of ARM SMMUv3, vDeviceID
> + * of AMD IOMMU, and vRID of a nested Intel VT-d to a Context Table
So it is one vdevice per a passed through device (say, a network
adapter), right? I am asking as there are passed through devices and
IOMMU devices, and (at least on AMD) IOMMUs look like PCI devices, both
in hosts and guests. For example, from the above: "@dev_id: The physical
device ..." - both a network card and IOMMU are physical, so dev_id is a
NIC or IOMMU? I assume that шы a NIC (but it is a source of constant
confusion).
Is there any plan to add guest device BDFn as well, or I can add one
here for my TEE-IO exercise, if it is the right place? It is the same as
vDeviceID for AMD but I am not sure about the others, hence the
question. Thanks,
> + *
> + * Allocate a virtual device instance (for a physical device) against a vIOMMU.
> + * This instance holds the device's information (related to its vIOMMU) in a VM.
> + */
> +struct iommu_vdevice_alloc {
> + __u32 size;
> + __u32 viommu_id;
> + __u32 dev_id;
> + __u32 out_vdevice_id;
> + __aligned_u64 virt_id;
> +};
> +#define IOMMU_VDEVICE_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VDEVICE_ALLOC)
> #endif
> diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c
> index cc514f9bc3e6..d735fe04197f 100644
> --- a/drivers/iommu/iommufd/main.c
> +++ b/drivers/iommu/iommufd/main.c
> @@ -308,6 +308,7 @@ union ucmd_buffer {
> struct iommu_option option;
> struct iommu_vfio_ioas vfio_ioas;
> struct iommu_viommu_alloc viommu;
> + struct iommu_vdevice_alloc vdev;
> #ifdef CONFIG_IOMMUFD_TEST
> struct iommu_test_cmd test;
> #endif
> @@ -363,6 +364,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = {
> __reserved),
> IOCTL_OP(IOMMU_VIOMMU_ALLOC, iommufd_viommu_alloc_ioctl,
> struct iommu_viommu_alloc, out_viommu_id),
> + IOCTL_OP(IOMMU_VDEVICE_ALLOC, iommufd_vdevice_alloc_ioctl,
> + struct iommu_vdevice_alloc, virt_id),
> #ifdef CONFIG_IOMMUFD_TEST
> IOCTL_OP(IOMMU_TEST_CMD, iommufd_test, struct iommu_test_cmd, last),
> #endif
> @@ -501,6 +504,9 @@ static const struct iommufd_object_ops iommufd_object_ops[] = {
> [IOMMUFD_OBJ_VIOMMU] = {
> .destroy = iommufd_viommu_destroy,
> },
> + [IOMMUFD_OBJ_VDEVICE] = {
> + .destroy = iommufd_vdevice_destroy,
> + },
> #ifdef CONFIG_IOMMUFD_TEST
> [IOMMUFD_OBJ_SELFTEST] = {
> .destroy = iommufd_selftest_destroy,
> diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c
> index 888239b78667..c82b4a07a4b1 100644
> --- a/drivers/iommu/iommufd/viommu.c
> +++ b/drivers/iommu/iommufd/viommu.c
> @@ -11,6 +11,7 @@ void iommufd_viommu_destroy(struct iommufd_object *obj)
> if (viommu->ops && viommu->ops->destroy)
> viommu->ops->destroy(viommu);
> refcount_dec(&viommu->hwpt->common.obj.users);
> + xa_destroy(&viommu->vdevs);
> }
>
> int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
> @@ -53,6 +54,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
> goto out_put_hwpt;
> }
>
> + xa_init(&viommu->vdevs);
> viommu->type = cmd->type;
> viommu->ictx = ucmd->ictx;
> viommu->hwpt = hwpt_paging;
> @@ -79,3 +81,77 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)
> iommufd_put_object(ucmd->ictx, &idev->obj);
> return rc;
> }
> +
> +void iommufd_vdevice_destroy(struct iommufd_object *obj)
> +{
> + struct iommufd_vdevice *vdev =
> + container_of(obj, struct iommufd_vdevice, obj);
> + struct iommufd_viommu *viommu = vdev->viommu;
> +
> + /* xa_cmpxchg is okay to fail if alloc returned -EEXIST previously */
> + xa_cmpxchg(&viommu->vdevs, vdev->id, vdev, NULL, GFP_KERNEL);
> + refcount_dec(&viommu->obj.users);
> + put_device(vdev->dev);
> +}
> +
> +int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd)
> +{
> + struct iommu_vdevice_alloc *cmd = ucmd->cmd;
> + struct iommufd_vdevice *vdev, *curr;
> + struct iommufd_viommu *viommu;
> + struct iommufd_device *idev;
> + u64 virt_id = cmd->virt_id;
> + int rc = 0;
> +
> + /* virt_id indexes an xarray */
> + if (virt_id > ULONG_MAX)
> + return -EINVAL;
> +
> + viommu = iommufd_get_viommu(ucmd, cmd->viommu_id);
> + if (IS_ERR(viommu))
> + return PTR_ERR(viommu);
> +
> + idev = iommufd_get_device(ucmd, cmd->dev_id);
> + if (IS_ERR(idev)) {
> + rc = PTR_ERR(idev);
> + goto out_put_viommu;
> + }
> +
> + if (viommu->iommu_dev != __iommu_get_iommu_dev(idev->dev)) {
> + rc = -EINVAL;
> + goto out_put_idev;
> + }
> +
> + vdev = iommufd_object_alloc(ucmd->ictx, vdev, IOMMUFD_OBJ_VDEVICE);
> + if (IS_ERR(vdev)) {
> + rc = PTR_ERR(vdev);
> + goto out_put_idev;
> + }
> +
> + vdev->id = virt_id;
> + vdev->dev = idev->dev;
> + get_device(idev->dev);
> + vdev->viommu = viommu;
> + refcount_inc(&viommu->obj.users);
> +
> + curr = xa_cmpxchg(&viommu->vdevs, virt_id, NULL, vdev, GFP_KERNEL);
> + if (curr) {
> + rc = xa_err(curr) ?: -EEXIST;
> + goto out_abort;
> + }
> +
> + cmd->out_vdevice_id = vdev->obj.id;
> + rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));
> + if (rc)
> + goto out_abort;
> + iommufd_object_finalize(ucmd->ictx, &vdev->obj);
> + goto out_put_idev;
> +
> +out_abort:
> + iommufd_object_abort_and_destroy(ucmd->ictx, &vdev->obj);
> +out_put_idev:
> + iommufd_put_object(ucmd->ictx, &idev->obj);
> +out_put_viommu:
> + iommufd_put_object(ucmd->ictx, &viommu->obj);
> + return rc;
> +}
--
Alexey
Powered by blists - more mailing lists