Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1675888 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 2/2] iommu/s390: Add support for iommu_device handling Joerg Roedel <joro@8bytes.org> - 2017-06-27 17:30 +0200
    Re: [PATCH 2/2] iommu/s390: Add support for iommu_device handling Gerald Schaefer <gerald.schaefer@de.ibm.com> - 2017-06-29 15:30 +0200

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

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 :)


Regards,

	Joerg

[toc] | [next] | [standalone]

#1677747

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web