From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753154AbdF2N1c (ORCPT ); Thu, 29 Jun 2017 09:27:32 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:59186 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752829AbdF2N1Z (ORCPT ); Thu, 29 Jun 2017 09:27:25 -0400 Date: Thu, 29 Jun 2017 15:27:16 +0200 From: Gerald Schaefer To: Joerg Roedel Cc: Sebastian Ott , iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org, jroedel@suse.de Subject: Re: [PATCH 2/2] iommu/s390: Add support for iommu_device handling In-Reply-To: <20170627152806.GZ30388@8bytes.org> References: <1497532312-30470-1-git-send-email-joro@8bytes.org> <1497532312-30470-3-git-send-email-joro@8bytes.org> <20170619170219.0a8626b6@thinkpad> <20170627152806.GZ30388@8bytes.org> Organization: IBM Deutschland Research & Development GmbH / Vorsitzende des Aufsichtsrats: Martina Koederitz / Geschaeftsfuehrung: Dirk Wittkopp / Sitz der Gesellschaft: Boeblingen / Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.9.0 (GTK+ 2.24.23; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17062913-0040-0000-0000-000003B75D03 X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17062913-0041-0000-0000-000025B15CB6 Message-Id: <20170629152716.1c888f20@thinkpad> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-06-29_09:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=2 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1703280000 definitions=main-1706290220 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 27 Jun 2017 17:28:06 +0200 Joerg Roedel 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 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