* Re: [OPW kernel] dma_set_coherent_mask [not found] <5192F5C8.3090100@gmail.com> @ 2013-05-15 5:39 ` Sarah Sharp 2013-05-15 14:37 ` Alan Stern 0 siblings, 1 reply; 6+ messages in thread From: Sarah Sharp @ 2013-05-15 5:39 UTC (permalink / raw) To: Xenia Ragiadakou; +Cc: linux-usb, Alan Stern, linux-pci On Wed, May 15, 2013 at 05:41:12AM +0300, Xenia Ragiadakou wrote: > Hi Sarah, (again) > > Also I noticed that dma_set_coherent_mask() is not called somewhere, > which according to DMA-API-HOWTO.txt, means the even if the DMA mask > is set to 64 bits by dma_set_mask, dma_alloc_coherent and > dma_pool_alloc wont return 64 bit addresses (32 MSbits wont be > addressed). Another good question for Alan, and the USB and PCI list. (Alan, Ksenia is one of the applicants for the FOSS Outreach Program for Women that I've been coordinating: http://kernelnewbies.org/OPWIntro) We do allocate memory using DMA pools, and we do want 64-bit context addresses if the xHCI host controller can handle it. The xHCI driver calls dma_set_mask, but not dma_set_coherent_mask(): temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params); if (HCC_64BIT_ADDR(temp)) { xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n"); dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)); } else { dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32)); } Alan, should it be calling dma_set_coherent_mask()? I think I may have noticed the context addresses were never 64-bit addresses, but I didn't think to look whether the host supported 64-bit addresses. I just assumed it could only handle 32-bit addresses. > I hope I'm not doing a huge mistake in my reasonance, and you loose > your time with these emails. Nope, I want you to ask questions, so don't worry about that. Sarah Sharp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OPW kernel] dma_set_coherent_mask 2013-05-15 5:39 ` [OPW kernel] dma_set_coherent_mask Sarah Sharp @ 2013-05-15 14:37 ` Alan Stern 2013-05-15 22:42 ` Sarah Sharp 0 siblings, 1 reply; 6+ messages in thread From: Alan Stern @ 2013-05-15 14:37 UTC (permalink / raw) To: Sarah Sharp; +Cc: Xenia Ragiadakou, linux-usb, linux-pci On Tue, 14 May 2013, Sarah Sharp wrote: > On Wed, May 15, 2013 at 05:41:12AM +0300, Xenia Ragiadakou wrote: > > Hi Sarah, (again) > > > > Also I noticed that dma_set_coherent_mask() is not called somewhere, > > which according to DMA-API-HOWTO.txt, means the even if the DMA mask > > is set to 64 bits by dma_set_mask, dma_alloc_coherent and > > dma_pool_alloc wont return 64 bit addresses (32 MSbits wont be > > addressed). > > Another good question for Alan, and the USB and PCI list. This does sound like a bug. The PCI core initializes all devices by default with 32-bit masks for DMA and coherent DMA. (PCI-E, PCI-X, or other extensions may have more liberal policies.) If a larger mask can work, the driver is responsible for setting it up. > (Alan, Ksenia is one of the applicants for the FOSS Outreach Program for > Women that I've been coordinating: http://kernelnewbies.org/OPWIntro) > > We do allocate memory using DMA pools, and we do want 64-bit context > addresses if the xHCI host controller can handle it. > > The xHCI driver calls dma_set_mask, but not dma_set_coherent_mask(): > > temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params); > if (HCC_64BIT_ADDR(temp)) { > xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n"); > dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)); > } else { > dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32)); > } > > Alan, should it be calling dma_set_coherent_mask()? I think I may have > noticed the context addresses were never 64-bit addresses, but I didn't > think to look whether the host supported 64-bit addresses. I just > assumed it could only handle 32-bit addresses. If you're using 64-bit DMA then you almost certainly do want to call dma_set_coherent_mask(). On the plus side, it is guaranteed that if dma_set_mask() succeeds with a particular mask value then dma_set_coherent_mask() for the same mask value will also succeed. Ala Stern ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OPW kernel] dma_set_coherent_mask 2013-05-15 14:37 ` Alan Stern @ 2013-05-15 22:42 ` Sarah Sharp 2013-05-16 14:11 ` Alan Stern 0 siblings, 1 reply; 6+ messages in thread From: Sarah Sharp @ 2013-05-15 22:42 UTC (permalink / raw) To: Alan Stern; +Cc: Xenia Ragiadakou, linux-usb, linux-pci On Wed, May 15, 2013 at 10:37:00AM -0400, Alan Stern wrote: > On Tue, 14 May 2013, Sarah Sharp wrote: > > We do allocate memory using DMA pools, and we do want 64-bit context > > addresses if the xHCI host controller can handle it. > > > > The xHCI driver calls dma_set_mask, but not dma_set_coherent_mask(): > > > > temp = xhci_readl(xhci, &xhci->cap_regs->hcc_params); > > if (HCC_64BIT_ADDR(temp)) { > > xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n"); > > dma_set_mask(hcd->self.controller, DMA_BIT_MASK(64)); > > } else { > > dma_set_mask(hcd->self.controller, DMA_BIT_MASK(32)); > > } > > > > Alan, should it be calling dma_set_coherent_mask()? I think I may have > > noticed the context addresses were never 64-bit addresses, but I didn't > > think to look whether the host supported 64-bit addresses. I just > > assumed it could only handle 32-bit addresses. > > If you're using 64-bit DMA then you almost certainly do want to call > dma_set_coherent_mask(). On the plus side, it is guaranteed that if > dma_set_mask() succeeds with a particular mask value then > dma_set_coherent_mask() for the same mask value will also succeed. So we need to call both dma_set_coherent_mask() and dma_set_mask()? Or just dma_set_coherent_mask()? Ksenia, thanks for catching this! You should send a patch to fix it once Alan clarifies this. Sarah Sharp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OPW kernel] dma_set_coherent_mask 2013-05-15 22:42 ` Sarah Sharp @ 2013-05-16 14:11 ` Alan Stern 2013-05-16 17:19 ` Sarah Sharp 0 siblings, 1 reply; 6+ messages in thread From: Alan Stern @ 2013-05-16 14:11 UTC (permalink / raw) To: Sarah Sharp; +Cc: Xenia Ragiadakou, linux-usb, linux-pci On Wed, 15 May 2013, Sarah Sharp wrote: > > If you're using 64-bit DMA then you almost certainly do want to call > > dma_set_coherent_mask(). On the plus side, it is guaranteed that if > > dma_set_mask() succeeds with a particular mask value then > > dma_set_coherent_mask() for the same mask value will also succeed. > > So we need to call both dma_set_coherent_mask() and dma_set_mask()? Or > just dma_set_coherent_mask()? It depends on what kind of DMA transfers you're going to do. For streaming transfers (the ones that use dma_map_single() or dma_map_sg(), for example), you need to call dma_set_mask(). For coherent transfers (the ones that use dma_alloc_coherent() or dma_pool_create()), you need to call dma_set_coherent_mask(). If you want to do both kinds of transfers then you need to call both routines. Alan Stern ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OPW kernel] dma_set_coherent_mask 2013-05-16 14:11 ` Alan Stern @ 2013-05-16 17:19 ` Sarah Sharp 2013-05-22 20:43 ` Don Dutile 0 siblings, 1 reply; 6+ messages in thread From: Sarah Sharp @ 2013-05-16 17:19 UTC (permalink / raw) To: Alan Stern; +Cc: Xenia Ragiadakou, linux-usb, linux-pci On Thu, May 16, 2013 at 10:11:00AM -0400, Alan Stern wrote: > On Wed, 15 May 2013, Sarah Sharp wrote: > > > > If you're using 64-bit DMA then you almost certainly do want to call > > > dma_set_coherent_mask(). On the plus side, it is guaranteed that if > > > dma_set_mask() succeeds with a particular mask value then > > > dma_set_coherent_mask() for the same mask value will also succeed. > > > > So we need to call both dma_set_coherent_mask() and dma_set_mask()? Or > > just dma_set_coherent_mask()? > > It depends on what kind of DMA transfers you're going to do. For > streaming transfers (the ones that use dma_map_single() or > dma_map_sg(), for example), you need to call dma_set_mask(). For > coherent transfers (the ones that use dma_alloc_coherent() or > dma_pool_create()), you need to call dma_set_coherent_mask(). > > If you want to do both kinds of transfers then you need to call both > routines. I think we need the host to be able to do DMA to URB buffers that are mapped with dma_map_single() or dma_map_sg(), since that's what usb_hcd_submit_urb() uses. So the driver needs to call dma_set_mask(), which it does. The xHCI endpoint rings are allocated from DMA pools, so we need to call dma_set_coherent_mask() as well. Missing that call explains why I've never seen 64-bit endpoint rings, but I have seen 64-bit URB buffer pointers. Ksenia, do you want to add code to add the call to dma_set_coherent_mask() in those two places in xhci_gen_setup()? As I mentioned, let's add this function call first, and then have a separate commit refactor the copy-paste code into a new function. Again, thanks for catching this and asking questions! Sarah Sharp ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [OPW kernel] dma_set_coherent_mask 2013-05-16 17:19 ` Sarah Sharp @ 2013-05-22 20:43 ` Don Dutile 0 siblings, 0 replies; 6+ messages in thread From: Don Dutile @ 2013-05-22 20:43 UTC (permalink / raw) To: Sarah Sharp; +Cc: Alan Stern, Xenia Ragiadakou, linux-usb, linux-pci On 05/16/2013 01:19 PM, Sarah Sharp wrote: > On Thu, May 16, 2013 at 10:11:00AM -0400, Alan Stern wrote: >> On Wed, 15 May 2013, Sarah Sharp wrote: >> >>>> If you're using 64-bit DMA then you almost certainly do want to call >>>> dma_set_coherent_mask(). On the plus side, it is guaranteed that if >>>> dma_set_mask() succeeds with a particular mask value then >>>> dma_set_coherent_mask() for the same mask value will also succeed. >>> >>> So we need to call both dma_set_coherent_mask() and dma_set_mask()? Or >>> just dma_set_coherent_mask()? >> >> It depends on what kind of DMA transfers you're going to do. For >> streaming transfers (the ones that use dma_map_single() or >> dma_map_sg(), for example), you need to call dma_set_mask(). For >> coherent transfers (the ones that use dma_alloc_coherent() or >> dma_pool_create()), you need to call dma_set_coherent_mask(). >> >> If you want to do both kinds of transfers then you need to call both >> routines. > > I think we need the host to be able to do DMA to URB buffers that are > mapped with dma_map_single() or dma_map_sg(), since that's what > usb_hcd_submit_urb() uses. So the driver needs to call dma_set_mask(), > which it does. > > The xHCI endpoint rings are allocated from DMA pools, so we need to > call dma_set_coherent_mask() as well. Missing that call explains why > I've never seen 64-bit endpoint rings, but I have seen 64-bit URB buffer > pointers. > Correct. Otherwise, that DMA is going through the (more limited 64MB) bounce buffer which is allocated out of low memory since... a device can't do 64-bit addressing; > Ksenia, do you want to add code to add the call to > dma_set_coherent_mask() in those two places in xhci_gen_setup()? As I > mentioned, let's add this function call first, and then have a separate > commit refactor the copy-paste code into a new function. > > Again, thanks for catching this and asking questions! > > Sarah Sharp > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-22 20:44 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5192F5C8.3090100@gmail.com>
2013-05-15 5:39 ` [OPW kernel] dma_set_coherent_mask Sarah Sharp
2013-05-15 14:37 ` Alan Stern
2013-05-15 22:42 ` Sarah Sharp
2013-05-16 14:11 ` Alan Stern
2013-05-16 17:19 ` Sarah Sharp
2013-05-22 20:43 ` Don Dutile
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).