Re: [PATCH 2/2] iommu/s390: Add support for iommu_device handling

[Joerg Roedel]

On Mon, Jun 19, 2017 at 05:02:19PM +0200, Gerald Schaefer wrote:
> On Thu, 15 Jun 2017 15:11:52 +0200
> Joerg Roedel <joro@xxxxxxxxxx> wrote:
> > +	rc = zpci_init_iommu(zdev);
> > +	if (rc)
> > +		goto out_free;
> > +
> 
> After this point, there are two options for failure (zpci_enable_device +
> zpci_scan_bus), but missing error handling with an appropriate call to
> zpci_destroy_iommu().

Right, I'll fix that in the next version.

> I must admit that I do not understand what these sysfs attributes are
> used for, and by whom and when. Is it really necessary/correct to register
> them (including the s390_iommu_ops) _before_ the zpci_dev is registered
> to the bus (zpci_scan_bus -> pci_scan_root_bus/pci_bus_add_devices)?
> 
> What is the expected life cycle for the attributes, and are they already
> needed when a device is still under DMA API access, or even before it is
> added to the PCI bus?

The sysfs attributes are used by user-space for topology detection. A
user-space program using VFIO needs to know which PCI-devices it needs
to assign to the VFIO driver in order to gain access.

On s390 this probably doesn't matter much, as the real PCI topology is
not exposed, but it matters on other architectures. The purpose of this
patch is not so much the sysfs attributes. Its a step to convert all
IOMMU drivers to use the iommu_device_register() interface. Goal is that
the iommu core code knows about individual hardware iommus and the
devices behind it.

When this is implemented in all iommu drivers, we can start moving more
code out of the drivers into the iommu core. This also probably doesn't
matter much for s390, but all iommu drivers need to use this interface
before we can move on. The sysfs-stuff is just a by-product of that.

So to move on with that, it would be good to get an Acked-by on this
patch from you guys once you think I fixed everything :)


Regards,

	Joerg