Re: [PATCH] iommu/intel-iommu: set as DUMMY_DEVICE_DOMAIN_INFO if no IOMMU
Lu Baolu 於 2020年2月6日 週四 下午6:49寫道: > > Hi, > > On 2020/2/5 18:06, Jian-Hong Pan wrote: > > Lu Baolu 於 2020年2月5日 週三 上午9:28寫道: > >> > >> Hi, > >> > >> On 2020/2/4 17:25, Jian-Hong Pan wrote: > >>> Lu Baolu 於 2020年2月4日 週二 下午2:11寫道: > > Hi, > > On 2020/2/3 17:10, Jian-Hong Pan wrote: > > If the device has no IOMMU, it still invokes iommu_need_mapping during > > intel_alloc_coherent. However, iommu_need_mapping can only check the > > device is DUMMY_DEVICE_DOMAIN_INFO or not. This patch marks the device > > is a DUMMY_DEVICE_DOMAIN_INFO if the device has no IOMMU. > > > > Signed-off-by: Jian-Hong Pan > > --- > > drivers/iommu/intel-iommu.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > > index 35a4a3abedc6..878bc986a015 100644 > > --- a/drivers/iommu/intel-iommu.c > > +++ b/drivers/iommu/intel-iommu.c > > @@ -5612,8 +5612,10 @@ static int intel_iommu_add_device(struct device > > *dev) > > int ret; > > > > iommu = device_to_iommu(dev, &bus, &devfn); > > - if (!iommu) > > + if (!iommu) { > > + dev->archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO; > > Is this a DMA capable device? > >>> > >>> Do you mean is the device in DMA Remapping table? > >>> Dump DMAR from ACPI table. The device is not in the table. > >>> So, it does not support DMAR, Intel IOMMU. > >>> > >>> Or, should device_to_iommu be invoked in iommu_need_mapping to check > >>> IOMMU feature again? > >>> > >> > >> Normally intel_iommu_add_device() should only be called for PCI devices > >> and APCI name space devices (reported in ACPI/DMAR table). In both > >> cases, device_to_iommu() should always return a corresponding iommu. I > >> just tried to understand why it failed for your case. > > > > We found all of the DMAR featured devices's PCI Segment Number is **. > > But the devices locating under segment/domain *0001* hit the issue, > > until the patch is applied. > > > > Because of different segment numbers, none of iommu will be matched by > > for_each_active_iommu(iommu, drhd) loop in function device_to_iommu() > > and it will return NULL. So, intel_iommu_add_device() returns no > > device. > > > > I can share the DMAR: > > /* > > * Intel ACPI Component Architecture > > * AML/ASL+ Disassembler version 20200110 (64-bit version) > > * Copyright (c) 2000 - 2020 Intel Corporation > > * > > * Disassembly of dmar.dat, Wed Jan 22 11:41:50 2020 > > * > > * ACPI Data Table [DMAR] > > * > > * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue > > */ > > > > [000h 4]Signature : "DMAR"[DMA Remapping > > table] > > [004h 0004 4] Table Length : 00A8 > > [008h 0008 1] Revision : 01 > > [009h 0009 1] Checksum : 5E > > [00Ah 0010 6] Oem ID : "INTEL " > > [010h 0016 8] Oem Table ID : "EDK2" > > [018h 0024 4] Oem Revision : 0002 > > [01Ch 0028 4] Asl Compiler ID : "" > > [020h 0032 4]Asl Compiler Revision : 0113 > > > > [024h 0036 1] Host Address Width : 26 > > [025h 0037 1]Flags : 05 > > [026h 0038 10] Reserved : 00 00 00 00 00 00 00 00 00 00 > > > > [030h 0048 2]Subtable Type : [Hardware Unit > > Definition] > > [032h 0050 2] Length : 0018 > > > > [034h 0052 1]Flags : 00 > > [035h 0053 1] Reserved : 00 > > [036h 0054 2] PCI Segment Number : > > [038h 0056 8]Register Base Address : FED9 > > > > [040h 0064 1]Device Scope Type : 01 [PCI Endpoint Device] > > [041h 0065 1] Entry Length : 08 > > [042h 0066 2] Reserved : > > [044h 0068 1] Enumeration ID : 00 > > [045h 0069 1] PCI Bus Number : 00 > > > > [046h 0070 2] PCI Path : 02,00 > > > > > > [048h 0072 2]Subtable Type : [Hardware Unit > > Definition] > > [04Ah 0074 2] Length : 0020 > > > > [04Ch 0076 1]Flags : 01 > > [04Dh 0077 1] Reserved : 00 > > [04Eh 0078 2] PCI Segment Number : > > [050h 0080 8]Register Base Address : FED91000 > > > > [058h 0088 1]Device Scope Type : 03 [IOAPIC Device] > > [059h 0089 1] Entry Length : 08 > > [05Ah 0090 2] Reserved : > > [05Ch 0092 1] Enumeration ID : 02 > > [05Dh 0093 1] PCI Bus Number : 00 > > > > [05Eh 0094 2] PCI Path : 1E,07 > > > > > > [060h 0096
Re: Seeing some another issue with mixed domains in the same iommu_group
Hi Jerry, On 2020/2/7 7:16, Jerry Snitselaar wrote: Hi Baolu, Would something along these lines makes sense? diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 9dc37672bf89..40cc8f5a3ebb 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3614,6 +3614,20 @@ static bool iommu_need_mapping(struct device *dev) } dmar_remove_one_dev_info(dev); get_private_domain_for_dev(dev); + } else { + if (dev->archdata.iommu == NULL) { + struct iommu_domain *domain; + struct iommu_group *group; + struct dmar_domain *dmar_domain, *tmp; + + group = iommu_group_get_for_dev(dev); + domain = iommu_group_default_domain(group); + dmar_domain = to_dmar_domain(domain); + tmp = set_domain_for_dev(dev, dmar_domain); + } } dev_info(dev, "32bit DMA uses non-identity mapping\n"); Thanks for reporting. Actually, I prefer to removing this domain switch as long as users are able to make a 32-bit device use DMA domain while system default is identity or it breaks anything. 32-bit devices (or normally devices with limited addressing capability over the whole system memory) using DMA domain helps by removing the swiotlb performance overhead, which is the original motivation of this code. Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Seeing some another issue with mixed domains in the same iommu_group
On Thu Feb 06 20, Jerry Snitselaar wrote: ... The above cases seem to be avoided by: 9235cb13d7d1 | 2020-01-24 | iommu/vt-d: Allow devices with RMRRs to use identity domain (Lu Baolu) which results in the watchdog device no longer taking a dma domain and switching the group default. Without that patch though when it gets into the iommu_need_mapping code for :01:00.4 after the following: dmar_remove_one_dev_info(dev); ret = iommu_request_dma_domain_for_dev(dev); ret is 0 and dev->archdata.iommu is NULL. Even with 9235cb13d7d1 device_def_domain_type can return return dma, but I'm not sure how likely it is for there to be an iommu group like that again where the group default ends up dma, a device gets removed and added to the identity domain, and then ends up in that code in iommu_need_mapping. Hi Baolu, Would something along these lines makes sense? diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 9dc37672bf89..40cc8f5a3ebb 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -3614,6 +3614,20 @@ static bool iommu_need_mapping(struct device *dev) } dmar_remove_one_dev_info(dev); get_private_domain_for_dev(dev); + } else { + if (dev->archdata.iommu == NULL) { + struct iommu_domain *domain; + struct iommu_group *group; + struct dmar_domain *dmar_domain, *tmp; + + group = iommu_group_get_for_dev(dev); + domain = iommu_group_default_domain(group); + dmar_domain = to_dmar_domain(domain); + tmp = set_domain_for_dev(dev, dmar_domain); + } } dev_info(dev, "32bit DMA uses non-identity mapping\n"); -- Obviously needs some checks added, but this was just an initial test I was trying. Regards, Jerry ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Seeing some another issue with mixed domains in the same iommu_group
On Thu Feb 06 20, Jerry Snitselaar wrote: On Thu Feb 06 20, Jerry Snitselaar wrote: Hi Baolu, I'm seeing another issue with the devices in the HP ilo when the system is booted with intel_iommu=on and iommu=pt (iommu=nopt does not run into problems). first system: 01:00.0 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support (rev 05) 01:00.1 VGA compatible controller: Matrox Electronics Systems Ltd. MGA G200EH 01:00.2 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging (rev 05) 01:00.4 USB controller: Hewlett-Packard Company Integrated Lights-Out Standard Virtual USB Controller (rev 02) [ 21.208103] pci :01:00.0: Adding to iommu group 24 [ 21.210911] pci :01:00.0: Using iommu dma mapping [ 21.212635] pci :01:00.1: Adding to iommu group 24 [ 21.214326] pci :01:00.1: Device uses a private identity domain. [ 21.216507] pci :01:00.2: Adding to iommu group 24 [ 21.618173] pci :01:00.4: Adding to iommu group 24 [ 21.619839] pci :01:00.4: Device uses a private identity domain. [ 26.206832] uhci_hcd: USB Universal Host Controller Interface driver [ 26.209044] uhci_hcd :01:00.4: UHCI Host Controller [ 26.210897] uhci_hcd :01:00.4: new USB bus registered, assigned bus number 3 [ 26.213247] uhci_hcd :01:00.4: detected 8 ports [ 26.214810] uhci_hcd :01:00.4: port count misdetected? forcing to 2 ports [ 26.217153] uhci_hcd :01:00.4: irq 16, io base 0x3c00 [ 26.219171] uhci_hcd :01:00.4: 32bit DMA uses non-identity mapping [ 26.221261] uhci_hcd :01:00.4: unable to allocate consistent memory for frame list [ 26.223787] uhci_hcd :01:00.4: startup error -16 [ 26.225381] uhci_hcd :01:00.4: USB bus 3 deregistered [ 26.227378] uhci_hcd :01:00.4: init :01:00.4 fail, -16 [ 26.229296] uhci_hcd: probe of :01:00.4 failed with error -16 different system with similar issue: 01:00.0 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support [103c:3306] (rev 07) 01:00.1 VGA compatible controller [0300]: Matrox Electronics Systems Ltd. MGA G200eH3 [102b:0538] (rev 02) (prog-if 00 [VGA controller]) 01:00.2 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging [103c:3307] (rev 07) 01:00.4 USB controller [0c03]: Hewlett-Packard Company iLO5 Virtual USB Controller [103c:22f6] (prog-if 20 [EHCI]) [ 13.695663] pci :01:00.0: Adding to iommu group 10 [ 13.703667] pci :01:00.0: Using iommu dma mapping [ 13.708871] pci :01:00.1: Adding to iommu group 10 [ 13.714033] pci :01:00.1: DMAR: Device uses a private identity domain. [ 13.721033] pci :01:00.2: Adding to iommu group 10 [ 13.726290] pci :01:00.4: Adding to iommu group 10 [ 13.731453] pci :01:00.4: DMAR: Device uses a private identity domain. [ 17.157796] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [ 17.164348] ehci-pci: EHCI PCI platform driver [ 17.170061] ehci-pci :01:00.4: EHCI Host Controller [ 17.175457] ehci-pci :01:00.4: new USB bus registered, assigned bus number 1 [ 17.182912] ehci-pci :01:00.4: DMAR: 32bit DMA uses non-identity mapping [ 17.189988] ehci-pci :01:00.4: can't setup: -12 [ 17.194884] ehci-pci :01:00.4: USB bus 1 deregistered [ 17.200567] ehci-pci :01:00.4: init :01:00.4 fail, -12 [ 17.206508] ehci-pci: probe of :01:00.4 failed with error -12 I'm looking through the code and trying to debug it, but any thoughts on this? Regards, Jerry In iommu_need_mapping, in a case like the above does something like dmar_insert_one_dev_info need to happen to associate the device back with the group default domain? In intel_iommu_add_device it is going to get removed and added to the identity domain, and then in iommu_need_mapping it gets removed from the identity domain, and iommu_request_dma_domain_for_dev should return 0 because the group default domain at this point is the correct type. The above cases seem to be avoided by: 9235cb13d7d1 | 2020-01-24 | iommu/vt-d: Allow devices with RMRRs to use identity domain (Lu Baolu) which results in the watchdog device no longer taking a dma domain and switching the group default. Without that patch though when it gets into the iommu_need_mapping code for :01:00.4 after the following: dmar_remove_one_dev_info(dev); ret = iommu_request_dma_domain_for_dev(dev); ret is 0 and dev->archdata.iommu is NULL. Even with 9235cb13d7d1 device_def_domain_type can return return dma, but I'm not sure how likely it is for there to be an iommu group like that again where the group default ends up dma, a device gets removed and added to the identity domain, and then ends up in that code in iommu_need_mapping.
Re: Seeing some another issue with mixed domains in the same iommu_group
On Thu Feb 06 20, Jerry Snitselaar wrote: Hi Baolu, I'm seeing another issue with the devices in the HP ilo when the system is booted with intel_iommu=on and iommu=pt (iommu=nopt does not run into problems). first system: 01:00.0 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support (rev 05) 01:00.1 VGA compatible controller: Matrox Electronics Systems Ltd. MGA G200EH 01:00.2 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging (rev 05) 01:00.4 USB controller: Hewlett-Packard Company Integrated Lights-Out Standard Virtual USB Controller (rev 02) [ 21.208103] pci :01:00.0: Adding to iommu group 24 [ 21.210911] pci :01:00.0: Using iommu dma mapping [ 21.212635] pci :01:00.1: Adding to iommu group 24 [ 21.214326] pci :01:00.1: Device uses a private identity domain. [ 21.216507] pci :01:00.2: Adding to iommu group 24 [ 21.618173] pci :01:00.4: Adding to iommu group 24 [ 21.619839] pci :01:00.4: Device uses a private identity domain. [ 26.206832] uhci_hcd: USB Universal Host Controller Interface driver [ 26.209044] uhci_hcd :01:00.4: UHCI Host Controller [ 26.210897] uhci_hcd :01:00.4: new USB bus registered, assigned bus number 3 [ 26.213247] uhci_hcd :01:00.4: detected 8 ports [ 26.214810] uhci_hcd :01:00.4: port count misdetected? forcing to 2 ports [ 26.217153] uhci_hcd :01:00.4: irq 16, io base 0x3c00 [ 26.219171] uhci_hcd :01:00.4: 32bit DMA uses non-identity mapping [ 26.221261] uhci_hcd :01:00.4: unable to allocate consistent memory for frame list [ 26.223787] uhci_hcd :01:00.4: startup error -16 [ 26.225381] uhci_hcd :01:00.4: USB bus 3 deregistered [ 26.227378] uhci_hcd :01:00.4: init :01:00.4 fail, -16 [ 26.229296] uhci_hcd: probe of :01:00.4 failed with error -16 different system with similar issue: 01:00.0 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support [103c:3306] (rev 07) 01:00.1 VGA compatible controller [0300]: Matrox Electronics Systems Ltd. MGA G200eH3 [102b:0538] (rev 02) (prog-if 00 [VGA controller]) 01:00.2 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging [103c:3307] (rev 07) 01:00.4 USB controller [0c03]: Hewlett-Packard Company iLO5 Virtual USB Controller [103c:22f6] (prog-if 20 [EHCI]) [ 13.695663] pci :01:00.0: Adding to iommu group 10 [ 13.703667] pci :01:00.0: Using iommu dma mapping [ 13.708871] pci :01:00.1: Adding to iommu group 10 [ 13.714033] pci :01:00.1: DMAR: Device uses a private identity domain. [ 13.721033] pci :01:00.2: Adding to iommu group 10 [ 13.726290] pci :01:00.4: Adding to iommu group 10 [ 13.731453] pci :01:00.4: DMAR: Device uses a private identity domain. [ 17.157796] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [ 17.164348] ehci-pci: EHCI PCI platform driver [ 17.170061] ehci-pci :01:00.4: EHCI Host Controller [ 17.175457] ehci-pci :01:00.4: new USB bus registered, assigned bus number 1 [ 17.182912] ehci-pci :01:00.4: DMAR: 32bit DMA uses non-identity mapping [ 17.189988] ehci-pci :01:00.4: can't setup: -12 [ 17.194884] ehci-pci :01:00.4: USB bus 1 deregistered [ 17.200567] ehci-pci :01:00.4: init :01:00.4 fail, -12 [ 17.206508] ehci-pci: probe of :01:00.4 failed with error -12 I'm looking through the code and trying to debug it, but any thoughts on this? Regards, Jerry In iommu_need_mapping, in a case like the above does something like dmar_insert_one_dev_info need to happen to associate the device back with the group default domain? In intel_iommu_add_device it is going to get removed and added to the identity domain, and then in iommu_need_mapping it gets removed from the identity domain, and iommu_request_dma_domain_for_dev should return 0 because the group default domain at this point is the correct type. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] iommu/uapi: Define uapi version and capabilities
On Thu, 6 Feb 2020 11:14:27 +0100 Auger Eric wrote: > Hi Jacob, > On 1/29/20 7:02 AM, Jacob Pan wrote: > > Define a unified UAPI version to be used for compatibility > > checks between user and kernel. > > > > Signed-off-by: Liu Yi L > > Signed-off-by: Jacob Pan > > --- > > include/uapi/linux/iommu.h | 48 > > ++ 1 file changed, 48 > > insertions(+) > > > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > > index fcafb6401430..65a26c2519ee 100644 > > --- a/include/uapi/linux/iommu.h > > +++ b/include/uapi/linux/iommu.h > > @@ -8,6 +8,54 @@ > > > > #include > > > > +/** > > + * Current version of the IOMMU user API. This is intended for > > query > > + * between user and kernel to determine compatible data structures. > > + * > > + * Having a single UAPI version to govern the user-kernel data > > structures > > + * makes compatibility check straightforward. On the contrary, > > supporting > > + * combinations of multiple versions of the data can be a > > nightmare. > I would rather put the above justification in the commit msg and not > here. make sense. > > + * > > + * UAPI version can be bumped up with the following rules: > > + * 1. All data structures passed between user and kernel space > > share > > + *the same version number. i.e. any extension to to any > > structure > s/to to/to will fix. > > + *results in version bump up. > in a version number increment? sounds good, more specific. > > + * > > + * 2. Data structures are open to extension but closed to > > modification.> + *New fields must be added at the end of each > > data structure with > > + *64bit alignment. Flag bits can be added without size change > > but > > + *existing ones cannot be altered. > > + * > > + * 3. Versions are backward compatible. > > + * > > + * 4. Version to size lookup is supported by kernel internal API > > for each > > + *API function type. @version is mandatory for new data > > structures > > + *and must be at the beginning with type of __u32. > > + */ > > +#define IOMMU_UAPI_VERSION 1 > > +static inline int iommu_get_uapi_version(void) > > +{ > > + return IOMMU_UAPI_VERSION; > > +} > > + > > +/* > > + * Supported UAPI features that can be reported to user space. > > + * These types represent the capability available in the kernel. > > + * > > + * REVISIT: UAPI version also implies the capabilities. Should we > > + * report them explicitly? > > + */ > > +enum IOMMU_UAPI_DATA_TYPES { > > + IOMMU_UAPI_BIND_GPASID, > > + IOMMU_UAPI_CACHE_INVAL, > > + IOMMU_UAPI_PAGE_RESP, > > + NR_IOMMU_UAPI_TYPE, > > +}; > > + > > +#define IOMMU_UAPI_CAP_MASK ((1 << IOMMU_UAPI_BIND_GPASID) > > | \ > > + (1 << IOMMU_UAPI_CACHE_INVAL) > > | \ > > + (1 << IOMMU_UAPI_PAGE_RESP)) > > + > > #define IOMMU_FAULT_PERM_READ (1 << 0) /* read */ > > #define IOMMU_FAULT_PERM_WRITE (1 << 1) /* write */ > > #define IOMMU_FAULT_PERM_EXEC (1 << 2) /* exec */ > > > Thanks > > Eric > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v3 1/8] vfio: Add VFIO_IOMMU_PASID_REQUEST(alloc/free)
Hi Alex, On Fri, 31 Jan 2020 12:41:06 + "Liu, Yi L" wrote: > > > +static int vfio_iommu_type1_pasid_free(struct vfio_iommu *iommu, > > > +unsigned int pasid) > > > +{ > > > + struct vfio_mm *vmm = iommu->vmm; > > > + int ret = 0; > > > + > > > + mutex_lock(&iommu->lock); > > > + if (!IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu)) { > > > > But we could have been IOMMU backed when the pasid was allocated, > > did we just leak something? In fact, I didn't spot anything in > > this series that handles a container with pasids allocated losing > > iommu backing. I'd think we want to release all pasids when that > > happens since permission for the user to hold pasids goes along > > with having an iommu backed device. > > oh, yes. If a container lose iommu backend, then needs to reclaim the > allocated PASIDs. right? I'll add it. :-) > > > Also, do we want _free() paths that can fail? > > I remember we discussed if a _free() path can fail, I think we agreed > to let _free() path always success. :-) Just to add some details. We introduced IOASID notifier such that when VFIO frees a PASID, consumers such as IOMMU, can do the cleanup therefore ensure free always succeeds. https://www.spinics.net/lists/kernel/msg3349928.html https://www.spinics.net/lists/kernel/msg3349930.html This was not in my v9 set as I was considering some race conditions w.r.t. registering notifier, gets notifications, and free call. I will post it in v10. Thanks, Jacob ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Seeing some another issue with mixed domains in the same iommu_group
Hi Baolu, I'm seeing another issue with the devices in the HP ilo when the system is booted with intel_iommu=on and iommu=pt (iommu=nopt does not run into problems). first system: 01:00.0 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support (rev 05) 01:00.1 VGA compatible controller: Matrox Electronics Systems Ltd. MGA G200EH 01:00.2 System peripheral: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging (rev 05) 01:00.4 USB controller: Hewlett-Packard Company Integrated Lights-Out Standard Virtual USB Controller (rev 02) [ 21.208103] pci :01:00.0: Adding to iommu group 24 [ 21.210911] pci :01:00.0: Using iommu dma mapping [ 21.212635] pci :01:00.1: Adding to iommu group 24 [ 21.214326] pci :01:00.1: Device uses a private identity domain. [ 21.216507] pci :01:00.2: Adding to iommu group 24 [ 21.618173] pci :01:00.4: Adding to iommu group 24 [ 21.619839] pci :01:00.4: Device uses a private identity domain. [ 26.206832] uhci_hcd: USB Universal Host Controller Interface driver [ 26.209044] uhci_hcd :01:00.4: UHCI Host Controller [ 26.210897] uhci_hcd :01:00.4: new USB bus registered, assigned bus number 3 [ 26.213247] uhci_hcd :01:00.4: detected 8 ports [ 26.214810] uhci_hcd :01:00.4: port count misdetected? forcing to 2 ports [ 26.217153] uhci_hcd :01:00.4: irq 16, io base 0x3c00 [ 26.219171] uhci_hcd :01:00.4: 32bit DMA uses non-identity mapping [ 26.221261] uhci_hcd :01:00.4: unable to allocate consistent memory for frame list [ 26.223787] uhci_hcd :01:00.4: startup error -16 [ 26.225381] uhci_hcd :01:00.4: USB bus 3 deregistered [ 26.227378] uhci_hcd :01:00.4: init :01:00.4 fail, -16 [ 26.229296] uhci_hcd: probe of :01:00.4 failed with error -16 different system with similar issue: 01:00.0 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Slave Instrumentation & System Support [103c:3306] (rev 07) 01:00.1 VGA compatible controller [0300]: Matrox Electronics Systems Ltd. MGA G200eH3 [102b:0538] (rev 02) (prog-if 00 [VGA controller]) 01:00.2 System peripheral [0880]: Hewlett-Packard Company Integrated Lights-Out Standard Management Processor Support and Messaging [103c:3307] (rev 07) 01:00.4 USB controller [0c03]: Hewlett-Packard Company iLO5 Virtual USB Controller [103c:22f6] (prog-if 20 [EHCI]) [ 13.695663] pci :01:00.0: Adding to iommu group 10 [ 13.703667] pci :01:00.0: Using iommu dma mapping [ 13.708871] pci :01:00.1: Adding to iommu group 10 [ 13.714033] pci :01:00.1: DMAR: Device uses a private identity domain. [ 13.721033] pci :01:00.2: Adding to iommu group 10 [ 13.726290] pci :01:00.4: Adding to iommu group 10 [ 13.731453] pci :01:00.4: DMAR: Device uses a private identity domain. [ 17.157796] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [ 17.164348] ehci-pci: EHCI PCI platform driver [ 17.170061] ehci-pci :01:00.4: EHCI Host Controller [ 17.175457] ehci-pci :01:00.4: new USB bus registered, assigned bus number 1 [ 17.182912] ehci-pci :01:00.4: DMAR: 32bit DMA uses non-identity mapping [ 17.189988] ehci-pci :01:00.4: can't setup: -12 [ 17.194884] ehci-pci :01:00.4: USB bus 1 deregistered [ 17.200567] ehci-pci :01:00.4: init :01:00.4 fail, -12 [ 17.206508] ehci-pci: probe of :01:00.4 failed with error -12 I'm looking through the code and trying to debug it, but any thoughts on this? Regards, Jerry ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: warning from domain_get_iommu
On Tue Feb 04 20, Jerry Snitselaar wrote: I'm working on getting a system to reproduce this, and verify it also occurs with 5.5, but I have a report of a case where the kdump kernel gives warnings like the following on a hp dl360 gen9: [2.830589] ehci_hcd: USB 2.0 'Enhanced' Host Controller (EHCI) Driver [2.832615] ehci-pci: EHCI PCI platform driver [2.834190] ehci-pci :00:1a.0: EHCI Host Controller [2.835974] ehci-pci :00:1a.0: new USB bus registered, assigned bus number 1 [2.838276] ehci-pci :00:1a.0: debug port 2 [2.839700] WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:598 domain_get_iommu+0x55/0x60 [2.840671] Modules linked in: [2.840671] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.18.0-170.el8.kdump2.x86_64 #1 [2.840671] Hardware name: HP ProLiant DL360 Gen9/ProLiant DL360 Gen9, BIOS P89 07/21/2019 [2.840671] RIP: 0010:domain_get_iommu+0x55/0x60 [2.840671] Code: c2 01 eb 0b 48 83 c0 01 8b 34 87 85 f6 75 0b 48 63 c8 48 39 c2 75 ed 31 c0 c3 48 c1 e1 03 48 8b 05 70 f3 91 01 48 8b 04 08 c3 <0f> 0b 31 c0 c3 31 c9 eb eb 66 90 0f 1f 44 00 00 41 55 40 0f b6 f6 [2.840671] RSP: 0018:c90dfab8 EFLAGS: 00010202 [2.840671] RAX: 88ec7f1c8000 RBX: 006c7c867000 RCX: [2.840671] RDX: fff0 RSI: RDI: 88ec7f1c8000 [2.840671] RBP: 88ec6f7000b0 R08: 88ec7f19d000 R09: 88ec7cbfcd00 [2.840671] R10: 0095 R11: c90df928 R12: [2.840671] R13: 88ec7f1c8000 R14: 1000 R15: [2.840671] FS: () GS:88ec7f60() knlGS: [2.840671] CS: 0010 DS: ES: CR0: 80050033 [2.840671] CR2: 7ff3e1713000 CR3: 006c7de0a004 CR4: 001606b0 [2.840671] Call Trace: [2.840671] __intel_map_single+0x62/0x140 [2.840671] intel_alloc_coherent+0xa6/0x130 [2.840671] dma_pool_alloc+0xd8/0x1e0 [2.840671] e_qh_alloc+0x55/0x130 [2.840671] ehci_setup+0x284/0x7b0 [2.840671] ehci_pci_setup+0xa3/0x530 [2.840671] usb_add_hcd+0x2b6/0x800 [2.840671] usb_hcd_pci_probe+0x375/0x460 [2.840671] local_pci_probe+0x41/0x90 [2.840671] pci_device_probe+0x105/0x1b0 [2.840671] driver_probe_device+0x12d/0x460 [2.840671] device_driver_attach+0x50/0x60 [2.840671] __driver_attach+0x61/0x130 [2.840671] ? device_driver_attach+0x60/0x60 [2.840671] bus_for_each_dev+0x77/0xc0 [2.840671] ? klist_add_tail+0x3b/0x70 [2.840671] bus_add_driver+0x14d/0x1e0 [2.840671] ? ehci_hcd_init+0xaa/0xaa [2.840671] ? do_early_param+0x91/0x91 [2.840671] driver_register+0x6b/0xb0 [2.840671] ? ehci_hcd_init+0xaa/0xaa [2.840671] do_one_initcall+0x46/0x1c3 [2.840671] ? do_early_param+0x91/0x91 [2.840671] kernel_init_freeable+0x1af/0x258 [2.840671] ? rest_init+0xaa/0xaa [2.840671] kernel_init+0xa/0xf9 [2.840671] ret_from_fork+0x35/0x40 [2.840671] ---[ end trace e87b0d9a1c8135c4 ]--- [3.010848] ehci-pci :00:1a.0: Using iommu dma mapping [3.012551] ehci-pci :00:1a.0: 32bit DMA uses non-identity mapping [3.018537] ehci-pci :00:1a.0: cache line size of 64 is not supported [3.021188] ehci-pci :00:1a.0: irq 18, io mem 0x93002000 [3.029006] ehci-pci :00:1a.0: USB 2.0 started, EHCI 1.00 [3.030918] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= 4.18 [3.033491] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [3.035900] usb usb1: Product: EHCI Host Controller [3.037423] usb usb1: Manufacturer: Linux 4.18.0-170.el8.kdump2.x86_64 ehci_hcd [3.039691] usb usb1: SerialNumber: :00:1a.0 It looks like the device finishes initializing once it figures out it needs dma mapping instead of the default passthrough. intel_alloc_coherent calls iommu_need_mapping, before it calls __intel_map_single, so I'm not sure why it is tripping over the WARN_ON in domain_get_iommu. one thing I noticed while looking at this is that domain_get_iommu can return NULL. So should there be something like the following in __intel_map_single after the domain_get_iommu call? if (!iommu) goto error; It is possible to deref the null pointer later otherwise. Regards, Jerry I reproduced the warning with a 5.5 kernel on an Intel NUC5i5MYBE. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/intel-iommu: set as DUMMY_DEVICE_DOMAIN_INFO if no IOMMU
Hi, On 2020/2/5 18:06, Jian-Hong Pan wrote: Lu Baolu 於 2020年2月5日 週三 上午9:28寫道: Hi, On 2020/2/4 17:25, Jian-Hong Pan wrote: Lu Baolu 於 2020年2月4日 週二 下午2:11寫道: Hi, On 2020/2/3 17:10, Jian-Hong Pan wrote: If the device has no IOMMU, it still invokes iommu_need_mapping during intel_alloc_coherent. However, iommu_need_mapping can only check the device is DUMMY_DEVICE_DOMAIN_INFO or not. This patch marks the device is a DUMMY_DEVICE_DOMAIN_INFO if the device has no IOMMU. Signed-off-by: Jian-Hong Pan --- drivers/iommu/intel-iommu.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 35a4a3abedc6..878bc986a015 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -5612,8 +5612,10 @@ static int intel_iommu_add_device(struct device *dev) int ret; iommu = device_to_iommu(dev, &bus, &devfn); - if (!iommu) + if (!iommu) { + dev->archdata.iommu = DUMMY_DEVICE_DOMAIN_INFO; Is this a DMA capable device? Do you mean is the device in DMA Remapping table? Dump DMAR from ACPI table. The device is not in the table. So, it does not support DMAR, Intel IOMMU. Or, should device_to_iommu be invoked in iommu_need_mapping to check IOMMU feature again? Normally intel_iommu_add_device() should only be called for PCI devices and APCI name space devices (reported in ACPI/DMAR table). In both cases, device_to_iommu() should always return a corresponding iommu. I just tried to understand why it failed for your case. We found all of the DMAR featured devices's PCI Segment Number is **. But the devices locating under segment/domain *0001* hit the issue, until the patch is applied. Because of different segment numbers, none of iommu will be matched by for_each_active_iommu(iommu, drhd) loop in function device_to_iommu() and it will return NULL. So, intel_iommu_add_device() returns no device. I can share the DMAR: /* * Intel ACPI Component Architecture * AML/ASL+ Disassembler version 20200110 (64-bit version) * Copyright (c) 2000 - 2020 Intel Corporation * * Disassembly of dmar.dat, Wed Jan 22 11:41:50 2020 * * ACPI Data Table [DMAR] * * Format: [HexOffset DecimalOffset ByteLength] FieldName : FieldValue */ [000h 4]Signature : "DMAR"[DMA Remapping table] [004h 0004 4] Table Length : 00A8 [008h 0008 1] Revision : 01 [009h 0009 1] Checksum : 5E [00Ah 0010 6] Oem ID : "INTEL " [010h 0016 8] Oem Table ID : "EDK2" [018h 0024 4] Oem Revision : 0002 [01Ch 0028 4] Asl Compiler ID : "" [020h 0032 4]Asl Compiler Revision : 0113 [024h 0036 1] Host Address Width : 26 [025h 0037 1]Flags : 05 [026h 0038 10] Reserved : 00 00 00 00 00 00 00 00 00 00 [030h 0048 2]Subtable Type : [Hardware Unit Definition] [032h 0050 2] Length : 0018 [034h 0052 1]Flags : 00 [035h 0053 1] Reserved : 00 [036h 0054 2] PCI Segment Number : [038h 0056 8]Register Base Address : FED9 [040h 0064 1]Device Scope Type : 01 [PCI Endpoint Device] [041h 0065 1] Entry Length : 08 [042h 0066 2] Reserved : [044h 0068 1] Enumeration ID : 00 [045h 0069 1] PCI Bus Number : 00 [046h 0070 2] PCI Path : 02,00 [048h 0072 2]Subtable Type : [Hardware Unit Definition] [04Ah 0074 2] Length : 0020 [04Ch 0076 1]Flags : 01 [04Dh 0077 1] Reserved : 00 [04Eh 0078 2] PCI Segment Number : [050h 0080 8]Register Base Address : FED91000 [058h 0088 1]Device Scope Type : 03 [IOAPIC Device] [059h 0089 1] Entry Length : 08 [05Ah 0090 2] Reserved : [05Ch 0092 1] Enumeration ID : 02 [05Dh 0093 1] PCI Bus Number : 00 [05Eh 0094 2] PCI Path : 1E,07 [060h 0096 1]Device Scope Type : 04 [Message-capable HPET Device] [061h 0097 1] Entry Length : 08 [062h 0098 2] Reserved : [064h 0100 1] Enumeration ID : 00 [065h 0101 1] PCI Bus Number : 00 [066h 0102 2] PCI Path : 1E,06 [068h 0104 2]Subtable Type : 0001 [Reserved Memory Region] [06Ah 0106 2] Length : 0020 [06Ch 0108 2] Reserved : [06Eh 0110 2] PCI Segment Number : [070h 0112 8] Base Address : 6F58B000 [078
Re: [PATCH 3/3] iommu/uapi: Add helper function for size lookup
Hi Jacob, On 2/3/20 11:41 PM, Jacob Pan wrote: > On Mon, 3 Feb 2020 14:12:36 -0700 > Alex Williamson wrote: > >> On Mon, 3 Feb 2020 12:41:43 -0800 >> Jacob Pan wrote: >> >>> On Mon, 3 Feb 2020 11:27:08 -0700 >>> Alex Williamson wrote: >>> On Fri, 31 Jan 2020 15:51:25 -0800 Jacob Pan wrote: > Hi Alex, > Sorry I missed this part in the previous reply. Comments below. > > On Wed, 29 Jan 2020 15:19:51 -0700 > Alex Williamson wrote: > >> Also, is the 12-bytes of padding in struct >> iommu_gpasid_bind_data excessive with this new versioning >> scheme? Per rule #2 I'm not sure if we're allowed to >> repurpose those padding bytes, > We can still use the padding bytes as long as there is a new > flag bit to indicate the validity of the new filed within the > padding. I should have made it clear in rule #2 when mentioning > the flags bits. Should define what extension constitutes. > How about this? > " > * 2. Data structures are open to extension but closed to > modification. > *Extension should leverage the padding bytes first where a > new > *flag bit is required to indicate the validity of each new > member. > *The above rule for padding bytes also applies to adding > new union > *members. > *After padding bytes are exhausted, new fields must be > added at the > *end of each data structure with 64bit alignment. Flag bits > can be > *added without size change but existing ones cannot be > altered. * > " > So if we add new field by doing re-purpose of padding bytes, > size lookup result will remain the same. New code would > recognize the new flag, old code stays the same. > > VFIO layer checks for UAPI compatibility and size to copy, > version sanity check and flag usage are done in the IOMMU code. > >> but if we add >> fields to the end of the structure as the scheme suggests, >> we're stuck with not being able to expand the union for new >> fields. > Good point, it does sound contradictory. I hope the rewritten > rule #2 address that. > Adding data after the union should be extremely rare. Do you > see any issues with the example below? > > offsetofend() can still find the right size. > e.g. > V1 > struct iommu_gpasid_bind_data { > __u32 version; > #define IOMMU_PASID_FORMAT_INTEL_VTD 1 > __u32 format; > #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID > valid */ __u64 flags; > __u64 gpgd; > __u64 hpasid; > __u64 gpasid; > __u32 addr_width; > __u8 padding[12]; > /* Vendor specific data */ > union { > struct iommu_gpasid_bind_data_vtd vtd; > }; > }; > > const static int > iommu_uapi_data_size[NR_IOMMU_UAPI_TYPE][IOMMU_UAPI_VERSION] = > { /* IOMMU_UAPI_BIND_GPASID */ {offsetofend(struct > iommu_gpasid_bind_data, vtd)}, ... > }; > > V2, Add new_member at the end (forget padding for now). > struct iommu_gpasid_bind_data { > __u32 version; > #define IOMMU_PASID_FORMAT_INTEL_VTD 1 > __u32 format; > #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID > valid */ #define IOMMU_NEW_MEMBER_VAL (1 << 1) /* new > member added */ __u64 flags; > __u64 gpgd; > __u64 hpasid; > __u64 gpasid; > __u32 addr_width; > __u8 padding[12]; > /* Vendor specific data */ > union { > struct iommu_gpasid_bind_data_vtd vtd; > }; > __u64 new_member; > }; > const static int > iommu_uapi_data_size[NR_IOMMU_UAPI_TYPE][IOMMU_UAPI_VERSION] = > { /* IOMMU_UAPI_BIND_GPASID */ > {offsetofend(struct iommu_gpasid_bind_data, > vtd), offsetofend(struct > iommu_gpasid_bind_data,new_member)}, > > }; > > V3, Add smmu to the union,larger than vtd > > struct iommu_gpasid_bind_data { > __u32 version; > #define IOMMU_PASID_FORMAT_INTEL_VTD 1 > #define IOMMU_PASID_FORMAT_INTEL_SMMU 2 > __u32 format; > #define IOMMU_SVA_GPASID_VAL (1 << 0) /* guest PASID > valid */ #define IOMMU_NEW_MEMBER_VAL (1 << 1) /* new > member added */ #define IOMMU_SVA_SMMU_SUPP (1 << 2) /* > SMMU data supported */ __u64 flags; > __u64 gpgd; > __u64 hpasid; > __u64 gpasid; > __u32 addr_width; > __u8 padding[12]; > /* Vendor specific data */ > union { > struct iommu_gpasid_bind_data_vtd vtd; > struct iommu_gpasid_bind_data_smmu smmu; > }; > __u64 new_member; > }; > const static int > iommu_uapi_data_size[NR_IOMMU_UAPI_TYPE][IOMMU_UAPI_VERSION] = { > /* IOMMU_UAPI_BIND_GPASID */ > {offsetofend(struct iommu_gpasid_bind_data,
Re: [PATCH 1/3] iommu/uapi: Define uapi version and capabilities
Hi Jacob, On 1/29/20 7:02 AM, Jacob Pan wrote: > Define a unified UAPI version to be used for compatibility > checks between user and kernel. > > Signed-off-by: Liu Yi L > Signed-off-by: Jacob Pan > --- > include/uapi/linux/iommu.h | 48 > ++ > 1 file changed, 48 insertions(+) > > diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h > index fcafb6401430..65a26c2519ee 100644 > --- a/include/uapi/linux/iommu.h > +++ b/include/uapi/linux/iommu.h > @@ -8,6 +8,54 @@ > > #include > > +/** > + * Current version of the IOMMU user API. This is intended for query > + * between user and kernel to determine compatible data structures. > + * > + * Having a single UAPI version to govern the user-kernel data structures > + * makes compatibility check straightforward. On the contrary, supporting > + * combinations of multiple versions of the data can be a nightmare. I would rather put the above justification in the commit msg and not here. > + * > + * UAPI version can be bumped up with the following rules: > + * 1. All data structures passed between user and kernel space share > + *the same version number. i.e. any extension to to any structure s/to to/to > + *results in version bump up. in a version number increment? > + * > + * 2. Data structures are open to extension but closed to modification.> + * >New fields must be added at the end of each data structure with > + *64bit alignment. Flag bits can be added without size change but > + *existing ones cannot be altered. > + * > + * 3. Versions are backward compatible. > + * > + * 4. Version to size lookup is supported by kernel internal API for each > + *API function type. @version is mandatory for new data structures > + *and must be at the beginning with type of __u32. > + */ > +#define IOMMU_UAPI_VERSION 1 > +static inline int iommu_get_uapi_version(void) > +{ > + return IOMMU_UAPI_VERSION; > +} > + > +/* > + * Supported UAPI features that can be reported to user space. > + * These types represent the capability available in the kernel. > + * > + * REVISIT: UAPI version also implies the capabilities. Should we > + * report them explicitly? > + */ > +enum IOMMU_UAPI_DATA_TYPES { > + IOMMU_UAPI_BIND_GPASID, > + IOMMU_UAPI_CACHE_INVAL, > + IOMMU_UAPI_PAGE_RESP, > + NR_IOMMU_UAPI_TYPE, > +}; > + > +#define IOMMU_UAPI_CAP_MASK ((1 << IOMMU_UAPI_BIND_GPASID) | \ > + (1 << IOMMU_UAPI_CACHE_INVAL) | \ > + (1 << IOMMU_UAPI_PAGE_RESP)) > + > #define IOMMU_FAULT_PERM_READ(1 << 0) /* read */ > #define IOMMU_FAULT_PERM_WRITE (1 << 1) /* write */ > #define IOMMU_FAULT_PERM_EXEC(1 << 2) /* exec */ > Thanks Eric ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC v3 1/8] vfio: Add VFIO_IOMMU_PASID_REQUEST(alloc/free)
> From: Liu, Yi L > Sent: Friday, January 31, 2020 8:41 PM > To: Alex Williamson > Subject: RE: [RFC v3 1/8] vfio: Add VFIO_IOMMU_PASID_REQUEST(alloc/free) > > Hi Alex, > > > From: Alex Williamson [mailto:alex.william...@redhat.com] > > Sent: Thursday, January 30, 2020 7:56 AM > > To: Liu, Yi L > > Subject: Re: [RFC v3 1/8] vfio: Add > > VFIO_IOMMU_PASID_REQUEST(alloc/free) > > > > On Wed, 29 Jan 2020 04:11:45 -0800 > > "Liu, Yi L" wrote: > > > > > From: Liu Yi L > > > [...] > > > + > > > +int vfio_mm_pasid_alloc(struct vfio_mm *vmm, int min, int max) { > > > + ioasid_t pasid; > > > + int ret = -ENOSPC; > > > + > > > + mutex_lock(&vmm->pasid_lock); > > > + if (vmm->pasid_count >= vmm->pasid_quota) { > > > + ret = -ENOSPC; > > > + goto out_unlock; > > > + } > > > + /* Track ioasid allocation owner by mm */ > > > + pasid = ioasid_alloc((struct ioasid_set *)vmm->mm, min, > > > + max, NULL); > > > > Is mm effectively only a token for this? Maybe we should have a > > struct vfio_mm_token since gets and puts are not creating a reference > > to an mm, but to an "mm token". > > yes, it is supposed to be a kind of token. vfio_mm_token is better naming. :-) Hi Alex, Just to double check if I got your point. Your point is to have a separate structure which is only wrap of mm or just renaming current vfio_mm would be enough? Regards, Yi Liu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu