Re: warning from domain_get_iommu
Hi, On 2020/2/8 18:19, Jerry Snitselaar wrote: On Sat Feb 08 20, Lu Baolu wrote: Hi Jerry, On 2020/2/7 17:34, Jerry Snitselaar wrote: On Thu Feb 06 20, Jerry Snitselaar wrote: 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. Hi Baolu, I think I understand what is happening here. With the kdump boot translation is pre-enabled, so in intel_iommu_add_device things are getting set to DEFER_DEVICE_DOMAIN_INFO. When intel_alloc_coherent calls iommu_need_mapping it returns true, but doesn't do the dma domain switch because of DEFER_DEVICE_DOMAIN_INFO. Then __intel_map_single g
Re: [PATCH 1/2] iommu: arm-smmu-impl: Convert to a generic reset implementation
On Wed 22 Jan 03:48 PST 2020, Sai Prakash Ranjan wrote:
> Currently the QCOM specific smmu reset implementation is very
> specific to SDM845 SoC and has a wait-for-safe logic which
> may not be required for other SoCs. So move the SDM845 specific
> logic to its specific reset function. Also add SC7180 SMMU
> compatible for calling into QCOM specific implementation.
>
> Signed-off-by: Sai Prakash Ranjan
Reviewed-by: Bjorn Andersson
Regards,
Bjorn
> ---
> drivers/iommu/arm-smmu-impl.c | 8 +---
> drivers/iommu/arm-smmu-qcom.c | 16 +---
> 2 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-impl.c b/drivers/iommu/arm-smmu-impl.c
> index 74d97a886e93..c75b9d957b70 100644
> --- a/drivers/iommu/arm-smmu-impl.c
> +++ b/drivers/iommu/arm-smmu-impl.c
> @@ -150,6 +150,8 @@ static const struct arm_smmu_impl arm_mmu500_impl = {
>
> struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu)
> {
> + const struct device_node *np = smmu->dev->of_node;
> +
> /*
>* We will inevitably have to combine model-specific implementation
>* quirks with platform-specific integration quirks, but everything
> @@ -166,11 +168,11 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
> arm_smmu_device *smmu)
> break;
> }
>
> - if (of_property_read_bool(smmu->dev->of_node,
> - "calxeda,smmu-secure-config-access"))
> + if (of_property_read_bool(np, "calxeda,smmu-secure-config-access"))
> smmu->impl = &calxeda_impl;
>
> - if (of_device_is_compatible(smmu->dev->of_node, "qcom,sdm845-smmu-500"))
> + if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
> + of_device_is_compatible(np, "qcom,sc7180-smmu-500"))
> return qcom_smmu_impl_init(smmu);
>
> return smmu;
> diff --git a/drivers/iommu/arm-smmu-qcom.c b/drivers/iommu/arm-smmu-qcom.c
> index 24c071c1d8b0..64a4ab270ab7 100644
> --- a/drivers/iommu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm-smmu-qcom.c
> @@ -15,8 +15,6 @@ static int qcom_sdm845_smmu500_reset(struct arm_smmu_device
> *smmu)
> {
> int ret;
>
> - arm_mmu500_reset(smmu);
> -
> /*
>* To address performance degradation in non-real time clients,
>* such as USB and UFS, turn off wait-for-safe on sdm845 based boards,
> @@ -30,8 +28,20 @@ static int qcom_sdm845_smmu500_reset(struct
> arm_smmu_device *smmu)
> return ret;
> }
>
> +static int qcom_smmu500_reset(struct arm_smmu_device *smmu)
> +{
> + const struct device_node *np = smmu->dev->of_node;
> +
> + arm_mmu500_reset(smmu);
> +
> + if (of_device_is_compatible(np, "qcom,sdm845-smmu-500"))
> + return qcom_sdm845_smmu500_reset(smmu);
> +
> + return 0;
> +}
> +
> static const struct arm_smmu_impl qcom_smmu_impl = {
> - .reset = qcom_sdm845_smmu500_reset,
> + .reset = qcom_smmu500_reset,
> };
>
> struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
> --
> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
> of Code Aurora Forum, hosted by The Linux Foundation
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: warning from domain_get_iommu
On Sat Feb 08 20, Lu Baolu wrote: Hi Jerry, On 2020/2/7 17:34, Jerry Snitselaar wrote: On Thu Feb 06 20, Jerry Snitselaar wrote: 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. Hi Baolu, I think I understand what is happening here. With the kdump boot translation is pre-enabled, so in intel_iommu_add_device things are getting set to DEFER_DEVICE_DOMAIN_INFO. When intel_alloc_coherent calls iommu_need_mapping it returns true, but doesn't do the dma domain switch because of DEFER_DEVICE_DOMAIN_INFO. Then __intel_map_single gets called and it calls deferred_attach_domain, w
RE: [RFC v3 2/8] vfio/type1: Make per-application (VM) PASID quota tunable
Hi Jacob, > From: Jacob Pan [mailto:jacob.jun@linux.intel.com] > Sent: Saturday, February 8, 2020 3:44 AM > To: Liu, Yi L > Subject: Re: [RFC v3 2/8] vfio/type1: Make per-application (VM) PASID quota > tunable > > On Wed, 29 Jan 2020 04:11:46 -0800 > "Liu, Yi L" wrote: > > > From: Liu Yi L > > > > The PASID quota is per-application (VM) according to vfio's PASID > > management rule. For better flexibility, quota shall be user tunable > > . This patch provides a VFIO based user interface for which quota can > > be adjusted. However, quota cannot be adjusted downward below the > > number of outstanding PASIDs. > > > > This patch only makes the per-VM PASID quota tunable. While for the > > way to tune the default PASID quota, it may require a new vfio module > > option or other way. This may be another patchset in future. > > > One issue we need to solve is how to share PASIDs at the system > level, e.g. Both VMs and baremetal drivers could use PASIDs. > > This patch is granting quota to a guest w/o knowing the remaining > system capacity. So guest PASID allocation could fail even within its > quota. that's true. > The solution I am thinking is to enforce quota at IOASID common > code, since IOASID APIs already used to manage system-wide allocation. > How about the following changes to IOASID? > 1. introduce quota in ioasid_set (could have a soft limit for better > sharing) > > 2. introduce an API to create a set with quota before allocation, e.g. > ioasid_set_id = ioasid_alloc_set(size, token) > set_id will be used for ioasid_alloc() instead of token. Is the token the mm pointer? I guess you may want to add one more API like ioasid_get_set_id(token), thus that other ioasid user could get set_id with their token. If token is the same give them the same set_id. > > 3. introduce API to adjust set quota ioasid_adjust_set_size(set_id, > size) > > 4. API to check remaining PASIDs ioasid_get_capacity(set_id); //return > system capacity if set_id == 0; > > 5. API to set system capacity, ioasid_set_capacity(nr_pasids), e.g. if > system has 20 bit PASIDs, IOMMU driver needs to call > ioasid_set_capacity(1<<20) during boot. yes, this is definitely necessary. > 6. Optional set level APIs. e.g. ioasid_free_set(set_id), frees all > IOASIDs in the set. If this is provided. I think VFIO may be not necessary to track allocated PASIDs. When VM is down or crashed, VFIO just use this API to reclaim allocated PASIDs. > With these APIs, this patch could query PASID capacity at both system > and set level and adjust quota within range. i.e. > 1. IOMMU vendor driver(or other driver to use PASID w/o IOMMU) sets > system wide capacity during boot. > 2. VFIO Call ioasid_alloc_set() when allocating vfio_mm(), set default > quota > 3. Adjust quota per set with ioasid_adjust_set_size() as the tunable in > this patch. I think this is abstraction of the allocated PASID track logic in a common layer. It would simplify user logic. Regards, Yi Liu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
