* 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).