diff mbox series

[rfcv1,1/6] backends/iommufd_device: introduce IOMMUFDDevice

Message ID 20240115101313.131139-2-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series Check and sync host IOMMU cap/ecap with vIOMMU | expand

Commit Message

Duan, Zhenzhong Jan. 15, 2024, 10:13 a.m. UTC
IOMMUFDDevice represents a device in iommufd and can be used as
a communication interface between devices (i.e., VFIO, VDPA) and
vIOMMU.

Currently it includes iommufd handler and device id information
which could be used by vIOMMU to get hw IOMMU information.

In future nested translation support, vIOMMU is going to have
more iommufd related operations like allocate hwpt for a device,
attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.

IOMMUFDDevice is willingly not a QOM object because we don't want
it to be visible from the user interface.

Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.

Originally-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 MAINTAINERS                     |  4 +--
 include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
 backends/iommufd_device.c       | 50 +++++++++++++++++++++++++++++++++
 backends/meson.build            |  2 +-
 4 files changed, 84 insertions(+), 3 deletions(-)
 create mode 100644 include/sysemu/iommufd_device.h
 create mode 100644 backends/iommufd_device.c

Comments

Eric Auger Jan. 17, 2024, 2:11 p.m. UTC | #1
Hi Zhenzhong,

On 1/15/24 11:13, Zhenzhong Duan wrote:
> IOMMUFDDevice represents a device in iommufd and can be used as
> a communication interface between devices (i.e., VFIO, VDPA) and
> vIOMMU.
>
> Currently it includes iommufd handler and device id information
iommufd handle
> which could be used by vIOMMU to get hw IOMMU information.
>
> In future nested translation support, vIOMMU is going to have
> more iommufd related operations like allocate hwpt for a device,
> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>
> IOMMUFDDevice is willingly not a QOM object because we don't want
> it to be visible from the user interface.
>
> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.

+  iommufd_device_get_info helper
>
> Originally-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  MAINTAINERS                     |  4 +--
>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>  backends/iommufd_device.c       | 50 +++++++++++++++++++++++++++++++++
>  backends/meson.build            |  2 +-
>  4 files changed, 84 insertions(+), 3 deletions(-)
>  create mode 100644 include/sysemu/iommufd_device.h
>  create mode 100644 backends/iommufd_device.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00ec1f7eca..606dfeb2b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2171,8 +2171,8 @@ M: Yi Liu <yi.l.liu@intel.com>
>  M: Eric Auger <eric.auger@redhat.com>
>  M: Zhenzhong Duan <zhenzhong.duan@intel.com>
>  S: Supported
> -F: backends/iommufd.c
> -F: include/sysemu/iommufd.h
> +F: backends/iommufd*.c
> +F: include/sysemu/iommufd*.h
>  F: include/qemu/chardev_open.h
>  F: util/chardev_open.c
>  F: docs/devel/vfio-iommufd.rst
> diff --git a/include/sysemu/iommufd_device.h b/include/sysemu/iommufd_device.h
> new file mode 100644
> index 0000000000..795630324b
> --- /dev/null
> +++ b/include/sysemu/iommufd_device.h
> @@ -0,0 +1,31 @@
> +/*
> + * IOMMUFD Device
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef SYSEMU_IOMMUFD_DEVICE_H
> +#define SYSEMU_IOMMUFD_DEVICE_H
> +
> +#include <linux/iommufd.h>
> +#include "sysemu/iommufd.h"
> +
> +typedef struct IOMMUFDDevice IOMMUFDDevice;
> +
> +/* This is an abstraction of host IOMMUFD device */
> +struct IOMMUFDDevice {
> +    IOMMUFDBackend *iommufd;
> +    uint32_t dev_id;
> +};
> +
> +int iommufd_device_get_info(IOMMUFDDevice *idev,
> +                            enum iommu_hw_info_type *type,
> +                            uint32_t len, void *data);
> +void iommufd_device_init(void *_idev, size_t instance_size,
> +                         IOMMUFDBackend *iommufd, uint32_t dev_id);
> +#endif
> diff --git a/backends/iommufd_device.c b/backends/iommufd_device.c
> new file mode 100644
> index 0000000000..f6e7ca1dbf
> --- /dev/null
> +++ b/backends/iommufd_device.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU abstract of Host IOMMU
it is the abstraction of the IOMMU or of any assigned device?
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <sys/ioctl.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/iommufd_device.h"
> +
> +int (IOMMUFDDevice *idev,
> +                            enum iommu_hw_info_type *type,
> +                            uint32_t len, void *data)
> +{
> +    struct iommu_hw_info info = {
> +        .size = sizeof(info),
> +        .flags = 0,
> +        .dev_id = idev->dev_id,
> +        .data_len = len,
> +        .__reserved = 0,
> +        .data_uptr = (uintptr_t)data,
> +    };
> +    int ret;
> +
> +    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
> +    if (ret) {
> +        error_report("Failed to get info %m");
you may prefer using errp instead of hard traces.
> +    } else {
> +        *type = info.out_data_type;
> +    }
> +
> +    return ret;
> +}
> +
> +void iommufd_device_init(void *_idev, size_t instance_size,
nit: why the "_"
> +                         IOMMUFDBackend *iommufd, uint32_t dev_id)
> +{
> +    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
> +
> +    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
at this stage of the reading it is not clear why you input the
instance_size. worth to be clarified/documented.
> +
> +    idev->iommufd = iommufd;
> +    idev->dev_id = dev_id;
> +}
> diff --git a/backends/meson.build b/backends/meson.build
> index 8b2b111497..c437cdb363 100644
> --- a/backends/meson.build
> +++ b/backends/meson.build
> @@ -24,7 +24,7 @@ if have_vhost_user
>    system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('vhost-user.c'))
>  endif
>  system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost.c'))
> -system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c'))
> +system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c', 'iommufd_device.c'))
>  if have_vhost_user_crypto
>    system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost-user.c'))
>  endif
Thanks

Eric
Duan, Zhenzhong Jan. 18, 2024, 2:58 a.m. UTC | #2
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>Hi Zhenzhong,
>
>On 1/15/24 11:13, Zhenzhong Duan wrote:
>> IOMMUFDDevice represents a device in iommufd and can be used as
>> a communication interface between devices (i.e., VFIO, VDPA) and
>> vIOMMU.
>>
>> Currently it includes iommufd handler and device id information
>iommufd handle
>> which could be used by vIOMMU to get hw IOMMU information.
>>
>> In future nested translation support, vIOMMU is going to have
>> more iommufd related operations like allocate hwpt for a device,
>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>
>> IOMMUFDDevice is willingly not a QOM object because we don't want
>> it to be visible from the user interface.
>>
>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>
>+  iommufd_device_get_info helper

Will do.

>>
>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  MAINTAINERS                     |  4 +--
>>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>  backends/iommufd_device.c       | 50
>+++++++++++++++++++++++++++++++++
>>  backends/meson.build            |  2 +-
>>  4 files changed, 84 insertions(+), 3 deletions(-)
>>  create mode 100644 include/sysemu/iommufd_device.h
>>  create mode 100644 backends/iommufd_device.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 00ec1f7eca..606dfeb2b1 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -2171,8 +2171,8 @@ M: Yi Liu <yi.l.liu@intel.com>
>>  M: Eric Auger <eric.auger@redhat.com>
>>  M: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>  S: Supported
>> -F: backends/iommufd.c
>> -F: include/sysemu/iommufd.h
>> +F: backends/iommufd*.c
>> +F: include/sysemu/iommufd*.h
>>  F: include/qemu/chardev_open.h
>>  F: util/chardev_open.c
>>  F: docs/devel/vfio-iommufd.rst
>> diff --git a/include/sysemu/iommufd_device.h
>b/include/sysemu/iommufd_device.h
>> new file mode 100644
>> index 0000000000..795630324b
>> --- /dev/null
>> +++ b/include/sysemu/iommufd_device.h
>> @@ -0,0 +1,31 @@
>> +/*
>> + * IOMMUFD Device
>> + *
>> + * Copyright (C) 2024 Intel Corporation.
>> + *
>> + * Authors: Yi Liu <yi.l.liu@intel.com>
>> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#ifndef SYSEMU_IOMMUFD_DEVICE_H
>> +#define SYSEMU_IOMMUFD_DEVICE_H
>> +
>> +#include <linux/iommufd.h>
>> +#include "sysemu/iommufd.h"
>> +
>> +typedef struct IOMMUFDDevice IOMMUFDDevice;
>> +
>> +/* This is an abstraction of host IOMMUFD device */
>> +struct IOMMUFDDevice {
>> +    IOMMUFDBackend *iommufd;
>> +    uint32_t dev_id;
>> +};
>> +
>> +int iommufd_device_get_info(IOMMUFDDevice *idev,
>> +                            enum iommu_hw_info_type *type,
>> +                            uint32_t len, void *data);
>> +void iommufd_device_init(void *_idev, size_t instance_size,
>> +                         IOMMUFDBackend *iommufd, uint32_t dev_id);
>> +#endif
>> diff --git a/backends/iommufd_device.c b/backends/iommufd_device.c
>> new file mode 100644
>> index 0000000000..f6e7ca1dbf
>> --- /dev/null
>> +++ b/backends/iommufd_device.c
>> @@ -0,0 +1,50 @@
>> +/*
>> + * QEMU abstract of Host IOMMU
>it is the abstraction of the IOMMU or of any assigned device?

' QEMU abstract of Host IOMMUFD device' may be better.

>> + *
>> + * Copyright (C) 2024 Intel Corporation.
>> + *
>> + * Authors: Yi Liu <yi.l.liu@intel.com>
>> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
>> + *
>> + * SPDX-License-Identifier: GPL-2.0-or-later
>> + */
>> +
>> +#include <sys/ioctl.h>
>> +#include "qemu/osdep.h"
>> +#include "qemu/error-report.h"
>> +#include "sysemu/iommufd_device.h"
>> +
>> +int (IOMMUFDDevice *idev,
>> +                            enum iommu_hw_info_type *type,
>> +                            uint32_t len, void *data)
>> +{
>> +    struct iommu_hw_info info = {
>> +        .size = sizeof(info),
>> +        .flags = 0,
>> +        .dev_id = idev->dev_id,
>> +        .data_len = len,
>> +        .__reserved = 0,
>> +        .data_uptr = (uintptr_t)data,
>> +    };
>> +    int ret;
>> +
>> +    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
>> +    if (ret) {
>> +        error_report("Failed to get info %m");
>you may prefer using errp instead of hard traces.

Good suggestion, will do.

>> +    } else {
>> +        *type = info.out_data_type;
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>> +void iommufd_device_init(void *_idev, size_t instance_size,
>nit: why the "_"

To distinguish with local idev.

>> +                         IOMMUFDBackend *iommufd, uint32_t dev_id)
>> +{
>> +    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
>> +
>> +    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
>at this stage of the reading it is not clear why you input the
>instance_size. worth to be clarified/documented.

VFIO or VDPA may have IOMMUFD related attributes for its own usages.
It looks VFIO doesn't need this for now. I'll remove it, then _idev can be
removed too.

Thanks
Zhenzhong
Eric Auger Jan. 18, 2024, 12:42 p.m. UTC | #3
On 1/15/24 11:13, Zhenzhong Duan wrote:
> IOMMUFDDevice represents a device in iommufd and can be used as
> a communication interface between devices (i.e., VFIO, VDPA) and
> vIOMMU.
>
> Currently it includes iommufd handler and device id information
> which could be used by vIOMMU to get hw IOMMU information.
>
> In future nested translation support, vIOMMU is going to have
> more iommufd related operations like allocate hwpt for a device,
> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>
> IOMMUFDDevice is willingly not a QOM object because we don't want
> it to be visible from the user interface.
>
> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>
> Originally-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  MAINTAINERS                     |  4 +--
>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>  backends/iommufd_device.c       | 50 +++++++++++++++++++++++++++++++++
Maybe it is still time to move the iommufd files in a sepate dir, under
hw at the same level as vfio.

Thoughts?

Eric
>  backends/meson.build            |  2 +-
>  4 files changed, 84 insertions(+), 3 deletions(-)
>  create mode 100644 include/sysemu/iommufd_device.h
>  create mode 100644 backends/iommufd_device.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 00ec1f7eca..606dfeb2b1 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2171,8 +2171,8 @@ M: Yi Liu <yi.l.liu@intel.com>
>  M: Eric Auger <eric.auger@redhat.com>
>  M: Zhenzhong Duan <zhenzhong.duan@intel.com>
>  S: Supported
> -F: backends/iommufd.c
> -F: include/sysemu/iommufd.h
> +F: backends/iommufd*.c
> +F: include/sysemu/iommufd*.h
>  F: include/qemu/chardev_open.h
>  F: util/chardev_open.c
>  F: docs/devel/vfio-iommufd.rst
> diff --git a/include/sysemu/iommufd_device.h b/include/sysemu/iommufd_device.h
> new file mode 100644
> index 0000000000..795630324b
> --- /dev/null
> +++ b/include/sysemu/iommufd_device.h
> @@ -0,0 +1,31 @@
> +/*
> + * IOMMUFD Device
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef SYSEMU_IOMMUFD_DEVICE_H
> +#define SYSEMU_IOMMUFD_DEVICE_H
> +
> +#include <linux/iommufd.h>
> +#include "sysemu/iommufd.h"
> +
> +typedef struct IOMMUFDDevice IOMMUFDDevice;
> +
> +/* This is an abstraction of host IOMMUFD device */
> +struct IOMMUFDDevice {
> +    IOMMUFDBackend *iommufd;
> +    uint32_t dev_id;
> +};
> +
> +int iommufd_device_get_info(IOMMUFDDevice *idev,
> +                            enum iommu_hw_info_type *type,
> +                            uint32_t len, void *data);
> +void iommufd_device_init(void *_idev, size_t instance_size,
> +                         IOMMUFDBackend *iommufd, uint32_t dev_id);
> +#endif
> diff --git a/backends/iommufd_device.c b/backends/iommufd_device.c
> new file mode 100644
> index 0000000000..f6e7ca1dbf
> --- /dev/null
> +++ b/backends/iommufd_device.c
> @@ -0,0 +1,50 @@
> +/*
> + * QEMU abstract of Host IOMMU
> + *
> + * Copyright (C) 2024 Intel Corporation.
> + *
> + * Authors: Yi Liu <yi.l.liu@intel.com>
> + *          Zhenzhong Duan <zhenzhong.duan@intel.com>
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include <sys/ioctl.h>
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "sysemu/iommufd_device.h"
> +
> +int iommufd_device_get_info(IOMMUFDDevice *idev,
> +                            enum iommu_hw_info_type *type,
> +                            uint32_t len, void *data)
> +{
> +    struct iommu_hw_info info = {
> +        .size = sizeof(info),
> +        .flags = 0,
> +        .dev_id = idev->dev_id,
> +        .data_len = len,
> +        .__reserved = 0,
> +        .data_uptr = (uintptr_t)data,
> +    };
> +    int ret;
> +
> +    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
> +    if (ret) {
> +        error_report("Failed to get info %m");
> +    } else {
> +        *type = info.out_data_type;
> +    }
> +
> +    return ret;
> +}
> +
> +void iommufd_device_init(void *_idev, size_t instance_size,
> +                         IOMMUFDBackend *iommufd, uint32_t dev_id)
> +{
> +    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
> +
> +    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
> +
> +    idev->iommufd = iommufd;
> +    idev->dev_id = dev_id;
> +}
> diff --git a/backends/meson.build b/backends/meson.build
> index 8b2b111497..c437cdb363 100644
> --- a/backends/meson.build
> +++ b/backends/meson.build
> @@ -24,7 +24,7 @@ if have_vhost_user
>    system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('vhost-user.c'))
>  endif
>  system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost.c'))
> -system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c'))
> +system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c', 'iommufd_device.c'))
>  if have_vhost_user_crypto
>    system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost-user.c'))
>  endif
Duan, Zhenzhong Jan. 19, 2024, 7:31 a.m. UTC | #4
>-----Original Message-----
>From: Eric Auger <eric.auger@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>
>
>On 1/15/24 11:13, Zhenzhong Duan wrote:
>> IOMMUFDDevice represents a device in iommufd and can be used as
>> a communication interface between devices (i.e., VFIO, VDPA) and
>> vIOMMU.
>>
>> Currently it includes iommufd handler and device id information
>> which could be used by vIOMMU to get hw IOMMU information.
>>
>> In future nested translation support, vIOMMU is going to have
>> more iommufd related operations like allocate hwpt for a device,
>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>
>> IOMMUFDDevice is willingly not a QOM object because we don't want
>> it to be visible from the user interface.
>>
>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>>
>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  MAINTAINERS                     |  4 +--
>>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>  backends/iommufd_device.c       | 50
>+++++++++++++++++++++++++++++++++
>Maybe it is still time to move the iommufd files in a sepate dir, under
>hw at the same level as vfio.
>
>Thoughts?

Any reason for the movement? Hw dir contains entries to emulate different
Devices. Iommufd is not a real device. It's more a backend.

Thanks
Zhenzhong
Cédric Le Goater Jan. 22, 2024, 4:25 p.m. UTC | #5
On 1/19/24 08:31, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>> IOMMUFDDevice
>>
>>
>>
>> On 1/15/24 11:13, Zhenzhong Duan wrote:
>>> IOMMUFDDevice represents a device in iommufd and can be used as
>>> a communication interface between devices (i.e., VFIO, VDPA) and
>>> vIOMMU.
>>>
>>> Currently it includes iommufd handler and device id information
>>> which could be used by vIOMMU to get hw IOMMU information.
>>>
>>> In future nested translation support, vIOMMU is going to have
>>> more iommufd related operations like allocate hwpt for a device,
>>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>>
>>> IOMMUFDDevice is willingly not a QOM object because we don't want
>>> it to be visible from the user interface.
>>>
>>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>>>
>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>   MAINTAINERS                     |  4 +--
>>>   include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>   backends/iommufd_device.c       | 50
>> +++++++++++++++++++++++++++++++++
>> Maybe it is still time to move the iommufd files in a sepate dir, under
>> hw at the same level as vfio.
>>
>> Thoughts?
> 
> Any reason for the movement? Hw dir contains entries to emulate different
> Devices. Iommufd is not a real device. It's more a backend.

I would include the new services in the existing iommufd .[ch] files.
No need for a new file since the changes are all related to the IOMMUFD
device usage.

Thanks,

C.
Duan, Zhenzhong Jan. 23, 2024, 5:51 a.m. UTC | #6
>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>IOMMUFDDevice
>
>On 1/19/24 08:31, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Eric Auger <eric.auger@redhat.com>
>>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>>> IOMMUFDDevice
>>>
>>>
>>>
>>> On 1/15/24 11:13, Zhenzhong Duan wrote:
>>>> IOMMUFDDevice represents a device in iommufd and can be used as
>>>> a communication interface between devices (i.e., VFIO, VDPA) and
>>>> vIOMMU.
>>>>
>>>> Currently it includes iommufd handler and device id information
>>>> which could be used by vIOMMU to get hw IOMMU information.
>>>>
>>>> In future nested translation support, vIOMMU is going to have
>>>> more iommufd related operations like allocate hwpt for a device,
>>>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>>>
>>>> IOMMUFDDevice is willingly not a QOM object because we don't want
>>>> it to be visible from the user interface.
>>>>
>>>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>>>>
>>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>> ---
>>>>   MAINTAINERS                     |  4 +--
>>>>   include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>>   backends/iommufd_device.c       | 50
>>> +++++++++++++++++++++++++++++++++
>>> Maybe it is still time to move the iommufd files in a sepate dir, under
>>> hw at the same level as vfio.
>>>
>>> Thoughts?
>>
>> Any reason for the movement? Hw dir contains entries to emulate
>different
>> Devices. Iommufd is not a real device. It's more a backend.
>
>I would include the new services in the existing iommufd .[ch] files.
>No need for a new file since the changes are all related to the IOMMUFD
>device usage.

Make sense, will do.

Thanks
Zhenzhong
Eric Auger Jan. 23, 2024, 10:10 a.m. UTC | #7
On 1/19/24 08:31, Duan, Zhenzhong wrote:
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Subject: Re: [PATCH rfcv1 1/6] backends/iommufd_device: introduce
>> IOMMUFDDevice
>>
>>
>>
>> On 1/15/24 11:13, Zhenzhong Duan wrote:
>>> IOMMUFDDevice represents a device in iommufd and can be used as
>>> a communication interface between devices (i.e., VFIO, VDPA) and
>>> vIOMMU.
>>>
>>> Currently it includes iommufd handler and device id information
>>> which could be used by vIOMMU to get hw IOMMU information.
>>>
>>> In future nested translation support, vIOMMU is going to have
>>> more iommufd related operations like allocate hwpt for a device,
>>> attach/detach hwpt, etc. So IOMMUFDDevice will be further expanded.
>>>
>>> IOMMUFDDevice is willingly not a QOM object because we don't want
>>> it to be visible from the user interface.
>>>
>>> Introduce a helper iommufd_device_init to initialize IOMMUFDDevice.
>>>
>>> Originally-by: Yi Liu <yi.l.liu@intel.com>
>>> Signed-off-by: Yi Sun <yi.y.sun@linux.intel.com>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> ---
>>>  MAINTAINERS                     |  4 +--
>>>  include/sysemu/iommufd_device.h | 31 ++++++++++++++++++++
>>>  backends/iommufd_device.c       | 50
>> +++++++++++++++++++++++++++++++++
>> Maybe it is still time to move the iommufd files in a sepate dir, under
>> hw at the same level as vfio.
>>
>> Thoughts?
> Any reason for the movement? Hw dir contains entries to emulate different
> Devices. Iommufd is not a real device. It's more a backend.
Well I was thinking it would become bigger and bigger and since you
created a new .c file with new abstraction (devices) the backend dir
flat layout may not be well adapted. But as suggested by Cédric we may
use the existing files.

Thanks

Eric
>
> Thanks
> Zhenzhong
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 00ec1f7eca..606dfeb2b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2171,8 +2171,8 @@  M: Yi Liu <yi.l.liu@intel.com>
 M: Eric Auger <eric.auger@redhat.com>
 M: Zhenzhong Duan <zhenzhong.duan@intel.com>
 S: Supported
-F: backends/iommufd.c
-F: include/sysemu/iommufd.h
+F: backends/iommufd*.c
+F: include/sysemu/iommufd*.h
 F: include/qemu/chardev_open.h
 F: util/chardev_open.c
 F: docs/devel/vfio-iommufd.rst
diff --git a/include/sysemu/iommufd_device.h b/include/sysemu/iommufd_device.h
new file mode 100644
index 0000000000..795630324b
--- /dev/null
+++ b/include/sysemu/iommufd_device.h
@@ -0,0 +1,31 @@ 
+/*
+ * IOMMUFD Device
+ *
+ * Copyright (C) 2024 Intel Corporation.
+ *
+ * Authors: Yi Liu <yi.l.liu@intel.com>
+ *          Zhenzhong Duan <zhenzhong.duan@intel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#ifndef SYSEMU_IOMMUFD_DEVICE_H
+#define SYSEMU_IOMMUFD_DEVICE_H
+
+#include <linux/iommufd.h>
+#include "sysemu/iommufd.h"
+
+typedef struct IOMMUFDDevice IOMMUFDDevice;
+
+/* This is an abstraction of host IOMMUFD device */
+struct IOMMUFDDevice {
+    IOMMUFDBackend *iommufd;
+    uint32_t dev_id;
+};
+
+int iommufd_device_get_info(IOMMUFDDevice *idev,
+                            enum iommu_hw_info_type *type,
+                            uint32_t len, void *data);
+void iommufd_device_init(void *_idev, size_t instance_size,
+                         IOMMUFDBackend *iommufd, uint32_t dev_id);
+#endif
diff --git a/backends/iommufd_device.c b/backends/iommufd_device.c
new file mode 100644
index 0000000000..f6e7ca1dbf
--- /dev/null
+++ b/backends/iommufd_device.c
@@ -0,0 +1,50 @@ 
+/*
+ * QEMU abstract of Host IOMMU
+ *
+ * Copyright (C) 2024 Intel Corporation.
+ *
+ * Authors: Yi Liu <yi.l.liu@intel.com>
+ *          Zhenzhong Duan <zhenzhong.duan@intel.com>
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+
+#include <sys/ioctl.h>
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "sysemu/iommufd_device.h"
+
+int iommufd_device_get_info(IOMMUFDDevice *idev,
+                            enum iommu_hw_info_type *type,
+                            uint32_t len, void *data)
+{
+    struct iommu_hw_info info = {
+        .size = sizeof(info),
+        .flags = 0,
+        .dev_id = idev->dev_id,
+        .data_len = len,
+        .__reserved = 0,
+        .data_uptr = (uintptr_t)data,
+    };
+    int ret;
+
+    ret = ioctl(idev->iommufd->fd, IOMMU_GET_HW_INFO, &info);
+    if (ret) {
+        error_report("Failed to get info %m");
+    } else {
+        *type = info.out_data_type;
+    }
+
+    return ret;
+}
+
+void iommufd_device_init(void *_idev, size_t instance_size,
+                         IOMMUFDBackend *iommufd, uint32_t dev_id)
+{
+    IOMMUFDDevice *idev = (IOMMUFDDevice *)_idev;
+
+    g_assert(sizeof(IOMMUFDDevice) <= instance_size);
+
+    idev->iommufd = iommufd;
+    idev->dev_id = dev_id;
+}
diff --git a/backends/meson.build b/backends/meson.build
index 8b2b111497..c437cdb363 100644
--- a/backends/meson.build
+++ b/backends/meson.build
@@ -24,7 +24,7 @@  if have_vhost_user
   system_ss.add(when: 'CONFIG_VIRTIO', if_true: files('vhost-user.c'))
 endif
 system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost.c'))
-system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c'))
+system_ss.add(when: 'CONFIG_IOMMUFD', if_true: files('iommufd.c', 'iommufd_device.c'))
 if have_vhost_user_crypto
   system_ss.add(when: 'CONFIG_VIRTIO_CRYPTO', if_true: files('cryptodev-vhost-user.c'))
 endif