Messages in this thread | | | Date | Thu, 29 Jun 2017 15:27:16 +0200 | From | Gerald Schaefer <> | Subject | Re: [PATCH 2/2] iommu/s390: Add support for iommu_device handling |
| |
On Tue, 27 Jun 2017 17:28:06 +0200 Joerg Roedel <joro@8bytes.org> wrote:
> 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@8bytes.org> 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 :)
Ok, thanks for the explanation. So it should not be a problem if the attributes are registered in zpci_create_device() before the device is registered to the bus, especially since they do not provide any manipulation function but only topology information.
BTW, I get the following new attributes with this patch: /sys/devices/pci0000:00/0000:00:00.0/iommu /sys/devices/virtual/iommu /sys/devices/virtual/iommu/s390-iommu.00000012 /sys/class/iommu/s390-iommu.00000012
Regards, Gerald
| |