Re: [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug
Hi Janusz, On 8/27/19 5:35 PM, Janusz Krzysztofik wrote: Hi Lu, On Monday, August 26, 2019 10:29:12 AM CEST Lu Baolu wrote: Hi Janusz, On 8/26/19 4:15 PM, Janusz Krzysztofik wrote: Hi Lu, On Friday, August 23, 2019 3:51:11 AM CEST Lu Baolu wrote: Hi, On 8/22/19 10:29 PM, Janusz Krzysztofik wrote: When a perfectly working i915 device is hot unplugged (via sysfs) and hot re-plugged again, its dev->archdata.iommu field is not populated again with an IOMMU pointer. As a result, the device probe fails on DMA mapping error during scratch page setup. It looks like that happens because devices are not detached from their MMUIO bus before they are removed on device unplug. Then, when an already registered device/IOMMU association is identified by the reinstantiated device's bus and function IDs on IOMMU bus re-attach attempt, the device's archdata is not populated with IOMMU information and the bad happens. I'm not sure if this is a proper fix but it works for me so at least it confirms correctness of my analysis results, I believe. So far I haven't been able to identify a good place where the possibly missing IOMMU bus detach on device unplug operation could be added. Which kernel version are you testing with? Does it contain below commit? commit 458b7c8e0dde12d140e3472b80919cbb9ae793f4 Author: Lu Baolu Date: Thu Aug 1 11:14:58 2019 +0800 I was using an internal branch based on drm-tip which didn't contain this commit yet. Fortunately it has been already merged into drm-tip over last weekend and has effectively fixed the issue. Thanks for testing this. My testing appeared not sufficiently exhaustive. The fix indeed resolved my initially discovered issue of not being able to rebind the i915 driver to a re-plugged device, however it brought another, probably more serious problem to light. When an open i915 device is hot unplugged, IOMMU bus notifier now cleans up IOMMU info for the device on PCI device remove while the i915 driver is still not released, kept by open file descriptors. Then, on last device close, cleanup attempts lead to kernel panic raised from intel_unmap() on unresolved IOMMU domain. We should avoid kernel panic when a intel_unmap() is called against a non-existent domain. But we shouldn't expect the IOMMU driver not cleaning up the domain info when a device remove notification comes and wait until all file descriptors being closed, right? Best regards, Baolu With commit 458b7c8e0dde reverted and my fix applied, both late device close and device re-plug work for me. However, I can realize that's probably still not a complete solution, possibly missing some protection against reuse of a removed device other than for cleanup. If you think that's the right way to go, I can work more on that. I've had a look at other drivers and found AMD is using somehow similar approach. On the other hand, looking at the IOMMU common code I couldn't identify any arrangement that would support deferred device cleanup. If that approach is not acceptable for Intel IOMMU, please suggest a way you'd like to have it resolved and I can try to implement it. Thanks, Janusz Best regards, Lu Baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] vmd: Stop overriding dma_map_ops
On Tue, Aug 27, 2019 at 11:24:05AM -0700, Derrick, Jonathan wrote: > On Mon, 2019-08-26 at 17:06 +0200, Christoph Hellwig wrote: > > With a little tweak to the intel-iommu code we should be able to work > > around the VMD mess for the requester IDs without having to create giant > > amounts of boilerplate DMA ops wrapping code. The other advantage of > > this scheme is that we can respect the real DMA masks for the actual > > devices, and I bet it will only be a matter of time until we'll see the > > first DMA challeneged NVMe devices. > > > > The only downside is that we can't offer vmd as a module given that > > intel-iommu calls into it. But the driver only has about 700 lines > > of code, so this should not be a major issue. > If we're going to remove its ability to be a module, and given its > small size, could we make this default =y? > > Otherwise we risk breaking platforms which have it enabled with OSVs > who miss enabling it Can we keep this as a module if we stick the remapping struct device in pci_sysdata instead of going through the vmd driver to get it? Otherwise, very happy to see this dma wrapping go away.
Re: [PATCH] vmd: Stop overriding dma_map_ops
On Mon, 2019-08-26 at 17:06 +0200, Christoph Hellwig wrote: > With a little tweak to the intel-iommu code we should be able to work > around the VMD mess for the requester IDs without having to create giant > amounts of boilerplate DMA ops wrapping code. The other advantage of > this scheme is that we can respect the real DMA masks for the actual > devices, and I bet it will only be a matter of time until we'll see the > first DMA challeneged NVMe devices. > > The only downside is that we can't offer vmd as a module given that > intel-iommu calls into it. But the driver only has about 700 lines > of code, so this should not be a major issue. If we're going to remove its ability to be a module, and given its small size, could we make this default =y? Otherwise we risk breaking platforms which have it enabled with OSVs who miss enabling it > > This also removes the leftover bits of the X86_DEV_DMA_OPS dma_map_ops > registry. > > Signed-off-by: Christoph Hellwig > lgtm otherwise Reviewed-by: Jon Derrick ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/7] dt-bindings: arm-smmu: Add Adreno GPU variant
On Tue, 20 Aug 2019 13:06:27 -0600, Jordan Crouse wrote: > Add a compatible string to identify SMMUs that are attached > to Adreno GPU devices that wish to support split pagetables. > > Signed-off-by: Jordan Crouse > --- > > Documentation/devicetree/bindings/iommu/arm,smmu.txt | 7 +++ > 1 file changed, 7 insertions(+) > Reviewed-by: Rob Herring ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] ARM: OMAP2+: Add pdata for OMAP3 ISP IOMMU
Hi Suman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm/for-next]
[cannot apply to v5.3-rc6 next-20190827]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Suman-Anna/OMAP-IOMMU-fixes-to-go-with-5-4-OMAP-IOMMU-changes/20190827-121217
base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
arch/arm/mach-omap2/pdata-quirks.c:92:3: error: 'struct iommu_platform_data'
has no member named 'device_enable'
.device_enable = omap_device_enable,
^
arch/arm/mach-omap2/pdata-quirks.c:92:19: warning: excess elements in struct
initializer
.device_enable = omap_device_enable,
^~
arch/arm/mach-omap2/pdata-quirks.c:92:19: note: (near initialization for
'omap3_iommu_pdata')
arch/arm/mach-omap2/pdata-quirks.c:93:3: error: 'struct iommu_platform_data'
has no member named 'device_idle'
.device_idle = omap_device_idle,
^~~
arch/arm/mach-omap2/pdata-quirks.c:93:17: warning: excess elements in struct
initializer
.device_idle = omap_device_idle,
^~~~
arch/arm/mach-omap2/pdata-quirks.c:93:17: note: (near initialization for
'omap3_iommu_pdata')
arch/arm/mach-omap2/pdata-quirks.c:97:3: error: 'struct iommu_platform_data'
has no member named 'device_enable'
.device_enable = omap_device_enable,
^
>> arch/arm/mach-omap2/pdata-quirks.c:97:19: error: initialization from
>> incompatible pointer type [-Werror=incompatible-pointer-types]
.device_enable = omap_device_enable,
^~
arch/arm/mach-omap2/pdata-quirks.c:97:19: note: (near initialization for
'omap3_iommu_isp_pdata.reset_name')
arch/arm/mach-omap2/pdata-quirks.c:98:3: error: 'struct iommu_platform_data'
has no member named 'device_idle'
.device_idle = omap_device_idle,
^~~
arch/arm/mach-omap2/pdata-quirks.c:98:17: error: initialization from
incompatible pointer type [-Werror=incompatible-pointer-types]
.device_idle = omap_device_idle,
^~~~
arch/arm/mach-omap2/pdata-quirks.c:98:17: note: (near initialization for
'omap3_iommu_isp_pdata.assert_reset')
arch/arm/mach-omap2/pdata-quirks.c:434:3: error: 'struct
iommu_platform_data' has no member named 'device_enable'
.device_enable = omap_device_enable,
^
arch/arm/mach-omap2/pdata-quirks.c:434:19: warning: excess elements in
struct initializer
.device_enable = omap_device_enable,
^~
arch/arm/mach-omap2/pdata-quirks.c:434:19: note: (near initialization for
'omap4_iommu_pdata')
arch/arm/mach-omap2/pdata-quirks.c:435:3: error: 'struct
iommu_platform_data' has no member named 'device_idle'
.device_idle = omap_device_idle,
^~~
arch/arm/mach-omap2/pdata-quirks.c:435:17: warning: excess elements in
struct initializer
.device_idle = omap_device_idle,
^~~~
arch/arm/mach-omap2/pdata-quirks.c:435:17: note: (near initialization for
'omap4_iommu_pdata')
cc1: some warnings being treated as errors
vim +97 arch/arm/mach-omap2/pdata-quirks.c
87
88 static struct iommu_platform_data omap3_iommu_pdata = {
89 .reset_name = "mmu",
90 .assert_reset = omap_device_assert_hardreset,
91 .deassert_reset = omap_device_deassert_hardreset,
92 .device_enable = omap_device_enable,
> 93 .device_idle = omap_device_idle,
94 };
95
96 static struct iommu_platform_data omap3_iommu_isp_pdata = {
> 97 .device_enable = omap_device_enable,
98 .device_idle = omap_device_idle,
99 };
100
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] dma-contiguous: Use fallback alloc_pages for single pages
On Tue, Aug 27, 2019 at 06:03:14PM +0900, Masahiro Yamada wrote: > Yes, this makes my driver working again > when CONFIG_DMA_CMA=y. > > > If I apply the following, my driver gets back working > irrespective of CONFIG_DMA_CMA. That sounds a lot like the device simply isn't 64-bit DMA capable, and previously always got CMA allocations under the limit it actually supported. I suggest that you submit this quirk to the mmc maintainers. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] ARM: OMAP2+: Plug in device_enable/idle ops for IOMMUs
Hi Suman,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on arm/for-next]
[cannot apply to v5.3-rc6 next-20190826]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Suman-Anna/OMAP-IOMMU-fixes-to-go-with-5-4-OMAP-IOMMU-changes/20190827-121217
base: git://git.armlinux.org.uk/~rmk/linux-arm.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.4.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=arm
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
>> arch/arm/mach-omap2/pdata-quirks.c:92:3: error: 'struct iommu_platform_data'
>> has no member named 'device_enable'
.device_enable = omap_device_enable,
^
>> arch/arm/mach-omap2/pdata-quirks.c:92:19: warning: excess elements in struct
>> initializer
.device_enable = omap_device_enable,
^~
arch/arm/mach-omap2/pdata-quirks.c:92:19: note: (near initialization for
'omap3_iommu_pdata')
>> arch/arm/mach-omap2/pdata-quirks.c:93:3: error: 'struct iommu_platform_data'
>> has no member named 'device_idle'
.device_idle = omap_device_idle,
^~~
arch/arm/mach-omap2/pdata-quirks.c:93:17: warning: excess elements in struct
initializer
.device_idle = omap_device_idle,
^~~~
arch/arm/mach-omap2/pdata-quirks.c:93:17: note: (near initialization for
'omap3_iommu_pdata')
arch/arm/mach-omap2/pdata-quirks.c:429:3: error: 'struct
iommu_platform_data' has no member named 'device_enable'
.device_enable = omap_device_enable,
^
arch/arm/mach-omap2/pdata-quirks.c:429:19: warning: excess elements in
struct initializer
.device_enable = omap_device_enable,
^~
arch/arm/mach-omap2/pdata-quirks.c:429:19: note: (near initialization for
'omap4_iommu_pdata')
arch/arm/mach-omap2/pdata-quirks.c:430:3: error: 'struct
iommu_platform_data' has no member named 'device_idle'
.device_idle = omap_device_idle,
^~~
arch/arm/mach-omap2/pdata-quirks.c:430:17: warning: excess elements in
struct initializer
.device_idle = omap_device_idle,
^~~~
arch/arm/mach-omap2/pdata-quirks.c:430:17: note: (near initialization for
'omap4_iommu_pdata')
vim +92 arch/arm/mach-omap2/pdata-quirks.c
87
88 static struct iommu_platform_data omap3_iommu_pdata = {
89 .reset_name = "mmu",
90 .assert_reset = omap_device_assert_hardreset,
91 .deassert_reset = omap_device_deassert_hardreset,
> 92 .device_enable = omap_device_enable,
> 93 .device_idle = omap_device_idle,
94 };
95
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 6/6] MIPS: document mixing "slightly different CCAs"
Hi Christoph, On Mon, Aug 26, 2019 at 03:25:53PM +0200, Christoph Hellwig wrote: > Based on an email from Paul Burton, quoting section 4.8 "Cacheability and > Coherency Attributes and Access Types" of "MIPS Architecture Volume 1: > Introduction to the MIPS32 Architecture" (MD00080, revision 6.01). > > Signed-off-by: Christoph Hellwig Acked-by: Paul Burton Thanks, Paul > --- > arch/mips/Kconfig | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig > index fc88f68ea1ee..aff1cadeea43 100644 > --- a/arch/mips/Kconfig > +++ b/arch/mips/Kconfig > @@ -1119,6 +1119,13 @@ config DMA_PERDEV_COHERENT > > config DMA_NONCOHERENT > bool > + # > + # MIPS allows mixing "slightly different" Cacheability and Coherency > + # Attribute bits. It is believed that the uncached access through > + # KSEG1 and the implementation specific "uncached accelerated" used > + # by pgprot_writcombine can be mixed, and the latter sometimes provides > + # significant advantages. > + # > select ARCH_HAS_DMA_WRITE_COMBINE > select ARCH_HAS_SYNC_DMA_FOR_DEVICE > select ARCH_HAS_UNCACHED_SEGMENT > -- > 2.20.1 >
Re: [PATCH 3/6] dma-mapping: remove arch_dma_mmap_pgprot
Hi Christoph,
On Mon, Aug 26, 2019 at 03:25:50PM +0200, Christoph Hellwig wrote:
> arch_dma_mmap_pgprot is used for two things:
>
> 1) to override the "normal" uncached page attributes for mapping
> memory coherent to devices that can't snoop the CPU caches
> 2) to provide the special DMA_ATTR_WRITE_COMBINE semantics on older
> arm systems and some mips platforms
>
> Replace one with the pgprot_dmacoherent macro that is already provided
> by arm and much simpler to use, and lift the DMA_ATTR_WRITE_COMBINE
> handling to common code with an explicit arch opt-in.
>
> Signed-off-by: Christoph Hellwig
> Acked-by: Geert Uytterhoeven
For the MIPS bits:
Acked-by: Paul Burton
Thanks,
Paul
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm/mm/dma-mapping.c | 6 --
> arch/arm64/Kconfig | 1 -
> arch/arm64/include/asm/pgtable.h | 4
> arch/arm64/mm/dma-mapping.c| 6 --
> arch/m68k/Kconfig | 1 -
> arch/m68k/include/asm/pgtable_mm.h | 3 +++
> arch/m68k/kernel/dma.c | 3 +--
> arch/mips/Kconfig | 2 +-
> arch/mips/mm/dma-noncoherent.c | 8
> include/linux/dma-noncoherent.h| 13 +++--
> kernel/dma/Kconfig | 12 +---
> kernel/dma/mapping.c | 8 +---
> 13 files changed, 35 insertions(+), 34 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 24360211534a..217083caeabd 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -8,7 +8,7 @@ config ARM
> select ARCH_HAS_DEBUG_VIRTUAL if MMU
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_DMA_COHERENT_TO_PFN if SWIOTLB
> - select ARCH_HAS_DMA_MMAP_PGPROT if SWIOTLB
> + select ARCH_HAS_DMA_WRITE_COMBINE if !ARM_DMA_MEM_BUFFERABLE
> select ARCH_HAS_ELF_RANDOMIZE
> select ARCH_HAS_FORTIFY_SOURCE
> select ARCH_HAS_KEEPINITRD
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index d42557ee69c2..d27b12f61737 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -2402,12 +2402,6 @@ long arch_dma_coherent_to_pfn(struct device *dev, void
> *cpu_addr,
> return dma_to_pfn(dev, dma_addr);
> }
>
> -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> - unsigned long attrs)
> -{
> - return __get_dma_pgprot(attrs, prot);
> -}
> -
> void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
> gfp_t gfp, unsigned long attrs)
> {
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 3adcec05b1f6..dab9dda34206 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -13,7 +13,6 @@ config ARM64
> select ARCH_HAS_DEBUG_VIRTUAL
> select ARCH_HAS_DEVMEM_IS_ALLOWED
> select ARCH_HAS_DMA_COHERENT_TO_PFN
> - select ARCH_HAS_DMA_MMAP_PGPROT
> select ARCH_HAS_DMA_PREP_COHERENT
> select ARCH_HAS_ACPI_TABLE_UPGRADE if ACPI
> select ARCH_HAS_ELF_RANDOMIZE
> diff --git a/arch/arm64/include/asm/pgtable.h
> b/arch/arm64/include/asm/pgtable.h
> index e09760ece844..6700371227d1 100644
> --- a/arch/arm64/include/asm/pgtable.h
> +++ b/arch/arm64/include/asm/pgtable.h
> @@ -435,6 +435,10 @@ static inline pmd_t pmd_mkdevmap(pmd_t pmd)
> __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_NORMAL_NC) |
> PTE_PXN | PTE_UXN)
> #define pgprot_device(prot) \
> __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRE)
> | PTE_PXN | PTE_UXN)
> +#define pgprot_dmacoherent(prot) \
> + __pgprot_modify(prot, PTE_ATTRINDX_MASK, \
> + PTE_ATTRINDX(MT_NORMAL_NC) | PTE_PXN | PTE_UXN)
> +
> #define __HAVE_PHYS_MEM_ACCESS_PROT
> struct file;
> extern pgprot_t phys_mem_access_prot(struct file *file, unsigned long pfn,
> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
> index bd2b039f43a6..676efcda51e6 100644
> --- a/arch/arm64/mm/dma-mapping.c
> +++ b/arch/arm64/mm/dma-mapping.c
> @@ -11,12 +11,6 @@
>
> #include
>
> -pgprot_t arch_dma_mmap_pgprot(struct device *dev, pgprot_t prot,
> - unsigned long attrs)
> -{
> - return pgprot_writecombine(prot);
> -}
> -
> void arch_sync_dma_for_device(struct device *dev, phys_addr_t paddr,
> size_t size, enum dma_data_direction dir)
> {
> diff --git a/arch/m68k/Kconfig b/arch/m68k/Kconfig
> index c518d695c376..a9e564306d3e 100644
> --- a/arch/m68k/Kconfig
> +++ b/arch/m68k/Kconfig
> @@ -4,7 +4,6 @@ config M68K
> default y
> select ARCH_32BIT_OFF_T
> select ARCH_HAS_BINFMT_FLAT
> - select ARCH_HAS_DMA_MMAP_PGPROT if MMU && !COLDFIRE
> select ARCH_HAS_DMA_PREP_COHERENT if HAS_DMA && MMU && !COLDFIRE
> select ARCH_HAS_SYNC_DMA_FOR_DEVICE if HAS_DMA
> select ARCH_MIGHT_HAVE_PC_PARPORT if ISA
> diff --git a/arch/m68k/include/asm/pgtable_mm.h
> b/arch/m68k/include/asm/pgtable_mm.h
>
Re: [RFC PATCH] iommu/vt-d: Fix IOMMU field not populated on device hot re-plug
Hi Lu, On Monday, August 26, 2019 10:29:12 AM CEST Lu Baolu wrote: > Hi Janusz, > > On 8/26/19 4:15 PM, Janusz Krzysztofik wrote: > > Hi Lu, > > > > On Friday, August 23, 2019 3:51:11 AM CEST Lu Baolu wrote: > >> Hi, > >> > >> On 8/22/19 10:29 PM, Janusz Krzysztofik wrote: > >>> When a perfectly working i915 device is hot unplugged (via sysfs) and > >>> hot re-plugged again, its dev->archdata.iommu field is not populated > >>> again with an IOMMU pointer. As a result, the device probe fails on > >>> DMA mapping error during scratch page setup. > >>> > >>> It looks like that happens because devices are not detached from their > >>> MMUIO bus before they are removed on device unplug. Then, when an > >>> already registered device/IOMMU association is identified by the > >>> reinstantiated device's bus and function IDs on IOMMU bus re-attach > >>> attempt, the device's archdata is not populated with IOMMU information > >>> and the bad happens. > >>> > >>> I'm not sure if this is a proper fix but it works for me so at least it > >>> confirms correctness of my analysis results, I believe. So far I > >>> haven't been able to identify a good place where the possibly missing > >>> IOMMU bus detach on device unplug operation could be added. > >> > >> Which kernel version are you testing with? Does it contain below commit? > >> > >> commit 458b7c8e0dde12d140e3472b80919cbb9ae793f4 > >> Author: Lu Baolu > >> Date: Thu Aug 1 11:14:58 2019 +0800 > > > > I was using an internal branch based on drm-tip which didn't contain this > > commit yet. Fortunately it has been already merged into drm-tip over last > > weekend and has effectively fixed the issue. > > Thanks for testing this. My testing appeared not sufficiently exhaustive. The fix indeed resolved my initially discovered issue of not being able to rebind the i915 driver to a re-plugged device, however it brought another, probably more serious problem to light. When an open i915 device is hot unplugged, IOMMU bus notifier now cleans up IOMMU info for the device on PCI device remove while the i915 driver is still not released, kept by open file descriptors. Then, on last device close, cleanup attempts lead to kernel panic raised from intel_unmap() on unresolved IOMMU domain. With commit 458b7c8e0dde reverted and my fix applied, both late device close and device re-plug work for me. However, I can realize that's probably still not a complete solution, possibly missing some protection against reuse of a removed device other than for cleanup. If you think that's the right way to go, I can work more on that. I've had a look at other drivers and found AMD is using somehow similar approach. On the other hand, looking at the IOMMU common code I couldn't identify any arrangement that would support deferred device cleanup. If that approach is not acceptable for Intel IOMMU, please suggest a way you'd like to have it resolved and I can try to implement it. Thanks, Janusz > Best regards, > Lu Baolu > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] dma-contiguous: Use fallback alloc_pages for single pages
On Tue, Aug 27, 2019 at 4:50 PM Christoph Hellwig wrote:
>
> On Tue, Aug 27, 2019 at 04:45:20PM +0900, Masahiro Yamada wrote:
> > On Mon, Aug 26, 2019 at 4:33 PM Christoph Hellwig wrote:
> > >
> > > On Mon, Aug 26, 2019 at 11:05:00AM +0900, Masahiro Yamada wrote:
> > > > This is included in v5.3-rc6
> > > > so I tested it.
> > >
> > > So there is no allocation failure, but you get I/O errors later?
> >
> > Right.
> >
> > >
> > > Does the device use a device-private CMA area?
> >
> > Not sure.
> > My driver is drivers/mmc/host/sdhci-cadence.c
> > It reuses routines in drivers/mmc/host/sdhci.c
> >
> >
> >
> > > Does it work with Linux
> > > 5.2 if CONFIG_DMA_CMA is disabled?
> >
> > No.
> > 5.2 + disable CONFIG_DMA_CMA
> > failed in the same way.
>
> So it seems like the device wants CMA memory. I guess the patch
> below will fix it, but that isn't the solution. Can you try it
> to confirm? In the end it probably assumes a dma mask it doesn't
> set that the CMA memory satisfies or something similar.
Thanks for the pointer.
> diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c
> index 69cfb4345388..bd2f24aa7f19 100644
> --- a/kernel/dma/contiguous.c
> +++ b/kernel/dma/contiguous.c
> @@ -236,7 +236,7 @@ struct page *dma_alloc_contiguous(struct device *dev,
> size_t size, gfp_t gfp)
>
> if (dev && dev->cma_area)
> cma = dev->cma_area;
> - else if (count > 1)
> + else
> cma = dma_contiguous_default_area;
>
> /* CMA can be used only in the context which permits sleeping */
Yes, this makes my driver working again
when CONFIG_DMA_CMA=y.
If I apply the following, my driver gets back working
irrespective of CONFIG_DMA_CMA.
diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
index 163d1cf4367e..504459395c39 100644
--- a/drivers/mmc/host/sdhci-cadence.c
+++ b/drivers/mmc/host/sdhci-cadence.c
@@ -237,6 +237,7 @@ static const struct sdhci_ops sdhci_cdns_ops = {
static const struct sdhci_pltfm_data sdhci_cdns_pltfm_data = {
.ops = _cdns_ops,
+ .quirks2 = SDHCI_QUIRK2_BROKEN_64_BIT_DMA,
};
static int sdhci_cdns_set_tune_val(struct sdhci_host *host, unsigned int val)
--
Best Regards
Masahiro Yamada
Re: [PATCH v2 2/2] dma-contiguous: Use fallback alloc_pages for single pages
On Tue, Aug 27, 2019 at 04:45:20PM +0900, Masahiro Yamada wrote: > On Mon, Aug 26, 2019 at 4:33 PM Christoph Hellwig wrote: > > > > On Mon, Aug 26, 2019 at 11:05:00AM +0900, Masahiro Yamada wrote: > > > This is included in v5.3-rc6 > > > so I tested it. > > > > So there is no allocation failure, but you get I/O errors later? > > Right. > > > > > Does the device use a device-private CMA area? > > Not sure. > My driver is drivers/mmc/host/sdhci-cadence.c > It reuses routines in drivers/mmc/host/sdhci.c > > > > > Does it work with Linux > > 5.2 if CONFIG_DMA_CMA is disabled? > > No. > 5.2 + disable CONFIG_DMA_CMA > failed in the same way. So it seems like the device wants CMA memory. I guess the patch below will fix it, but that isn't the solution. Can you try it to confirm? In the end it probably assumes a dma mask it doesn't set that the CMA memory satisfies or something similar. diff --git a/kernel/dma/contiguous.c b/kernel/dma/contiguous.c index 69cfb4345388..bd2f24aa7f19 100644 --- a/kernel/dma/contiguous.c +++ b/kernel/dma/contiguous.c @@ -236,7 +236,7 @@ struct page *dma_alloc_contiguous(struct device *dev, size_t size, gfp_t gfp) if (dev && dev->cma_area) cma = dev->cma_area; - else if (count > 1) + else cma = dma_contiguous_default_area; /* CMA can be used only in the context which permits sleeping */ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 2/2] dma-contiguous: Use fallback alloc_pages for single pages
On Mon, Aug 26, 2019 at 4:33 PM Christoph Hellwig wrote: > > On Mon, Aug 26, 2019 at 11:05:00AM +0900, Masahiro Yamada wrote: > > This is included in v5.3-rc6 > > so I tested it. > > So there is no allocation failure, but you get I/O errors later? Right. > > Does the device use a device-private CMA area? Not sure. My driver is drivers/mmc/host/sdhci-cadence.c It reuses routines in drivers/mmc/host/sdhci.c > Does it work with Linux > 5.2 if CONFIG_DMA_CMA is disabled? No. 5.2 + disable CONFIG_DMA_CMA failed in the same way. -- Best Regards Masahiro Yamada ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 10/11] arm64: edit zone_dma_bits to fine tune dma-direct min mask
On Mon, 26 Aug 2019 13:08:50 +0200
Nicolas Saenz Julienne wrote:
> On Mon, 2019-08-26 at 09:06 +0200, Christoph Hellwig wrote:
> > On Tue, Aug 20, 2019 at 04:58:18PM +0200, Nicolas Saenz Julienne wrote:
> > > - if (IS_ENABLED(CONFIG_ZONE_DMA))
> > > + if (IS_ENABLED(CONFIG_ZONE_DMA)) {
> > > arm64_dma_phys_limit = max_zone_dma_phys();
> > > + zone_dma_bits = ilog2((arm64_dma_phys_limit - 1) &
> > > GENMASK_ULL(31, 0)) + 1;
> >
> Hi Christoph,
> thanks for the rewiews.
>
> > This adds a way too long line.
>
> I know, I couldn't find a way to split the operation without making it even
> harder to read. I'll find a solution.
If all else fails, move the code to an inline function and call it e.g.
phys_limit_to_dma_bits().
Just my two cents,
Petr T
pgpXgyUHAuHLs.pgp
Description: Digitálnà podpis OpenPGP
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/11] xen/arm: simplify dma_cache_maint
And this was still buggy I think, it really needs some real Xen/Arm
testing which I can't do. Hopefully better version below:
--
>From 5ad4b6e291dbb49f65480c9b769414931cbd485a Mon Sep 17 00:00:00 2001
From: Christoph Hellwig
Date: Wed, 24 Jul 2019 15:26:08 +0200
Subject: xen/arm: simplify dma_cache_maint
Calculate the required operation in the caller, and pass it directly
instead of recalculating it for each page, and use simple arithmetics
to get from the physical address to Xen page size aligned chunks.
Signed-off-by: Christoph Hellwig
---
arch/arm/xen/mm.c | 61 ---
1 file changed, 21 insertions(+), 40 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 90574d89d0d4..2fde161733b0 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -35,64 +35,45 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
return __get_free_pages(flags, order);
}
-enum dma_cache_op {
- DMA_UNMAP,
- DMA_MAP,
-};
static bool hypercall_cflush = false;
-/* functions called by SWIOTLB */
-
-static void dma_cache_maint(dma_addr_t handle, unsigned long offset,
- size_t size, enum dma_data_direction dir, enum dma_cache_op op)
+/* buffers in highmem or foreign pages cannot cross page boundaries */
+static void dma_cache_maint(dma_addr_t handle, size_t size, u32 op)
{
struct gnttab_cache_flush cflush;
- unsigned long xen_pfn;
- size_t left = size;
- xen_pfn = (handle >> XEN_PAGE_SHIFT) + offset / XEN_PAGE_SIZE;
- offset %= XEN_PAGE_SIZE;
+ cflush.a.dev_bus_addr = handle & XEN_PAGE_MASK;
+ cflush.offset = xen_offset_in_page(handle);
+ cflush.op = op;
do {
- size_t len = left;
-
- /* buffers in highmem or foreign pages cannot cross page
-* boundaries */
- if (len + offset > XEN_PAGE_SIZE)
- len = XEN_PAGE_SIZE - offset;
-
- cflush.op = 0;
- cflush.a.dev_bus_addr = xen_pfn << XEN_PAGE_SHIFT;
- cflush.offset = offset;
- cflush.length = len;
-
- if (op == DMA_UNMAP && dir != DMA_TO_DEVICE)
- cflush.op = GNTTAB_CACHE_INVAL;
- if (op == DMA_MAP) {
- if (dir == DMA_FROM_DEVICE)
- cflush.op = GNTTAB_CACHE_INVAL;
- else
- cflush.op = GNTTAB_CACHE_CLEAN;
- }
- if (cflush.op)
- HYPERVISOR_grant_table_op(GNTTABOP_cache_flush,
, 1);
+ if (size + cflush.offset > XEN_PAGE_SIZE)
+ cflush.length = XEN_PAGE_SIZE - cflush.offset;
+ else
+ cflush.length = size;
+
+ HYPERVISOR_grant_table_op(GNTTABOP_cache_flush, , 1);
- offset = 0;
- xen_pfn++;
- left -= len;
- } while (left);
+ cflush.offset = 0;
+ cflush.a.dev_bus_addr += cflush.length;
+ size -= cflush.length;
+ } while (size);
}
static void __xen_dma_page_dev_to_cpu(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
- dma_cache_maint(handle & PAGE_MASK, handle & ~PAGE_MASK, size, dir,
DMA_UNMAP);
+ if (dir != DMA_TO_DEVICE)
+ dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
}
static void __xen_dma_page_cpu_to_dev(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
- dma_cache_maint(handle & PAGE_MASK, handle & ~PAGE_MASK, size, dir,
DMA_MAP);
+ if (dir == DMA_FROM_DEVICE)
+ dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
+ else
+ dma_cache_maint(handle, size, GNTTAB_CACHE_CLEAN);
}
void __xen_dma_map_page(struct device *hwdev, struct page *page,
--
2.20.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: swiotlb-xen cleanups
On Mon, Aug 26, 2019 at 07:00:44PM -0700, Stefano Stabellini wrote: > On Fri, 16 Aug 2019, Christoph Hellwig wrote: > > Hi Xen maintainers and friends, > > > > please take a look at this series that cleans up the parts of swiotlb-xen > > that deal with non-coherent caches. > > Hi Christoph, > > I just wanted to let you know that your series is on my radar, but I > have been swamped the last few days. I hope to get to it by the end of > the week. Thanks, and no rush. Note that I posted a v2 with a few significant changes yesterday.
