From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933964AbdACTB6 (ORCPT ); Tue, 3 Jan 2017 14:01:58 -0500 Received: from mail-lf0-f47.google.com ([209.85.215.47]:33677 "EHLO mail-lf0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S965706AbdACTBu (ORCPT ); Tue, 3 Jan 2017 14:01:50 -0500 From: Nikita Yushchenko Subject: Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask To: Will Deacon References: <1483044304-2085-1-git-send-email-nikita.yoush@cogentembedded.com> <20170103184444.GP6986@arm.com> Cc: Catalin Marinas , Arnd Bergmann , linux-arm-kernel@lists.infradead.org, Simon Horman , Bjorn Helgaas , linux-pci@vger.kernel.org, linux-renesas-soc@vger.kernel.org, artemi.ivanov@cogentembedded.com, linux-kernel@vger.kernel.org Message-ID: Date: Tue, 3 Jan 2017 22:01:46 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1 MIME-Version: 1.0 In-Reply-To: <20170103184444.GP6986@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >> It is possible that PCI device supports 64-bit DMA addressing, and thus >> it's driver sets device's dma_mask to DMA_BIT_MASK(64), however PCI host >> bridge has limitations on inbound transactions addressing. Example of >> such setup is NVME SSD device connected to RCAR PCIe controller. >> >> Previously there was attempt to handle this via bus notifier: after >> driver is attached to PCI device, bridge driver gets notifier callback, >> and resets dma_mask from there. However, this is racy: PCI device driver >> could already allocate buffers and/or start i/o in probe routine. >> In NVME case, i/o is started in workqueue context, and this race gives >> "sometimes works, sometimes not" effect. >> >> Proper solution should make driver's dma_set_mask() call to fail if host >> bridge can't support mask being set. >> >> This patch makes __swiotlb_dma_supported() to check mask being set for >> PCI device against dma_mask of struct device corresponding to PCI host >> bridge (one with name "pciXXXX:YY"), if that dma_mask is set. >> >> This is the least destructive approach: currently dma_mask of that device >> object is not used anyhow, thus all existing setups will work as before, >> and modification is required only in actually affected components - >> driver of particular PCI host bridge, and dma_map_ops of particular >> platform. >> >> Signed-off-by: Nikita Yushchenko >> --- >> arch/arm64/mm/dma-mapping.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 290a84f..49645277 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -28,6 +28,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -347,6 +348,16 @@ static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt, >> >> static int __swiotlb_dma_supported(struct device *hwdev, u64 mask) >> { >> +#ifdef CONFIG_PCI >> + if (dev_is_pci(hwdev)) { >> + struct pci_dev *pdev = to_pci_dev(hwdev); >> + struct pci_host_bridge *br = pci_find_host_bridge(pdev->bus); >> + >> + if (br->dev.dma_mask && (*br->dev.dma_mask) && >> + (mask & (*br->dev.dma_mask)) != mask) >> + return 0; >> + } >> +#endif > > Hmm, but this makes it look like the problem is both arm64 and swiotlb > specific, when in reality it's not. Perhaps another hack you could try > would be to register a PCI bus notifier in the host bridge looking for > BUS_NOTIFY_BIND_DRIVER, then you could proxy the DMA ops for each child > device before the driver has probed, but adding a dma_set_mask callback > to limit the mask to what you need? This is what Renesas BSP tries to do and it does not work. BUS_NOTIFY_BIND_DRIVER arrives after driver's probe routine exits, but i/o can be started before that. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Return-Path: From: Nikita Yushchenko Subject: Re: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask To: Will Deacon References: <1483044304-2085-1-git-send-email-nikita.yoush@cogentembedded.com> <20170103184444.GP6986@arm.com> Message-ID: Date: Tue, 3 Jan 2017 22:01:46 +0300 MIME-Version: 1.0 In-Reply-To: <20170103184444.GP6986@arm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Arnd Bergmann , Catalin Marinas , linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org, Simon Horman , linux-pci@vger.kernel.org, Bjorn Helgaas , artemi.ivanov@cogentembedded.com, linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+bjorn=helgaas.com@lists.infradead.org List-ID: >> It is possible that PCI device supports 64-bit DMA addressing, and thus >> it's driver sets device's dma_mask to DMA_BIT_MASK(64), however PCI host >> bridge has limitations on inbound transactions addressing. Example of >> such setup is NVME SSD device connected to RCAR PCIe controller. >> >> Previously there was attempt to handle this via bus notifier: after >> driver is attached to PCI device, bridge driver gets notifier callback, >> and resets dma_mask from there. However, this is racy: PCI device driver >> could already allocate buffers and/or start i/o in probe routine. >> In NVME case, i/o is started in workqueue context, and this race gives >> "sometimes works, sometimes not" effect. >> >> Proper solution should make driver's dma_set_mask() call to fail if host >> bridge can't support mask being set. >> >> This patch makes __swiotlb_dma_supported() to check mask being set for >> PCI device against dma_mask of struct device corresponding to PCI host >> bridge (one with name "pciXXXX:YY"), if that dma_mask is set. >> >> This is the least destructive approach: currently dma_mask of that device >> object is not used anyhow, thus all existing setups will work as before, >> and modification is required only in actually affected components - >> driver of particular PCI host bridge, and dma_map_ops of particular >> platform. >> >> Signed-off-by: Nikita Yushchenko >> --- >> arch/arm64/mm/dma-mapping.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 290a84f..49645277 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -28,6 +28,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -347,6 +348,16 @@ static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt, >> >> static int __swiotlb_dma_supported(struct device *hwdev, u64 mask) >> { >> +#ifdef CONFIG_PCI >> + if (dev_is_pci(hwdev)) { >> + struct pci_dev *pdev = to_pci_dev(hwdev); >> + struct pci_host_bridge *br = pci_find_host_bridge(pdev->bus); >> + >> + if (br->dev.dma_mask && (*br->dev.dma_mask) && >> + (mask & (*br->dev.dma_mask)) != mask) >> + return 0; >> + } >> +#endif > > Hmm, but this makes it look like the problem is both arm64 and swiotlb > specific, when in reality it's not. Perhaps another hack you could try > would be to register a PCI bus notifier in the host bridge looking for > BUS_NOTIFY_BIND_DRIVER, then you could proxy the DMA ops for each child > device before the driver has probed, but adding a dma_set_mask callback > to limit the mask to what you need? This is what Renesas BSP tries to do and it does not work. BUS_NOTIFY_BIND_DRIVER arrives after driver's probe routine exits, but i/o can be started before that. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 From: nikita.yoush@cogentembedded.com (Nikita Yushchenko) Date: Tue, 3 Jan 2017 22:01:46 +0300 Subject: [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask In-Reply-To: <20170103184444.GP6986@arm.com> References: <1483044304-2085-1-git-send-email-nikita.yoush@cogentembedded.com> <20170103184444.GP6986@arm.com> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org >> It is possible that PCI device supports 64-bit DMA addressing, and thus >> it's driver sets device's dma_mask to DMA_BIT_MASK(64), however PCI host >> bridge has limitations on inbound transactions addressing. Example of >> such setup is NVME SSD device connected to RCAR PCIe controller. >> >> Previously there was attempt to handle this via bus notifier: after >> driver is attached to PCI device, bridge driver gets notifier callback, >> and resets dma_mask from there. However, this is racy: PCI device driver >> could already allocate buffers and/or start i/o in probe routine. >> In NVME case, i/o is started in workqueue context, and this race gives >> "sometimes works, sometimes not" effect. >> >> Proper solution should make driver's dma_set_mask() call to fail if host >> bridge can't support mask being set. >> >> This patch makes __swiotlb_dma_supported() to check mask being set for >> PCI device against dma_mask of struct device corresponding to PCI host >> bridge (one with name "pciXXXX:YY"), if that dma_mask is set. >> >> This is the least destructive approach: currently dma_mask of that device >> object is not used anyhow, thus all existing setups will work as before, >> and modification is required only in actually affected components - >> driver of particular PCI host bridge, and dma_map_ops of particular >> platform. >> >> Signed-off-by: Nikita Yushchenko >> --- >> arch/arm64/mm/dma-mapping.c | 11 +++++++++++ >> 1 file changed, 11 insertions(+) >> >> diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c >> index 290a84f..49645277 100644 >> --- a/arch/arm64/mm/dma-mapping.c >> +++ b/arch/arm64/mm/dma-mapping.c >> @@ -28,6 +28,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> >> @@ -347,6 +348,16 @@ static int __swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt, >> >> static int __swiotlb_dma_supported(struct device *hwdev, u64 mask) >> { >> +#ifdef CONFIG_PCI >> + if (dev_is_pci(hwdev)) { >> + struct pci_dev *pdev = to_pci_dev(hwdev); >> + struct pci_host_bridge *br = pci_find_host_bridge(pdev->bus); >> + >> + if (br->dev.dma_mask && (*br->dev.dma_mask) && >> + (mask & (*br->dev.dma_mask)) != mask) >> + return 0; >> + } >> +#endif > > Hmm, but this makes it look like the problem is both arm64 and swiotlb > specific, when in reality it's not. Perhaps another hack you could try > would be to register a PCI bus notifier in the host bridge looking for > BUS_NOTIFY_BIND_DRIVER, then you could proxy the DMA ops for each child > device before the driver has probed, but adding a dma_set_mask callback > to limit the mask to what you need? This is what Renesas BSP tries to do and it does not work. BUS_NOTIFY_BIND_DRIVER arrives after driver's probe routine exits, but i/o can be started before that.