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.
> 
>