[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFCv1 02/10] hw/arm/virt: Add iommufd link to virt-machine
From: |
Nicolin Chen |
Subject: |
Re: [PATCH RFCv1 02/10] hw/arm/virt: Add iommufd link to virt-machine |
Date: |
Tue, 9 Jul 2024 10:18:59 -0700 |
On Tue, Jul 09, 2024 at 07:06:50PM +0200, Eric Auger wrote:
> On 7/9/24 18:59, Nicolin Chen wrote:
> > Hi Eric,
> >
> > Thanks for the comments!
> >
> > On Tue, Jul 09, 2024 at 11:11:56AM +0200, Eric Auger wrote:
> >> On 6/26/24 02:28, Nicolin Chen wrote:
> >>> A nested SMMU must use iommufd ioctls to communicate with the host-level
> >>> SMMU instance for 2-stage translation support. Add an iommufd link to the
> >>> ARM virt-machine, allowing QEMU command to pass in an iommufd object.
> >> If I am not wrong vfio devices are allowed to use different iommufd's
> >> (although there is no real benefice). So this command line wouldn't
> >> match with that option.
> > I think Jason's remarks highlighted that FD should be one per VM:
> > https://lore.kernel.org/qemu-devel/20240503141024.GE3341011@nvidia.com/
> OK I thought this was still envisionned althought not really meaningful.
> By the way, please add Yi and Zhenzhong in cc since thre problematics
> are connected I think.
Yea.
Yi/Zhenzhong, would you please shed some light on forwarding an
iommufd handler to the intel_iommu code? IIRC, we did that at the
beginning but removed it later?
> >> Also while reading the commit msg it is not clear with the iommufd is
> >> needed in the machine whereas the vfio iommufd BE generally calls those
> >> ioctls.
> > I think I forgot to revisit it. Both intel_iommu and smmu-common
> > used to call iommufd_backend_connect() for counting, so there was
> > a need to pass in the same iommufd handler to the viommu driver.
> > For SMMU, since it is created in the virt code, we had to pass in
> > with this patch.
> >
> > That being said, it looks like intel_iommu had removed that. So,
> > likely we don't need an extra user counting for SMMU too.
> OK at least it deserves some explanation about the "why"
Yes, I agree that the commit message isn't good enough.
Thanks
Nicolin