In preparation to moving HostIOMMUDevice realize() being able to called
early during attach_device(), remove properties that rely on container
being initialized.
This means removing caps::aw_bits which requires the
bcontainer::iova_ranges to be inititalized after device is actually
attached. Instead defer that to .get_cap() and call
vfio_device_get_aw_bits() directly.
Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
include/sysemu/host_iommu_device.h | 1 -
backends/iommufd.c | 3 ++-
hw/vfio/container.c | 5 +----
hw/vfio/iommufd.c | 1 -
4 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
index ee6c813c8b22..20e77cf54568 100644
--- a/include/sysemu/host_iommu_device.h
+++ b/include/sysemu/host_iommu_device.h
@@ -24,7 +24,6 @@
*/
typedef struct HostIOMMUDeviceCaps {
uint32_t type;
- uint8_t aw_bits;
} HostIOMMUDeviceCaps;
#define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
diff --git a/backends/iommufd.c b/backends/iommufd.c
index 5d3dfa917415..41a9dec3b2c5 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -18,6 +18,7 @@
#include "qemu/error-report.h"
#include "monitor/monitor.h"
#include "trace.h"
+#include "hw/vfio/vfio-common.h"
#include <sys/ioctl.h>
#include <linux/iommufd.h>
@@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
return caps->type;
case HOST_IOMMU_DEVICE_CAP_AW_BITS:
- return caps->aw_bits;
+ return vfio_device_get_aw_bits(hiod->agent);
default:
error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
return -EINVAL;
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 88ede913d6f7..c27f448ba26e 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
VFIODevice *vdev = opaque;
hiod->name = g_strdup(vdev->name);
- hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
hiod->agent = opaque;
return true;
@@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
Error **errp)
{
- HostIOMMUDeviceCaps *caps = &hiod->caps;
-
switch (cap) {
case HOST_IOMMU_DEVICE_CAP_AW_BITS:
- return caps->aw_bits;
+ return vfio_device_get_aw_bits(hiod->agent);
default:
error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
return -EINVAL;
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 325c7598d5a1..873c919e319c 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
hiod->name = g_strdup(vdev->name);
caps->type = type;
- caps->aw_bits = vfio_device_get_aw_bits(vdev);
return true;
}
--
2.17.2Hi Joao,
On 7/12/24 13:46, Joao Martins wrote:
> In preparation to moving HostIOMMUDevice realize() being able to called
> early during attach_device(), remove properties that rely on container
> being initialized.
It is difficult to parse the above sentence. Would deserve some rephrasing.
Also properties have a different meaning in qemu.
>
> This means removing caps::aw_bits which requires the
> bcontainer::iova_ranges to be inititalized after device is actually
initialized
> attached. Instead defer that to .get_cap() and call
> vfio_device_get_aw_bits() directly.
>
> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
> include/sysemu/host_iommu_device.h | 1 -
> backends/iommufd.c | 3 ++-
> hw/vfio/container.c | 5 +----
> hw/vfio/iommufd.c | 1 -
> 4 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
> index ee6c813c8b22..20e77cf54568 100644
> --- a/include/sysemu/host_iommu_device.h
> +++ b/include/sysemu/host_iommu_device.h
> @@ -24,7 +24,6 @@
> */
> typedef struct HostIOMMUDeviceCaps {
> uint32_t type;
> - uint8_t aw_bits;
the doc comment needs to be updated accordingly.
> } HostIOMMUDeviceCaps;
>
> #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 5d3dfa917415..41a9dec3b2c5 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "monitor/monitor.h"
> #include "trace.h"
> +#include "hw/vfio/vfio-common.h"
> #include <sys/ioctl.h>
> #include <linux/iommufd.h>
>
> @@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
> case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
> return caps->type;
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> - return caps->aw_bits;
> + return vfio_device_get_aw_bits(hiod->agent);
> default:
> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> return -EINVAL;
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 88ede913d6f7..c27f448ba26e 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
> VFIODevice *vdev = opaque;
>
> hiod->name = g_strdup(vdev->name);
> - hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
> hiod->agent = opaque;
>
> return true;
> @@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
> static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
> Error **errp)
> {
> - HostIOMMUDeviceCaps *caps = &hiod->caps;
> -
> switch (cap) {
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> - return caps->aw_bits;
> + return vfio_device_get_aw_bits(hiod->agent);
> default:
> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> return -EINVAL;
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 325c7598d5a1..873c919e319c 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>
> hiod->name = g_strdup(vdev->name);
> caps->type = type;
> - caps->aw_bits = vfio_device_get_aw_bits(vdev);
>
> return true;
> }
On 16/07/2024 18:40, Eric Auger wrote:
> Hi Joao,
>
> On 7/12/24 13:46, Joao Martins wrote:
>> In preparation to moving HostIOMMUDevice realize() being able to called
>> early during attach_device(), remove properties that rely on container
>> being initialized.
> It is difficult to parse the above sentence. Would deserve some rephrasing.
>
> Also properties have a different meaning in qemu.
I think I will remove the above paragraph and instead adopt below with some
rephrasing:
Remove caps::aw_bits which requires the
bcontainer::iova_ranges to be inititalized after device is actually
initialized attached. Instead defer that to .get_cap() and call
vfio_device_get_aw_bits() directly.
This is in preparation for HostIOMMUDevice::realize() being called early during
attach_device().
Better?
>>
>> This means removing caps::aw_bits which requires the
>> bcontainer::iova_ranges to be inititalized after device is actually
> initialized
Yes
>> attached. Instead defer that to .get_cap() and call
>> vfio_device_get_aw_bits() directly.
>>
>> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> include/sysemu/host_iommu_device.h | 1 -
>> backends/iommufd.c | 3 ++-
>> hw/vfio/container.c | 5 +----
>> hw/vfio/iommufd.c | 1 -
>> 4 files changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
>> index ee6c813c8b22..20e77cf54568 100644
>> --- a/include/sysemu/host_iommu_device.h
>> +++ b/include/sysemu/host_iommu_device.h
>> @@ -24,7 +24,6 @@
>> */
>> typedef struct HostIOMMUDeviceCaps {
>> uint32_t type;
>> - uint8_t aw_bits;
> the doc comment needs to be updated accordingly.
>> } HostIOMMUDeviceCaps;
>>
>> #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>> index 5d3dfa917415..41a9dec3b2c5 100644
>> --- a/backends/iommufd.c
>> +++ b/backends/iommufd.c
>> @@ -18,6 +18,7 @@
>> #include "qemu/error-report.h"
>> #include "monitor/monitor.h"
>> #include "trace.h"
>> +#include "hw/vfio/vfio-common.h"
>> #include <sys/ioctl.h>
>> #include <linux/iommufd.h>
>>
>> @@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>> case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
>> return caps->type;
>> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>> - return caps->aw_bits;
>> + return vfio_device_get_aw_bits(hiod->agent);
>> default:
>> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>> return -EINVAL;
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 88ede913d6f7..c27f448ba26e 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>> VFIODevice *vdev = opaque;
>>
>> hiod->name = g_strdup(vdev->name);
>> - hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
>> hiod->agent = opaque;
>>
>> return true;
>> @@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>> static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
>> Error **errp)
>> {
>> - HostIOMMUDeviceCaps *caps = &hiod->caps;
>> -
>> switch (cap) {
>> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>> - return caps->aw_bits;
>> + return vfio_device_get_aw_bits(hiod->agent);
>> default:
>> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>> return -EINVAL;
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 325c7598d5a1..873c919e319c 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>
>> hiod->name = g_strdup(vdev->name);
>> caps->type = type;
>> - caps->aw_bits = vfio_device_get_aw_bits(vdev);
>>
>> return true;
>> }
>
Hi Joao,
On 7/16/24 20:22, Joao Martins wrote:
> On 16/07/2024 18:40, Eric Auger wrote:
>> Hi Joao,
>>
>> On 7/12/24 13:46, Joao Martins wrote:
>>> In preparation to moving HostIOMMUDevice realize() being able to called
>>> early during attach_device(), remove properties that rely on container
>>> being initialized.
>> It is difficult to parse the above sentence. Would deserve some rephrasing.
>>
>> Also properties have a different meaning in qemu.
> I think I will remove the above paragraph and instead adopt below with some
> rephrasing:
>
> Remove caps::aw_bits which requires the
> bcontainer::iova_ranges to be inititalized after device is actually
> initialized attached. Instead defer that to .get_cap() and call
s/initialized//g
> vfio_device_get_aw_bits() directly.
>
> This is in preparation for HostIOMMUDevice::realize() being called early during
> attach_device().
>
> Better?
Yes sounds better
Eric
>
>>> This means removing caps::aw_bits which requires the
>>> bcontainer::iova_ranges to be inititalized after device is actually
>> initialized
> Yes
>
>>> attached. Instead defer that to .get_cap() and call
>>> vfio_device_get_aw_bits() directly.
>>>
>>> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> include/sysemu/host_iommu_device.h | 1 -
>>> backends/iommufd.c | 3 ++-
>>> hw/vfio/container.c | 5 +----
>>> hw/vfio/iommufd.c | 1 -
>>> 4 files changed, 3 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
>>> index ee6c813c8b22..20e77cf54568 100644
>>> --- a/include/sysemu/host_iommu_device.h
>>> +++ b/include/sysemu/host_iommu_device.h
>>> @@ -24,7 +24,6 @@
>>> */
>>> typedef struct HostIOMMUDeviceCaps {
>>> uint32_t type;
>>> - uint8_t aw_bits;
>> the doc comment needs to be updated accordingly.
>>> } HostIOMMUDeviceCaps;
>>>
>>> #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
>>> diff --git a/backends/iommufd.c b/backends/iommufd.c
>>> index 5d3dfa917415..41a9dec3b2c5 100644
>>> --- a/backends/iommufd.c
>>> +++ b/backends/iommufd.c
>>> @@ -18,6 +18,7 @@
>>> #include "qemu/error-report.h"
>>> #include "monitor/monitor.h"
>>> #include "trace.h"
>>> +#include "hw/vfio/vfio-common.h"
>>> #include <sys/ioctl.h>
>>> #include <linux/iommufd.h>
>>>
>>> @@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
>>> case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
>>> return caps->type;
>>> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>>> - return caps->aw_bits;
>>> + return vfio_device_get_aw_bits(hiod->agent);
>>> default:
>>> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>>> return -EINVAL;
>>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>>> index 88ede913d6f7..c27f448ba26e 100644
>>> --- a/hw/vfio/container.c
>>> +++ b/hw/vfio/container.c
>>> @@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>> VFIODevice *vdev = opaque;
>>>
>>> hiod->name = g_strdup(vdev->name);
>>> - hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
>>> hiod->agent = opaque;
>>>
>>> return true;
>>> @@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>> static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
>>> Error **errp)
>>> {
>>> - HostIOMMUDeviceCaps *caps = &hiod->caps;
>>> -
>>> switch (cap) {
>>> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
>>> - return caps->aw_bits;
>>> + return vfio_device_get_aw_bits(hiod->agent);
>>> default:
>>> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
>>> return -EINVAL;
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index 325c7598d5a1..873c919e319c 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>>>
>>> hiod->name = g_strdup(vdev->name);
>>> caps->type = type;
>>> - caps->aw_bits = vfio_device_get_aw_bits(vdev);
>>>
>>> return true;
>>> }
On 7/12/24 13:46, Joao Martins wrote:
> In preparation to moving HostIOMMUDevice realize() being able to called
> early during attach_device(), remove properties that rely on container
> being initialized.
>
> This means removing caps::aw_bits which requires the
> bcontainer::iova_ranges to be inititalized after device is actually
> attached. Instead defer that to .get_cap() and call
> vfio_device_get_aw_bits() directly.
>
> Suggested-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> include/sysemu/host_iommu_device.h | 1 -
> backends/iommufd.c | 3 ++-
> hw/vfio/container.c | 5 +----
> hw/vfio/iommufd.c | 1 -
> 4 files changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/include/sysemu/host_iommu_device.h b/include/sysemu/host_iommu_device.h
> index ee6c813c8b22..20e77cf54568 100644
> --- a/include/sysemu/host_iommu_device.h
> +++ b/include/sysemu/host_iommu_device.h
> @@ -24,7 +24,6 @@
> */
> typedef struct HostIOMMUDeviceCaps {
> uint32_t type;
> - uint8_t aw_bits;
> } HostIOMMUDeviceCaps;
>
> #define TYPE_HOST_IOMMU_DEVICE "host-iommu-device"
> diff --git a/backends/iommufd.c b/backends/iommufd.c
> index 5d3dfa917415..41a9dec3b2c5 100644
> --- a/backends/iommufd.c
> +++ b/backends/iommufd.c
> @@ -18,6 +18,7 @@
> #include "qemu/error-report.h"
> #include "monitor/monitor.h"
> #include "trace.h"
> +#include "hw/vfio/vfio-common.h"
> #include <sys/ioctl.h>
> #include <linux/iommufd.h>
>
> @@ -270,7 +271,7 @@ static int hiod_iommufd_get_cap(HostIOMMUDevice *hiod, int cap, Error **errp)
> case HOST_IOMMU_DEVICE_CAP_IOMMU_TYPE:
> return caps->type;
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> - return caps->aw_bits;
> + return vfio_device_get_aw_bits(hiod->agent);
> default:
> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> return -EINVAL;
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 88ede913d6f7..c27f448ba26e 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -1144,7 +1144,6 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
> VFIODevice *vdev = opaque;
>
> hiod->name = g_strdup(vdev->name);
> - hiod->caps.aw_bits = vfio_device_get_aw_bits(vdev);
> hiod->agent = opaque;
>
> return true;
> @@ -1153,11 +1152,9 @@ static bool hiod_legacy_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
> static int hiod_legacy_vfio_get_cap(HostIOMMUDevice *hiod, int cap,
> Error **errp)
> {
> - HostIOMMUDeviceCaps *caps = &hiod->caps;
> -
> switch (cap) {
> case HOST_IOMMU_DEVICE_CAP_AW_BITS:
> - return caps->aw_bits;
> + return vfio_device_get_aw_bits(hiod->agent);
> default:
> error_setg(errp, "%s: unsupported capability %x", hiod->name, cap);
> return -EINVAL;
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 325c7598d5a1..873c919e319c 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -722,7 +722,6 @@ static bool hiod_iommufd_vfio_realize(HostIOMMUDevice *hiod, void *opaque,
>
> hiod->name = g_strdup(vdev->name);
> caps->type = type;
> - caps->aw_bits = vfio_device_get_aw_bits(vdev);
>
> return true;
> }
© 2016 - 2024 Red Hat, Inc.