Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Wed, Jul 14, 2021 at 6:25 PM Joerg Roedel wrote: > > On Tue, Jul 13, 2021 at 07:57:40PM -0400, Konrad Rzeszutek Wilk wrote: > > The SWIOTLB does have support to do late initialization (xen-pcifront > > does that for example - so if you add devices that can't do 64-bit it > > will allocate something like 4MB). > > That sounds like a way to evaluate. I suggest to allocate the SWIOTLB > memory at boot and when the IOMMUs are initialized we re-evaluate what > we ended up with and free the SWIOTLB memory if its not needed. > > If that turns out to be wrong during runtime (e.g. because a device is > switched to a passthrough default domain at runtime), we allocate a > small aperture for this device like the above mentioned 4MB. I am currently working on this but I found that 4MB is not enough, 16MB is the minimal size to make the device work. How do I know the right SWIOTLB size for each device? > > (A boot option to always keep the aperture around might also be helpful > for some setups) OK, will also implement this in next iteration. Kai-Heng > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Tue, Jul 13, 2021 at 07:57:40PM -0400, Konrad Rzeszutek Wilk wrote: > The SWIOTLB does have support to do late initialization (xen-pcifront > does that for example - so if you add devices that can't do 64-bit it > will allocate something like 4MB). That sounds like a way to evaluate. I suggest to allocate the SWIOTLB memory at boot and when the IOMMUs are initialized we re-evaluate what we ended up with and free the SWIOTLB memory if its not needed. If that turns out to be wrong during runtime (e.g. because a device is switched to a passthrough default domain at runtime), we allocate a small aperture for this device like the above mentioned 4MB. (A boot option to always keep the aperture around might also be helpful for some setups) Regards, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Wed, Jul 14, 2021 at 7:57 AM Konrad Rzeszutek Wilk wrote: > > On Thu, Jul 08, 2021 at 03:43:42PM +0100, Robin Murphy wrote: > > On 2021-07-08 14:57, Kai-Heng Feng wrote: > > > On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: > > > > > > > > On 2021-07-08 10:28, Joerg Roedel wrote: > > > > > On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > > > > > > @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > > > > > > > > > > > > iommu = amd_iommu_rlookup_table[dev_data->devid]; > > > > > > dev_data->iommu_v2 = iommu->is_iommu_v2; > > > > > > + > > > > > > +if (dev_data->iommu_v2) > > > > > > +swiotlb = 1; > > > > > > > > > > This looks like the big hammer, as it will affect all other systems > > > > > where the AMD GPUs are in their own group. > > > > > > > > > > What is needed here is an explicit check whether a non-iommu-v2 device > > > > > is direct-mapped because it shares a group with the GPU, and only > > > > > enable > > > > > swiotlb in this case. > > > > > > > > Right, it's basically about whether any DMA-limited device might at any > > > > time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the > > > > possibility of device hotplug and the user being silly with the sysfs > > > > interface, I don't think we can categorically determine that at boot > > > > time. > > > > > > > > Also note that Intel systems are likely to be similarly affected (in > > > > fact intel-iommu doesn't even have the iommu_default_passthough() check > > > > so it's probably even easier to blow up). > > > > > > swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't > > > disable it. > > > > Oh, right... I did say I found this dance hard to follow. Clearly I > > shouldn't have trusted what I thought I remembered from looking at it > > yesterday :) > > > > Also not helped by the fact that it sets iommu_detected which *does* disable > > SWIOTLB, but only on IA-64. > > > > > I wonder if we can take the same approach in amd-iommu? > > > > Certainly if there's a precedent for leaving SWIOTLB enabled even if it > > *might* be redundant, that seems like the easiest option (it's what we do on > > arm64 too, but then we have system topologies where some devices may not be > > behind IOMMUs even when others are). More fun would be to try to bring it up > > at the first sign of IOMMU_DOMAIN_IDENTITY if it was disabled previously, > > but I don't have the highest hope of that being practical. > > > It is kind of silly to enable SWIOTLB which will just eat 64MB of memory > "just in case". > > The SWIOTLB does have support to do late initialization (xen-pcifront > does that for example - so if you add devices that can't do 64-bit it > will allocate something like 4MB). > > Would that be a better choice going forward - that is allocate this > under those conditions? But how to practically do swiotlb late init on 32-bit capable devices? On the first DMA map requested by the driver? Kai-Heng > > > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Thu, Jul 08, 2021 at 03:43:42PM +0100, Robin Murphy wrote: > On 2021-07-08 14:57, Kai-Heng Feng wrote: > > On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: > > > > > > On 2021-07-08 10:28, Joerg Roedel wrote: > > > > On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > > > > > @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > > > > > > > > > > iommu = amd_iommu_rlookup_table[dev_data->devid]; > > > > > dev_data->iommu_v2 = iommu->is_iommu_v2; > > > > > + > > > > > +if (dev_data->iommu_v2) > > > > > +swiotlb = 1; > > > > > > > > This looks like the big hammer, as it will affect all other systems > > > > where the AMD GPUs are in their own group. > > > > > > > > What is needed here is an explicit check whether a non-iommu-v2 device > > > > is direct-mapped because it shares a group with the GPU, and only enable > > > > swiotlb in this case. > > > > > > Right, it's basically about whether any DMA-limited device might at any > > > time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the > > > possibility of device hotplug and the user being silly with the sysfs > > > interface, I don't think we can categorically determine that at boot time. > > > > > > Also note that Intel systems are likely to be similarly affected (in > > > fact intel-iommu doesn't even have the iommu_default_passthough() check > > > so it's probably even easier to blow up). > > > > swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't > > disable it. > > Oh, right... I did say I found this dance hard to follow. Clearly I > shouldn't have trusted what I thought I remembered from looking at it > yesterday :) > > Also not helped by the fact that it sets iommu_detected which *does* disable > SWIOTLB, but only on IA-64. > > > I wonder if we can take the same approach in amd-iommu? > > Certainly if there's a precedent for leaving SWIOTLB enabled even if it > *might* be redundant, that seems like the easiest option (it's what we do on > arm64 too, but then we have system topologies where some devices may not be > behind IOMMUs even when others are). More fun would be to try to bring it up > at the first sign of IOMMU_DOMAIN_IDENTITY if it was disabled previously, > but I don't have the highest hope of that being practical. It is kind of silly to enable SWIOTLB which will just eat 64MB of memory "just in case". The SWIOTLB does have support to do late initialization (xen-pcifront does that for example - so if you add devices that can't do 64-bit it will allocate something like 4MB). Would that be a better choice going forward - that is allocate this under those conditions? > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On 2021-07-08 14:57, Kai-Heng Feng wrote: On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: On 2021-07-08 10:28, Joerg Roedel wrote: On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) iommu = amd_iommu_rlookup_table[dev_data->devid]; dev_data->iommu_v2 = iommu->is_iommu_v2; + +if (dev_data->iommu_v2) +swiotlb = 1; This looks like the big hammer, as it will affect all other systems where the AMD GPUs are in their own group. What is needed here is an explicit check whether a non-iommu-v2 device is direct-mapped because it shares a group with the GPU, and only enable swiotlb in this case. Right, it's basically about whether any DMA-limited device might at any time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the possibility of device hotplug and the user being silly with the sysfs interface, I don't think we can categorically determine that at boot time. Also note that Intel systems are likely to be similarly affected (in fact intel-iommu doesn't even have the iommu_default_passthough() check so it's probably even easier to blow up). swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't disable it. Oh, right... I did say I found this dance hard to follow. Clearly I shouldn't have trusted what I thought I remembered from looking at it yesterday :) Also not helped by the fact that it sets iommu_detected which *does* disable SWIOTLB, but only on IA-64. I wonder if we can take the same approach in amd-iommu? Certainly if there's a precedent for leaving SWIOTLB enabled even if it *might* be redundant, that seems like the easiest option (it's what we do on arm64 too, but then we have system topologies where some devices may not be behind IOMMUs even when others are). More fun would be to try to bring it up at the first sign of IOMMU_DOMAIN_IDENTITY if it was disabled previously, but I don't have the highest hope of that being practical. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Thu, Jul 8, 2021 at 6:18 PM Robin Murphy wrote: > > On 2021-07-08 10:28, Joerg Roedel wrote: > > On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > >> @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > >> > >> iommu = amd_iommu_rlookup_table[dev_data->devid]; > >> dev_data->iommu_v2 = iommu->is_iommu_v2; > >> + > >> +if (dev_data->iommu_v2) > >> +swiotlb = 1; > > > > This looks like the big hammer, as it will affect all other systems > > where the AMD GPUs are in their own group. > > > > What is needed here is an explicit check whether a non-iommu-v2 device > > is direct-mapped because it shares a group with the GPU, and only enable > > swiotlb in this case. > > Right, it's basically about whether any DMA-limited device might at any > time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the > possibility of device hotplug and the user being silly with the sysfs > interface, I don't think we can categorically determine that at boot time. > > Also note that Intel systems are likely to be similarly affected (in > fact intel-iommu doesn't even have the iommu_default_passthough() check > so it's probably even easier to blow up). swiotlb is enabled by pci_swiotlb_detect_4gb() and intel-iommu doesn't disable it. I wonder if we can take the same approach in amd-iommu? Kai-Heng > > Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On 2021-07-08 10:28, Joerg Roedel wrote: On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) iommu = amd_iommu_rlookup_table[dev_data->devid]; dev_data->iommu_v2 = iommu->is_iommu_v2; + + if (dev_data->iommu_v2) + swiotlb = 1; This looks like the big hammer, as it will affect all other systems where the AMD GPUs are in their own group. What is needed here is an explicit check whether a non-iommu-v2 device is direct-mapped because it shares a group with the GPU, and only enable swiotlb in this case. Right, it's basically about whether any DMA-limited device might at any time end up in an IOMMU_DOMAIN_IDENTITY domain. And given the possibility of device hotplug and the user being silly with the sysfs interface, I don't think we can categorically determine that at boot time. Also note that Intel systems are likely to be similarly affected (in fact intel-iommu doesn't even have the iommu_default_passthough() check so it's probably even easier to blow up). Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu/amd: Enable swiotlb if any device supports iommu v2 and uses identity mapping
On Thu, Jul 08, 2021 at 03:42:32PM +0800, Kai-Heng Feng wrote: > @@ -344,6 +344,9 @@ static int iommu_init_device(struct device *dev) > > iommu = amd_iommu_rlookup_table[dev_data->devid]; > dev_data->iommu_v2 = iommu->is_iommu_v2; > + > + if (dev_data->iommu_v2) > + swiotlb = 1; This looks like the big hammer, as it will affect all other systems where the AMD GPUs are in their own group. What is needed here is an explicit check whether a non-iommu-v2 device is direct-mapped because it shares a group with the GPU, and only enable swiotlb in this case. Thanks, Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
