[PATCH 0/2] Don't initialize HOST_IOMMU_DEVICE with mdev

[PATCH 0/2] Don't initialize HOST_IOMMU_DEVICE with mdev

[Zhenzhong Duan]

This fixes a potential issue with mdev that fails to initialize HOST_IOMMU_DEVICE.
Reason is mdev isn't physical device and doesn't support IOMMU_GET_HW_INFO.

I thought ap/ccw are all mdev type and need a fix.

This series depends on a patch from Joao which fixes the same for vfio-pci.
See https://lists.gnu.org/archive/html/qemu-devel/2024-07/msg04612.html

Not tested due to no ap/ccw environment. But build test pass.

Thanks
Zhenzhong

Zhenzhong Duan (2):
  vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev
  vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

 hw/vfio/ap.c  | 3 +++
 hw/vfio/ccw.c | 3 +++
 2 files changed, 6 insertions(+)

-- 
2.34.1

[PATCH 1/2] vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev

[Zhenzhong Duan]

mdevs aren't "physical" devices and when asking for backing IOMMU info,
it fails the entire provisioning of the guest. Fix that by setting
vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
presence of mdevs.

Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/ap.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index 0c4354e3e7..391bfb72ca 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -230,6 +230,9 @@ static void vfio_ap_instance_init(Object *obj)
      */
     vfio_device_init(vbasedev, VFIO_DEVICE_TYPE_AP, &vfio_ap_ops,
                      DEVICE(vapdev), true);
+
+    /* AP device is mdev type device */
+    vbasedev->mdev = true;
 }
 
 #ifdef CONFIG_IOMMUFD
-- 
2.34.1

[PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Zhenzhong Duan]

mdevs aren't "physical" devices and when asking for backing IOMMU info,
it fails the entire provisioning of the guest. Fix that by setting
vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
presence of mdevs.

Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/vfio/ccw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 1f8e1272c7..70934b01d5 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -675,6 +675,9 @@ static void vfio_ccw_instance_init(Object *obj)
     VFIOCCWDevice *vcdev = VFIO_CCW(obj);
     VFIODevice *vbasedev = &vcdev->vdev;
 
+    /* CCW device is mdev type device */
+    vbasedev->mdev = true;
+
     /*
      * All vfio-ccw devices are believed to operate in a way compatible with
      * discarding of memory in RAM blocks, ie. pages pinned in the host are
-- 
2.34.1

Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Joao Martins]

On 22/07/2024 08:07, Zhenzhong Duan wrote:
> mdevs aren't "physical" devices and when asking for backing IOMMU info,
> it fails the entire provisioning of the guest. Fix that by setting
> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
> presence of mdevs.
> 
> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  hw/vfio/ccw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 1f8e1272c7..70934b01d5 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -675,6 +675,9 @@ static void vfio_ccw_instance_init(Object *obj)
>      VFIOCCWDevice *vcdev = VFIO_CCW(obj);
>      VFIODevice *vbasedev = &vcdev->vdev;
>  
> +    /* CCW device is mdev type device */
> +    vbasedev->mdev = true;
> +
>      /*
>       * All vfio-ccw devices are believed to operate in a way compatible with
>       * discarding of memory in RAM blocks, ie. pages pinned in the host are

Re: [PATCH 1/2] vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev

[Joao Martins]

On 22/07/2024 08:07, Zhenzhong Duan wrote:
> mdevs aren't "physical" devices and when asking for backing IOMMU info,
> it fails the entire provisioning of the guest. Fix that by setting
> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
> presence of mdevs.
> 
> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Reviewed-by: Joao Martins <joao.m.martins@oracle.com>

> ---
>  hw/vfio/ap.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> index 0c4354e3e7..391bfb72ca 100644
> --- a/hw/vfio/ap.c
> +++ b/hw/vfio/ap.c
> @@ -230,6 +230,9 @@ static void vfio_ap_instance_init(Object *obj)
>       */
>      vfio_device_init(vbasedev, VFIO_DEVICE_TYPE_AP, &vfio_ap_ops,
>                       DEVICE(vapdev), true);
> +
> +    /* AP device is mdev type device */
> +    vbasedev->mdev = true;
>  }
>  
>  #ifdef CONFIG_IOMMUFD

Re: [PATCH 0/2] Don't initialize HOST_IOMMU_DEVICE with mdev

[Eric Auger]

Hi Zhenzhong,

On 7/22/24 09:07, Zhenzhong Duan wrote:
> This fixes a potential issue with mdev that fails to initialize HOST_IOMMU_DEVICE.
> Reason is mdev isn't physical device and doesn't support IOMMU_GET_HW_INFO.
>
> I thought ap/ccw are all mdev type and need a fix.
>
> This series depends on a patch from Joao which fixes the same for vfio-pci.
> See https://lists.gnu.org/archive/html/qemu-devel/2024-07/msg04612.html
>
> Not tested due to no ap/ccw environment. But build test pass.
>
> Thanks
> Zhenzhong
>
> Zhenzhong Duan (2):
>   vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev
>   vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev
For the series
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric

>
>  hw/vfio/ap.c  | 3 +++
>  hw/vfio/ccw.c | 3 +++
>  2 files changed, 6 insertions(+)
>

Re: [PATCH 0/2] Don't initialize HOST_IOMMU_DEVICE with mdev

[Cédric Le Goater]

On 7/22/24 09:07, Zhenzhong Duan wrote:
> This fixes a potential issue with mdev that fails to initialize HOST_IOMMU_DEVICE.
> Reason is mdev isn't physical device and doesn't support IOMMU_GET_HW_INFO.
> 
> I thought ap/ccw are all mdev type and need a fix.
> 
> This series depends on a patch from Joao which fixes the same for vfio-pci.
> See https://lists.gnu.org/archive/html/qemu-devel/2024-07/msg04612.html
> 
> Not tested due to no ap/ccw environment. But build test pass.
> 
> Thanks
> Zhenzhong
> 
> Zhenzhong Duan (2):
>    vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev
>    vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev
> 
>   hw/vfio/ap.c  | 3 +++
>   hw/vfio/ccw.c | 3 +++
>   2 files changed, 6 insertions(+)
> 

I have queued this series and the first 2 patches of "[v5] hw/vfio: IOMMUFD
Dirty Tracking" :

   https://lore.kernel.org/all/20240719120501.81279-1-joao.m.martins@oracle.com/

I am trying to estimate now if the above could be fully merge for 9.1.

Thanks,

C.

Re: [PATCH 0/2] Don't initialize HOST_IOMMU_DEVICE with mdev

[Cédric Le Goater]

On 7/22/24 09:07, Zhenzhong Duan wrote:
> This fixes a potential issue with mdev that fails to initialize HOST_IOMMU_DEVICE.
> Reason is mdev isn't physical device and doesn't support IOMMU_GET_HW_INFO.
> 
> I thought ap/ccw are all mdev type and need a fix.
> 
> This series depends on a patch from Joao which fixes the same for vfio-pci.
> See https://lists.gnu.org/archive/html/qemu-devel/2024-07/msg04612.html
> 
> Not tested due to no ap/ccw environment. But build test pass.
> 
> Thanks
> Zhenzhong
> 
> Zhenzhong Duan (2):
>    vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev
>    vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev
> 
>   hw/vfio/ap.c  | 3 +++
>   hw/vfio/ccw.c | 3 +++
>   2 files changed, 6 insertions(+)
> 


Applied to vfio-next.

Thanks,

C.

Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Eric Farman]

On Mon, 2024-07-22 at 15:07 +0800, Zhenzhong Duan wrote:
> mdevs aren't "physical" devices and when asking for backing IOMMU info,
> it fails the entire provisioning of the guest. Fix that by setting
> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
> presence of mdevs.

Hmm, picking the two commits that Cedric mentioned in his cover-letter reply [1] doesn't "fail the entire provisioning of the guest" for me.

Applying this patch on top of that causes the call from vfio_attach_device() to hiod_legacy_vfio_realize() to be skipped, which seems odd. What am I missing?

[1] https://lore.kernel.org/qemu-devel/4c9a184b-514c-4276-95ca-9ed86623b9a4@redhat.com/

> 
> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  hw/vfio/ccw.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
> index 1f8e1272c7..70934b01d5 100644
> --- a/hw/vfio/ccw.c
> +++ b/hw/vfio/ccw.c
> @@ -675,6 +675,9 @@ static void vfio_ccw_instance_init(Object *obj)
>      VFIOCCWDevice *vcdev = VFIO_CCW(obj);
>      VFIODevice *vbasedev = &vcdev->vdev;
>  
> +    /* CCW device is mdev type device */
> +    vbasedev->mdev = true;
> +
>      /*
>       * All vfio-ccw devices are believed to operate in a way compatible with
>       * discarding of memory in RAM blocks, ie. pages pinned in the host are

Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Joao Martins]

On 22/07/2024 15:57, Eric Farman wrote:
> On Mon, 2024-07-22 at 15:07 +0800, Zhenzhong Duan wrote:
>> mdevs aren't "physical" devices and when asking for backing IOMMU info,
>> it fails the entire provisioning of the guest. Fix that by setting
>> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
>> presence of mdevs.
> 
> Hmm, picking the two commits that Cedric mentioned in his cover-letter reply [1] doesn't "fail the entire provisioning of the guest" for me.
> 
> Applying this patch on top of that causes the call from vfio_attach_device() to hiod_legacy_vfio_realize() to be skipped, which seems odd. What am I missing?
> 
> [1] https://lore.kernel.org/qemu-devel/4c9a184b-514c-4276-95ca-9ed86623b9a4@redhat.com/
>

If you are using IOMMUFD it will fail the entire provisioning i.e. GET_HW_INFO
fails because there's no actual device/IOMMU you can probe hardware information
from and you can't start a guest. This happened at least for me in x86 vfio-pci
mdevs (or at least I reproduced it when trying to test mdev_tty)

But if you don't support IOMMUFD, then it probably makes no difference as type1
doesn't do anything particularly special besides initializing some static data.
The realize is skipped because you technically don't have a physical host IOMMU
directly behind the mdev, but rather some parent function related software
entity doing that for you.

Zhengzhong noticed there were some other mdevs aside from vfio-pci and in an
attempt to prevent regression elsewhere it posted for the other mdevs in qemu.

	Joao

Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Cédric Le Goater]

On 7/22/24 17:09, Joao Martins wrote:
> On 22/07/2024 15:57, Eric Farman wrote:
>> On Mon, 2024-07-22 at 15:07 +0800, Zhenzhong Duan wrote:
>>> mdevs aren't "physical" devices and when asking for backing IOMMU info,
>>> it fails the entire provisioning of the guest. Fix that by setting
>>> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
>>> presence of mdevs.
>>
>> Hmm, picking the two commits that Cedric mentioned in his cover-letter reply [1] doesn't "fail the entire provisioning of the guest" for me.
>>
>> Applying this patch on top of that causes the call from vfio_attach_device() to hiod_legacy_vfio_realize() to be skipped, which seems odd. What am I missing?
>>
>> [1] https://lore.kernel.org/qemu-devel/4c9a184b-514c-4276-95ca-9ed86623b9a4@redhat.com/
>>
> 
> If you are using IOMMUFD it will fail the entire provisioning i.e. GET_HW_INFO
> fails because there's no actual device/IOMMU you can probe hardware information
> from and you can't start a guest. This happened at least for me in x86 vfio-pci
> mdevs (or at least I reproduced it when trying to test mdev_tty)
> 
> But if you don't support IOMMUFD, then it probably makes no difference as type1
> doesn't do anything particularly special besides initializing some static data.
> The realize is skipped because you technically don't have a physical host IOMMU
> directly behind the mdev, but rather some parent function related software
> entity doing that for you.
> 
> Zhengzhong noticed there were some other mdevs aside from vfio-pci and in an
> attempt to prevent regression elsewhere it posted for the other mdevs in qemu.


yes. I confirm with :

   -device vfio-ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-e656-4187-b773-fea4e926310d,iommufd=iommufd0 \
   -object iommufd,id=iommufd0 \
   -trace 'iommufd*'

iommufd_cdev_getfd  /dev/vfio/devices/vfio4 (fd=28)
iommufd_backend_connect fd=27 owned=1 users=1
iommufd_cdev_connect_and_bind  [iommufd=27] Successfully bound device 8eb8351a-e656-4187-b773-fea4e926310d (fd=28): output devid=1
iommufd_backend_alloc_ioas  iommufd=27 ioas=2
iommufd_cdev_alloc_ioas  [iommufd=27] new IOMMUFD container with ioasid=2
iommufd_cdev_attach_ioas_hwpt  [iommufd=27] Successfully attached device 8eb8351a-e656-4187-b773-fea4e926310d (28) to id=2
iommufd_backend_map_dma  iommufd=27 ioas=2 iova=0x0 size=0x200000000 addr=0x3fd9ff00000 readonly=0 (0)
iommufd_cdev_device_info  8eb8351a-e656-4187-b773-fea4e926310d (28) num_irqs=1 num_regions=0 flags=33
iommufd_cdev_detach_ioas_hwpt  [iommufd=27] Successfully detached 8eb8351a-e656-4187-b773-fea4e926310d
iommufd_backend_unmap_dma  iommufd=27 ioas=2 iova=0x0 size=0x200000000 (0)
iommufd_backend_free_id  iommufd=27 id=2 (0)
iommufd_backend_disconnect fd=-1 users=0

qemu-kvm: -device vfio-ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-e656-4187-b773-fea4e926310d,iommufd=iommufd0: vfio 8eb8351a-e656-4187-b773-fea4e926310d: Failed to get hardware info: No such file or directory



Thanks,

C.

Re: [PATCH 1/2] vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev

[Anthony Krowiak]

On 7/22/24 5:18 AM, Joao Martins wrote:
> On 22/07/2024 08:07, Zhenzhong Duan wrote:
>> mdevs aren't "physical" devices and when asking for backing IOMMU info,
>> it fails the entire provisioning of the guest. Fix that by setting
>> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
>> presence of mdevs.
>>
>> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


I'm at a bit of a loss here. We've been starting guests with AP devices 
being passed through using VFIO for years. Did the fix for

930589520128 precipitate this? Is there a bugzilla or something that 
describes how this problem was encountered?


> Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
>
>> ---
>>   hw/vfio/ap.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> index 0c4354e3e7..391bfb72ca 100644
>> --- a/hw/vfio/ap.c
>> +++ b/hw/vfio/ap.c
>> @@ -230,6 +230,9 @@ static void vfio_ap_instance_init(Object *obj)
>>        */
>>       vfio_device_init(vbasedev, VFIO_DEVICE_TYPE_AP, &vfio_ap_ops,
>>                        DEVICE(vapdev), true);
>> +
>> +    /* AP device is mdev type device */
>> +    vbasedev->mdev = true;
>>   }
>>   
>>   #ifdef CONFIG_IOMMUFD

Re: [PATCH 1/2] vfio/ap: Don't initialize HOST_IOMMU_DEVICE with mdev

[Joao Martins]

On 22/07/2024 16:46, Anthony Krowiak wrote:
> 
> On 7/22/24 5:18 AM, Joao Martins wrote:
>> On 22/07/2024 08:07, Zhenzhong Duan wrote:
>>> mdevs aren't "physical" devices and when asking for backing IOMMU info,
>>> it fails the entire provisioning of the guest. Fix that by setting
>>> vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
>>> presence of mdevs.
>>>
>>> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize()
>>> handler")
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> 
> 
> I'm at a bit of a loss here. We've been starting guests with AP devices being
> passed through using VFIO for years. Did the fix for
> 
> 930589520128 precipitate this?

Yes. The fix commit ids introduced this and it was not intended. Also the
failure, again, is only reproduceable with IOMMUFD and it doesn't apply for
type1-iommu that you are likely using. Both are different IOMMU backends.

	Joao

Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Eric Farman]

On Mon, 2024-07-22 at 17:36 +0200, Cédric Le Goater wrote:
> On 7/22/24 17:09, Joao Martins wrote:
> > On 22/07/2024 15:57, Eric Farman wrote:
> > > On Mon, 2024-07-22 at 15:07 +0800, Zhenzhong Duan wrote:
> > > > mdevs aren't "physical" devices and when asking for backing IOMMU info,
> > > > it fails the entire provisioning of the guest. Fix that by setting
> > > > vbasedev->mdev true so skipping HostIOMMUDevice initialization in the
> > > > presence of mdevs.
> > > 
> > > Hmm, picking the two commits that Cedric mentioned in his cover-letter reply [1] doesn't "fail the entire provisioning of the guest" for me.
> > > 
> > > Applying this patch on top of that causes the call from vfio_attach_device() to hiod_legacy_vfio_realize() to be skipped, which seems odd. What am I missing?
> > > 
> > > [1] https://lore.kernel.org/qemu-devel/4c9a184b-514c-4276-95ca-9ed86623b9a4@redhat.com/
> > > 
> > 
> > If you are using IOMMUFD
> > 

Which is not the case in defconfig.

> >  it will fail the entire provisioning i.e. GET_HW_INFO
> > fails because there's no actual device/IOMMU you can probe hardware information
> > from and you can't start a guest. This happened at least for me in x86 vfio-pci
> > mdevs (or at least I reproduced it when trying to test mdev_tty)
> > 
> > But if you don't support IOMMUFD, then it probably makes no difference as type1
> > doesn't do anything particularly special besides initializing some static data.

This was my concern. The static data doesn't look particularly exciting, but it does seem strange to
be skipping over it in the non-iommufd case now.

> > The realize is skipped because you technically don't have a physical host IOMMU
> > directly behind the mdev, but rather some parent function related software
> > entity doing that for you.
> > 
> > Zhengzhong noticed there were some other mdevs aside from vfio-pci and in an
> > attempt to prevent regression elsewhere it posted for the other mdevs in qemu.
> 
> 
> yes. I confirm with :
> 
>    -device vfio-ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-e656-4187-b773-fea4e926310d,iommufd=iommufd0 \
>    -object iommufd,id=iommufd0 \
>    -trace 'iommufd*'
> 
> iommufd_cdev_getfd  /dev/vfio/devices/vfio4 (fd=28)

Ah, right... Need to enable iommufd AND vfio_device_cdev to get into this potential situation. I
guess this is better than random failures down the road.

Acked-by: Eric Farman <farman@linux.ibm.com>

> iommufd_backend_connect fd=27 owned=1 users=1
> iommufd_cdev_connect_and_bind  [iommufd=27] Successfully bound device 8eb8351a-e656-4187-b773-fea4e926310d (fd=28): output devid=1
> iommufd_backend_alloc_ioas  iommufd=27 ioas=2
> iommufd_cdev_alloc_ioas  [iommufd=27] new IOMMUFD container with ioasid=2
> iommufd_cdev_attach_ioas_hwpt  [iommufd=27] Successfully attached device 8eb8351a-e656-4187-b773-fea4e926310d (28) to id=2
> iommufd_backend_map_dma  iommufd=27 ioas=2 iova=0x0 size=0x200000000 addr=0x3fd9ff00000 readonly=0 (0)
> iommufd_cdev_device_info  8eb8351a-e656-4187-b773-fea4e926310d (28) num_irqs=1 num_regions=0 flags=33
> iommufd_cdev_detach_ioas_hwpt  [iommufd=27] Successfully detached 8eb8351a-e656-4187-b773-fea4e926310d
> iommufd_backend_unmap_dma  iommufd=27 ioas=2 iova=0x0 size=0x200000000 (0)
> iommufd_backend_free_id  iommufd=27 id=2 (0)
> iommufd_backend_disconnect fd=-1 users=0
> 
> qemu-kvm: -device vfio-ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-e656-4187-b773-fea4e926310d,iommufd=iommufd0: vfio 8eb8351a-e656-4187-b773-fea4e926310d: Failed to get hardware info: No such file or directory
> 
> 
> 
> Thanks,
> 
> C.
> 
> 

RE: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE with mdev

[Duan, Zhenzhong]

>-----Original Message-----
>From: Eric Farman <farman@linux.ibm.com>
>Subject: Re: [PATCH 2/2] vfio/ccw: Don't initialize HOST_IOMMU_DEVICE
>with mdev
>
>On Mon, 2024-07-22 at 17:36 +0200, Cédric Le Goater wrote:
>> On 7/22/24 17:09, Joao Martins wrote:
>> > On 22/07/2024 15:57, Eric Farman wrote:
>> > > On Mon, 2024-07-22 at 15:07 +0800, Zhenzhong Duan wrote:
>> > > > mdevs aren't "physical" devices and when asking for backing IOMMU
>info,
>> > > > it fails the entire provisioning of the guest. Fix that by setting
>> > > > vbasedev->mdev true so skipping HostIOMMUDevice initialization in
>the
>> > > > presence of mdevs.
>> > >
>> > > Hmm, picking the two commits that Cedric mentioned in his cover-
>letter reply [1] doesn't "fail the entire provisioning of the guest" for me.
>> > >
>> > > Applying this patch on top of that causes the call from
>vfio_attach_device() to hiod_legacy_vfio_realize() to be skipped, which
>seems odd. What am I missing?
>> > >
>> > > [1] https://lore.kernel.org/qemu-devel/4c9a184b-514c-4276-95ca-
>9ed86623b9a4@redhat.com/
>> > >
>> >
>> > If you are using IOMMUFD
>> >
>
>Which is not the case in defconfig.
>
>> >  it will fail the entire provisioning i.e. GET_HW_INFO
>> > fails because there's no actual device/IOMMU you can probe hardware
>information
>> > from and you can't start a guest. This happened at least for me in x86
>vfio-pci
>> > mdevs (or at least I reproduced it when trying to test mdev_tty)
>> >
>> > But if you don't support IOMMUFD, then it probably makes no difference
>as type1
>> > doesn't do anything particularly special besides initializing some static
>data.
>
>This was my concern. The static data doesn't look particularly exciting, but it
>does seem strange to
>be skipping over it in the non-iommufd case now.

Thanks Joao and Cédric for helping explain and confirm.

Yes, after this fix HostIOMMUDevice is totally bypassed for mdev even in non-iommufd case.
In non-iommufd case, the only supported HostIOMMUDevice capability is aw_bits which is calculated through bcontainer->iova_ranges which is always NULL for mdev.
So HOST_IOMMU_DEVICE_CAP_AW_BITS_MAX(64) is returned which is larger enough that vIOMMU can safely ignore. Then we can safely bypass entire HostIOMMUDevice for mdev.

Thanks
Zhenzhong

>
>> > The realize is skipped because you technically don't have a physical host
>IOMMU
>> > directly behind the mdev, but rather some parent function related
>software
>> > entity doing that for you.
>> >
>> > Zhengzhong noticed there were some other mdevs aside from vfio-pci
>and in an
>> > attempt to prevent regression elsewhere it posted for the other mdevs in
>qemu.
>>
>>
>> yes. I confirm with :
>>
>>    -device vfio-ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-
>e656-4187-b773-fea4e926310d,iommufd=iommufd0 \
>>    -object iommufd,id=iommufd0 \
>>    -trace 'iommufd*'
>>
>> iommufd_cdev_getfd  /dev/vfio/devices/vfio4 (fd=28)
>
>Ah, right... Need to enable iommufd AND vfio_device_cdev to get into this
>potential situation. I
>guess this is better than random failures down the road.
>
>Acked-by: Eric Farman <farman@linux.ibm.com>
>
>> iommufd_backend_connect fd=27 owned=1 users=1
>> iommufd_cdev_connect_and_bind  [iommufd=27] Successfully bound
>device 8eb8351a-e656-4187-b773-fea4e926310d (fd=28): output devid=1
>> iommufd_backend_alloc_ioas  iommufd=27 ioas=2
>> iommufd_cdev_alloc_ioas  [iommufd=27] new IOMMUFD container with
>ioasid=2
>> iommufd_cdev_attach_ioas_hwpt  [iommufd=27] Successfully attached
>device 8eb8351a-e656-4187-b773-fea4e926310d (28) to id=2
>> iommufd_backend_map_dma  iommufd=27 ioas=2 iova=0x0
>size=0x200000000 addr=0x3fd9ff00000 readonly=0 (0)
>> iommufd_cdev_device_info  8eb8351a-e656-4187-b773-fea4e926310d
>(28) num_irqs=1 num_regions=0 flags=33
>> iommufd_cdev_detach_ioas_hwpt  [iommufd=27] Successfully detached
>8eb8351a-e656-4187-b773-fea4e926310d
>> iommufd_backend_unmap_dma  iommufd=27 ioas=2 iova=0x0
>size=0x200000000 (0)
>> iommufd_backend_free_id  iommufd=27 id=2 (0)
>> iommufd_backend_disconnect fd=-1 users=0
>>
>> qemu-kvm: -device vfio-
>ap,id=hostdev0,sysfsdev=/sys/bus/mdev/devices/8eb8351a-e656-4187-
>b773-fea4e926310d,iommufd=iommufd0: vfio 8eb8351a-e656-4187-b773-
>fea4e926310d: Failed to get hardware info: No such file or directory
>>
>>
>>
>> Thanks,
>>
>> C.
>>
>>