* fixups for the dma_set_mask beahvior change in 5.1 @ 2019-05-21 12:47 Christoph Hellwig 2019-05-21 12:47 ` [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold Christoph Hellwig 2019-05-21 12:47 ` [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported Christoph Hellwig 0 siblings, 2 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-05-21 12:47 UTC (permalink / raw) To: iommu, Russell King; +Cc: Robin Murphy, linux-kernel Hi all, in 5.1 I fixed up the DMA mapping API to always accept larger than required DMA mask. Except that I forgot about a check in the arm code that is not required any more, and about the case where a architecture only supports a 32-bit dma mask, but could potentially generate large addresses due to offsets for the DMA address. These two patches should fix up these issues (which were only found by code inspection). _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-05-21 12:47 fixups for the dma_set_mask beahvior change in 5.1 Christoph Hellwig @ 2019-05-21 12:47 ` Christoph Hellwig 2019-05-21 13:04 ` Russell King - ARM Linux admin 2019-05-21 12:47 ` [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported Christoph Hellwig 1 sibling, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-05-21 12:47 UTC (permalink / raw) To: iommu, Russell King; +Cc: Robin Murphy, linux-kernel The dma masks in struct device are always 64-bits wide. But for builds using a 32-bit dma_addr_t we need to ensure we don't store an unsupportable value. Before Linux 5.0 this was handled at least by the ARM dma mapping code by never allowing to set a larger dma_mask, but these days we allow the driver to just set the largest supported value and never fall back to a smaller one. Ensure this always works by truncating the value. Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections") Signed-off-by: Christoph Hellwig <hch@lst.de> --- kernel/dma/mapping.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c index f7afdadb6770..1f628e7ac709 100644 --- a/kernel/dma/mapping.c +++ b/kernel/dma/mapping.c @@ -317,6 +317,12 @@ void arch_dma_set_mask(struct device *dev, u64 mask); int dma_set_mask(struct device *dev, u64 mask) { + /* + * Truncate the mask to the actually supported dma_addr_t width to + * avoid generating unsupportable addresses. + */ + mask = (dma_addr_t)mask; + if (!dev->dma_mask || !dma_supported(dev, mask)) return -EIO; @@ -330,6 +336,12 @@ EXPORT_SYMBOL(dma_set_mask); #ifndef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK int dma_set_coherent_mask(struct device *dev, u64 mask) { + /* + * Truncate the mask to the actually supported dma_addr_t width to + * avoid generating unsupportable addresses. + */ + mask = (dma_addr_t)mask; + if (!dma_supported(dev, mask)) return -EIO; -- 2.20.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-05-21 12:47 ` [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold Christoph Hellwig @ 2019-05-21 13:04 ` Russell King - ARM Linux admin 2019-05-21 13:15 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux admin @ 2019-05-21 13:04 UTC (permalink / raw) To: Christoph Hellwig; +Cc: iommu, Robin Murphy, linux-kernel On Tue, May 21, 2019 at 02:47:28PM +0200, Christoph Hellwig wrote: > The dma masks in struct device are always 64-bits wide. But for builds > using a 32-bit dma_addr_t we need to ensure we don't store an > unsupportable value. Before Linux 5.0 this was handled at least by > the ARM dma mapping code by never allowing to set a larger dma_mask, > but these days we allow the driver to just set the largest supported > value and never fall back to a smaller one. Ensure this always works > by truncating the value. So how does the driver negotiation for >32bit addresses work if we don't fail for large masks? I'm thinking about all those PCI drivers that need DAC cycles for >32bit addresses, such as e1000, which negotiate via (eg): /* there is a workaround being applied below that limits * 64-bit DMA addresses to 64-bit hardware. There are some * 32-bit adapters that Tx hang when given 64-bit DMA addresses */ pci_using_dac = 0; if ((hw->bus_type == e1000_bus_type_pcix) && !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { pci_using_dac = 1; } else { err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); if (err) { pr_err("No usable DMA config, aborting\n"); goto err_dma; } } and similar. If we blindly trunate the 64-bit to 32-bit, aren't we going to end up with PCI cards using DAC cycles to a host bridge that do not support DAC cycles? > > Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections") > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > kernel/dma/mapping.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c > index f7afdadb6770..1f628e7ac709 100644 > --- a/kernel/dma/mapping.c > +++ b/kernel/dma/mapping.c > @@ -317,6 +317,12 @@ void arch_dma_set_mask(struct device *dev, u64 mask); > > int dma_set_mask(struct device *dev, u64 mask) > { > + /* > + * Truncate the mask to the actually supported dma_addr_t width to > + * avoid generating unsupportable addresses. > + */ > + mask = (dma_addr_t)mask; > + > if (!dev->dma_mask || !dma_supported(dev, mask)) > return -EIO; > > @@ -330,6 +336,12 @@ EXPORT_SYMBOL(dma_set_mask); > #ifndef CONFIG_ARCH_HAS_DMA_SET_COHERENT_MASK > int dma_set_coherent_mask(struct device *dev, u64 mask) > { > + /* > + * Truncate the mask to the actually supported dma_addr_t width to > + * avoid generating unsupportable addresses. > + */ > + mask = (dma_addr_t)mask; > + > if (!dma_supported(dev, mask)) > return -EIO; > > -- > 2.20.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-05-21 13:04 ` Russell King - ARM Linux admin @ 2019-05-21 13:15 ` Christoph Hellwig 2019-05-29 12:22 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-05-21 13:15 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-kernel, iommu, Robin Murphy, Christoph Hellwig On Tue, May 21, 2019 at 02:04:37PM +0100, Russell King - ARM Linux admin wrote: > So how does the driver negotiation for >32bit addresses work if we don't > fail for large masks? > > I'm thinking about all those PCI drivers that need DAC cycles for >32bit > addresses, such as e1000, which negotiate via (eg): > > /* there is a workaround being applied below that limits > * 64-bit DMA addresses to 64-bit hardware. There are some > * 32-bit adapters that Tx hang when given 64-bit DMA addresses > */ > pci_using_dac = 0; > if ((hw->bus_type == e1000_bus_type_pcix) && > !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { > pci_using_dac = 1; > } else { > err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > if (err) { > pr_err("No usable DMA config, aborting\n"); > goto err_dma; > } > } > > and similar. If we blindly trunate the 64-bit to 32-bit, aren't we > going to end up with PCI cards using DAC cycles to a host bridge that > do not support DAC cycles? In general PCI devices just use DAC cycles when they need it. I only know of about a handful of devices that need to negotiate their addressing mode, and those already use the proper API for that, which is dma_get_required_mask. The e1000 example is a good case of how the old API confused people. First it only sets the 64-bit mask for devices which can support it, which is good, but then it sets the NETIF_F_HIGHDMA flag only if we set a 64-bit mask, which is completely unrelated to the DMA mask, it just means the driver can handle sk_buff fragments that do not have a kernel mapping, which really is a driver and not a hardware issue. So what this driver really should do is something like: diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c index 551de8c2fef2..d9236083da94 100644 --- a/drivers/net/ethernet/intel/e1000/e1000_main.c +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c @@ -925,7 +925,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) static int cards_found; static int global_quad_port_a; /* global ksp3 port a indication */ - int i, err, pci_using_dac; + int i, err; u16 eeprom_data = 0; u16 tmp = 0; u16 eeprom_apme_mask = E1000_EEPROM_APME; @@ -996,16 +996,11 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) * 64-bit DMA addresses to 64-bit hardware. There are some * 32-bit adapters that Tx hang when given 64-bit DMA addresses */ - pci_using_dac = 0; - if ((hw->bus_type == e1000_bus_type_pcix) && - !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { - pci_using_dac = 1; - } else { - err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); - if (err) { - pr_err("No usable DMA config, aborting\n"); - goto err_dma; - } + err = dma_set_mask_and_coherent(&pdev->dev, + DMA_BIT_MASK(hw->bus_type == e1000_bus_type_pcix ? 64 : 32)); + if (err) { + pr_err("No usable DMA config, aborting\n"); + goto err_dma; } netdev->netdev_ops = &e1000_netdev_ops; @@ -1047,19 +1042,15 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) netdev->priv_flags |= IFF_SUPP_NOFCS; - netdev->features |= netdev->hw_features; + netdev->features |= netdev->hw_features | NETIF_F_HIGHDMA; netdev->hw_features |= (NETIF_F_RXCSUM | NETIF_F_RXALL | NETIF_F_RXFCS); - if (pci_using_dac) { - netdev->features |= NETIF_F_HIGHDMA; - netdev->vlan_features |= NETIF_F_HIGHDMA; - } - netdev->vlan_features |= (NETIF_F_TSO | NETIF_F_HW_CSUM | - NETIF_F_SG); + NETIF_F_SG | + NETIF_F_HIGHDMA); /* Do not set IFF_UNICAST_FLT for VMWare's 82545EM */ if (hw->device_id != E1000_DEV_ID_82545EM_COPPER || _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-05-21 13:15 ` Christoph Hellwig @ 2019-05-29 12:22 ` Christoph Hellwig 2019-06-14 7:46 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-05-29 12:22 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-kernel, iommu, Robin Murphy, Christoph Hellwig Russell, any additional comments on this series? On Tue, May 21, 2019 at 03:15:03PM +0200, Christoph Hellwig wrote: > On Tue, May 21, 2019 at 02:04:37PM +0100, Russell King - ARM Linux admin wrote: > > So how does the driver negotiation for >32bit addresses work if we don't > > fail for large masks? > > > > I'm thinking about all those PCI drivers that need DAC cycles for >32bit > > addresses, such as e1000, which negotiate via (eg): > > > > /* there is a workaround being applied below that limits > > * 64-bit DMA addresses to 64-bit hardware. There are some > > * 32-bit adapters that Tx hang when given 64-bit DMA addresses > > */ > > pci_using_dac = 0; > > if ((hw->bus_type == e1000_bus_type_pcix) && > > !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { > > pci_using_dac = 1; > > } else { > > err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > if (err) { > > pr_err("No usable DMA config, aborting\n"); > > goto err_dma; > > } > > } > > > > and similar. If we blindly trunate the 64-bit to 32-bit, aren't we > > going to end up with PCI cards using DAC cycles to a host bridge that > > do not support DAC cycles? > > In general PCI devices just use DAC cycles when they need it. I only > know of about a handful of devices that need to negotiate their > addressing mode, and those already use the proper API for that, which > is dma_get_required_mask. > > The e1000 example is a good case of how the old API confused people. > First it only sets the 64-bit mask for devices which can support it, > which is good, but then it sets the NETIF_F_HIGHDMA flag only if we > set a 64-bit mask, which is completely unrelated to the DMA mask, > it just means the driver can handle sk_buff fragments that do not > have a kernel mapping, which really is a driver and not a hardware > issue. > > So what this driver really should do is something like: > > > diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c > index 551de8c2fef2..d9236083da94 100644 > --- a/drivers/net/ethernet/intel/e1000/e1000_main.c > +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c > @@ -925,7 +925,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > static int cards_found; > static int global_quad_port_a; /* global ksp3 port a indication */ > - int i, err, pci_using_dac; > + int i, err; > u16 eeprom_data = 0; > u16 tmp = 0; > u16 eeprom_apme_mask = E1000_EEPROM_APME; > @@ -996,16 +996,11 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > * 64-bit DMA addresses to 64-bit hardware. There are some > * 32-bit adapters that Tx hang when given 64-bit DMA addresses > */ > - pci_using_dac = 0; > - if ((hw->bus_type == e1000_bus_type_pcix) && > - !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { > - pci_using_dac = 1; > - } else { > - err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > - if (err) { > - pr_err("No usable DMA config, aborting\n"); > - goto err_dma; > - } > + err = dma_set_mask_and_coherent(&pdev->dev, > + DMA_BIT_MASK(hw->bus_type == e1000_bus_type_pcix ? 64 : 32)); > + if (err) { > + pr_err("No usable DMA config, aborting\n"); > + goto err_dma; > } > > netdev->netdev_ops = &e1000_netdev_ops; > @@ -1047,19 +1042,15 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > netdev->priv_flags |= IFF_SUPP_NOFCS; > > - netdev->features |= netdev->hw_features; > + netdev->features |= netdev->hw_features | NETIF_F_HIGHDMA; > netdev->hw_features |= (NETIF_F_RXCSUM | > NETIF_F_RXALL | > NETIF_F_RXFCS); > > - if (pci_using_dac) { > - netdev->features |= NETIF_F_HIGHDMA; > - netdev->vlan_features |= NETIF_F_HIGHDMA; > - } > - > netdev->vlan_features |= (NETIF_F_TSO | > NETIF_F_HW_CSUM | > - NETIF_F_SG); > + NETIF_F_SG | > + NETIF_F_HIGHDMA); > > /* Do not set IFF_UNICAST_FLT for VMWare's 82545EM */ > if (hw->device_id != E1000_DEV_ID_82545EM_COPPER || > ---end quoted text--- _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-05-29 12:22 ` Christoph Hellwig @ 2019-06-14 7:46 ` Christoph Hellwig 2019-06-25 5:54 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-06-14 7:46 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-kernel, iommu, Robin Murphy, Christoph Hellwig If I don't hear anything back in the next days I will just merge these patches, please comment. On Wed, May 29, 2019 at 02:22:19PM +0200, Christoph Hellwig wrote: > Russell, > > any additional comments on this series? > > On Tue, May 21, 2019 at 03:15:03PM +0200, Christoph Hellwig wrote: > > On Tue, May 21, 2019 at 02:04:37PM +0100, Russell King - ARM Linux admin wrote: > > > So how does the driver negotiation for >32bit addresses work if we don't > > > fail for large masks? > > > > > > I'm thinking about all those PCI drivers that need DAC cycles for >32bit > > > addresses, such as e1000, which negotiate via (eg): > > > > > > /* there is a workaround being applied below that limits > > > * 64-bit DMA addresses to 64-bit hardware. There are some > > > * 32-bit adapters that Tx hang when given 64-bit DMA addresses > > > */ > > > pci_using_dac = 0; > > > if ((hw->bus_type == e1000_bus_type_pcix) && > > > !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { > > > pci_using_dac = 1; > > > } else { > > > err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > > if (err) { > > > pr_err("No usable DMA config, aborting\n"); > > > goto err_dma; > > > } > > > } > > > > > > and similar. If we blindly trunate the 64-bit to 32-bit, aren't we > > > going to end up with PCI cards using DAC cycles to a host bridge that > > > do not support DAC cycles? > > > > In general PCI devices just use DAC cycles when they need it. I only > > know of about a handful of devices that need to negotiate their > > addressing mode, and those already use the proper API for that, which > > is dma_get_required_mask. > > > > The e1000 example is a good case of how the old API confused people. > > First it only sets the 64-bit mask for devices which can support it, > > which is good, but then it sets the NETIF_F_HIGHDMA flag only if we > > set a 64-bit mask, which is completely unrelated to the DMA mask, > > it just means the driver can handle sk_buff fragments that do not > > have a kernel mapping, which really is a driver and not a hardware > > issue. > > > > So what this driver really should do is something like: > > > > > > diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c > > index 551de8c2fef2..d9236083da94 100644 > > --- a/drivers/net/ethernet/intel/e1000/e1000_main.c > > +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c > > @@ -925,7 +925,7 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > > static int cards_found; > > static int global_quad_port_a; /* global ksp3 port a indication */ > > - int i, err, pci_using_dac; > > + int i, err; > > u16 eeprom_data = 0; > > u16 tmp = 0; > > u16 eeprom_apme_mask = E1000_EEPROM_APME; > > @@ -996,16 +996,11 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > * 64-bit DMA addresses to 64-bit hardware. There are some > > * 32-bit adapters that Tx hang when given 64-bit DMA addresses > > */ > > - pci_using_dac = 0; > > - if ((hw->bus_type == e1000_bus_type_pcix) && > > - !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) { > > - pci_using_dac = 1; > > - } else { > > - err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)); > > - if (err) { > > - pr_err("No usable DMA config, aborting\n"); > > - goto err_dma; > > - } > > + err = dma_set_mask_and_coherent(&pdev->dev, > > + DMA_BIT_MASK(hw->bus_type == e1000_bus_type_pcix ? 64 : 32)); > > + if (err) { > > + pr_err("No usable DMA config, aborting\n"); > > + goto err_dma; > > } > > > > netdev->netdev_ops = &e1000_netdev_ops; > > @@ -1047,19 +1042,15 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent) > > > > netdev->priv_flags |= IFF_SUPP_NOFCS; > > > > - netdev->features |= netdev->hw_features; > > + netdev->features |= netdev->hw_features | NETIF_F_HIGHDMA; > > netdev->hw_features |= (NETIF_F_RXCSUM | > > NETIF_F_RXALL | > > NETIF_F_RXFCS); > > > > - if (pci_using_dac) { > > - netdev->features |= NETIF_F_HIGHDMA; > > - netdev->vlan_features |= NETIF_F_HIGHDMA; > > - } > > - > > netdev->vlan_features |= (NETIF_F_TSO | > > NETIF_F_HW_CSUM | > > - NETIF_F_SG); > > + NETIF_F_SG | > > + NETIF_F_HIGHDMA); > > > > /* Do not set IFF_UNICAST_FLT for VMWare's 82545EM */ > > if (hw->device_id != E1000_DEV_ID_82545EM_COPPER || > > > ---end quoted text--- ---end quoted text--- _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold 2019-06-14 7:46 ` Christoph Hellwig @ 2019-06-25 5:54 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-06-25 5:54 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-kernel, iommu, Robin Murphy, Christoph Hellwig On Fri, Jun 14, 2019 at 09:46:48AM +0200, Christoph Hellwig wrote: > If I don't hear anything back in the next days I will just merge > these patches, please comment. Applied to the dma-mapping for-next tree now. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported 2019-05-21 12:47 fixups for the dma_set_mask beahvior change in 5.1 Christoph Hellwig 2019-05-21 12:47 ` [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold Christoph Hellwig @ 2019-05-21 12:47 ` Christoph Hellwig 2019-05-21 13:00 ` Russell King - ARM Linux admin 1 sibling, 1 reply; 10+ messages in thread From: Christoph Hellwig @ 2019-05-21 12:47 UTC (permalink / raw) To: iommu, Russell King; +Cc: Robin Murphy, linux-kernel Since Linux 5.1 we allow drivers to just set the largest DMA mask they support instead of falling back to smaller ones. When fixing up all the dma ops instances to allow for this behavior the arm direct mapping code was missed. Fix it up by removing the sanity check, as all the actual mapping code handles this case just fine. Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections") Signed-off-by: Christoph Hellwig <hch@lst.de> --- arch/arm/mm/dma-mapping.c | 20 +------------------- 1 file changed, 1 insertion(+), 19 deletions(-) diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c index 0a75058c11f3..bdf0d236aaee 100644 --- a/arch/arm/mm/dma-mapping.c +++ b/arch/arm/mm/dma-mapping.c @@ -219,25 +219,7 @@ EXPORT_SYMBOL(arm_coherent_dma_ops); static int __dma_supported(struct device *dev, u64 mask, bool warn) { - unsigned long max_dma_pfn; - - /* - * If the mask allows for more memory than we can address, - * and we actually have that much memory, then we must - * indicate that DMA to this device is not supported. - */ - if (sizeof(mask) != sizeof(dma_addr_t) && - mask > (dma_addr_t)~0 && - dma_to_pfn(dev, ~0) < max_pfn - 1) { - if (warn) { - dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n", - mask); - dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n"); - } - return 0; - } - - max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); + unsigned long max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); /* * Translate the device's DMA mask to a PFN limit. This -- 2.20.1 _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported 2019-05-21 12:47 ` [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported Christoph Hellwig @ 2019-05-21 13:00 ` Russell King - ARM Linux admin 2019-05-21 13:04 ` Christoph Hellwig 0 siblings, 1 reply; 10+ messages in thread From: Russell King - ARM Linux admin @ 2019-05-21 13:00 UTC (permalink / raw) To: Christoph Hellwig; +Cc: iommu, Robin Murphy, linux-kernel On Tue, May 21, 2019 at 02:47:29PM +0200, Christoph Hellwig wrote: > Since Linux 5.1 we allow drivers to just set the largest DMA mask they > support instead of falling back to smaller ones. This doesn't make sense. "they" is confusing - why would a driver set a DMA mask larger than the driver supports? Or is "they" not referring to the drivers (in which case, what is it referring to?) > When fixing up all the dma ops instances to allow for this behavior > the arm direct mapping code was missed. Fix it up by removing the > sanity check, as all the actual mapping code handles this case just > fine. > > Fixes: 9eb9e96e97b3 ("Documentation/DMA-API-HOWTO: update dma_mask sections") > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > arch/arm/mm/dma-mapping.c | 20 +------------------- > 1 file changed, 1 insertion(+), 19 deletions(-) > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c > index 0a75058c11f3..bdf0d236aaee 100644 > --- a/arch/arm/mm/dma-mapping.c > +++ b/arch/arm/mm/dma-mapping.c > @@ -219,25 +219,7 @@ EXPORT_SYMBOL(arm_coherent_dma_ops); > > static int __dma_supported(struct device *dev, u64 mask, bool warn) > { > - unsigned long max_dma_pfn; > - > - /* > - * If the mask allows for more memory than we can address, > - * and we actually have that much memory, then we must > - * indicate that DMA to this device is not supported. > - */ > - if (sizeof(mask) != sizeof(dma_addr_t) && > - mask > (dma_addr_t)~0 && > - dma_to_pfn(dev, ~0) < max_pfn - 1) { > - if (warn) { > - dev_warn(dev, "Coherent DMA mask %#llx is larger than dma_addr_t allows\n", > - mask); > - dev_warn(dev, "Driver did not use or check the return value from dma_set_coherent_mask()?\n"); > - } > - return 0; > - } The point of this check is to trap the case where we have, for example, 8GB of memory, but dma_addr_t is 32-bit. We can allocate in the high 4GB, but we can't represent the address in a dma_addr_t. > - > - max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); > + unsigned long max_dma_pfn = min(max_pfn, arm_dma_pfn_limit); > > /* > * Translate the device's DMA mask to a PFN limit. This > -- > 2.20.1 > > -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported 2019-05-21 13:00 ` Russell King - ARM Linux admin @ 2019-05-21 13:04 ` Christoph Hellwig 0 siblings, 0 replies; 10+ messages in thread From: Christoph Hellwig @ 2019-05-21 13:04 UTC (permalink / raw) To: Russell King - ARM Linux admin Cc: linux-kernel, iommu, Robin Murphy, Christoph Hellwig On Tue, May 21, 2019 at 02:00:47PM +0100, Russell King - ARM Linux admin wrote: > On Tue, May 21, 2019 at 02:47:29PM +0200, Christoph Hellwig wrote: > > Since Linux 5.1 we allow drivers to just set the largest DMA mask they > > support instead of falling back to smaller ones. > > This doesn't make sense. "they" is confusing - why would a driver set > a DMA mask larger than the driver supports? Or is "they" not > referring to the drivers (in which case, what is it referring to?) The current plan is: - the driver sets whatever the device supports, and unless that mask is too small to be supportable it will always succeed Which replaces the previous scheme of: - the driver tries to set whatever the device supports. If there are addressing limitation outside the device (e.g. the PCIe root port, or the AXI interconnect) that will fail, and the device will set a 32-bit mask instead which it assumes will generally work. > The point of this check is to trap the case where we have, for example, > 8GB of memory, but dma_addr_t is 32-bit. We can allocate in the high > 4GB, but we can't represent the address in a dma_addr_t. Yep, and that is what patch 1/2 should handle by truncating the dma mask to something that can work. I don't actually have hardware I could test this scenario on, though. _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-06-25 5:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-21 12:47 fixups for the dma_set_mask beahvior change in 5.1 Christoph Hellwig 2019-05-21 12:47 ` [PATCH 1/2] dma-mapping: truncate dma masks to what dma_addr_t can hold Christoph Hellwig 2019-05-21 13:04 ` Russell King - ARM Linux admin 2019-05-21 13:15 ` Christoph Hellwig 2019-05-29 12:22 ` Christoph Hellwig 2019-06-14 7:46 ` Christoph Hellwig 2019-06-25 5:54 ` Christoph Hellwig 2019-05-21 12:47 ` [PATCH 2/2] ARM: dma-mapping: allow larger DMA mask than supported Christoph Hellwig 2019-05-21 13:00 ` Russell King - ARM Linux admin 2019-05-21 13:04 ` Christoph Hellwig
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).