RE: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
From: Mathias Nyman
> Sent: 19 May 2017 10:49
> To: David Laight; gre...@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; Matthias Lange; sta...@vger.kernel.org
> Subject: Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
>
> On 19.05.2017 12:10, David Laight wrote:
> > From: Mathias Nyman
> >> Sent: 17 May 2017 16:32
> >> There is no reason to restrict allocations to the first 16MB ISA DMA
> >> addresses.
> >>
> >> It is causing problems in a virtualization setup with enabled IOMMU
> >> (x86_64). The result is that USB is not working in the VM.
> > ...
> >> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> >> index 12b573c..1f1687e 100644
> >> --- a/drivers/usb/host/xhci-mem.c
> >> +++ b/drivers/usb/host/xhci-mem.c
> >> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct
> >> xhci_hcd *xhci,
> >>}
> >>
> >>if (max_packet) {
> >> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> >> + seg->bounce_buf = kzalloc(max_packet, flags);
> >
> > This might allocate memory that the device cannot access.
> > So can only work if dma_map_single() itself allocates a bounce buffer.
> > There must be a sane way to do this that doesn't ever require
> > double copies.
> >
>
> We are using dma_map_single()
> This allocated memory is used as the processor virtual memory required by
> dma_map_single()
> i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:
>
>
> dma_map_single(struct device *dev, void *cpu_addr, size_t size,
>enum dma_data_direction direction)
>
> Maps a piece of processor virtual memory so it can be accessed by the
> device and returns the DMA address of the memory.
>
> ...
>
> Notes: Not all memory regions in a machine can be mapped by this API.
> Further, contiguous kernel virtual space may not be contiguous as
> physical memory. Since this API does not provide any scatter/gather
> capability, it will fail if the user tries to map a non-physically
> contiguous piece of memory. For this reason, memory to be mapped by
> this API should be obtained from sources which guarantee it to be
> physically contiguous (like kmalloc).
>
> I'm not fully sure I understand yout concern, are you thinking the driver
> should
> doublecheck the dma address returned by dma_map_single() and make sure it's
> within the
> dma mask set for the device?
The physical memory returned by kzalloc() (without GFP_DMA) can definitely be
outside the dma mask for the device.
If that happens something must allocate a dma bounce buffer, I'm guessing that
dma_map_single() does so - it certainly can since it can copy the data during
the unmap (for a read transfer).
So you really want to allocate a buffer that is within the devices dma window.
The buffer you are allocating isn't really a 'bounce' buffer at all - is it?
It is more of a de_fragmentation buffer.
It is there so that the LINK TRB will always be at a USB segment boundary.
(I'm guessing the bug I found several years ago still isn't fixed??)
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
On 19.05.2017 12:10, David Laight wrote:
From: Mathias Nyman
Sent: 17 May 2017 16:32
There is no reason to restrict allocations to the first 16MB ISA DMA
addresses.
It is causing problems in a virtualization setup with enabled IOMMU
(x86_64). The result is that USB is not working in the VM.
...
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 12b573c..1f1687e 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct
xhci_hcd *xhci,
}
if (max_packet) {
- seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
+ seg->bounce_buf = kzalloc(max_packet, flags);
This might allocate memory that the device cannot access.
So can only work if dma_map_single() itself allocates a bounce buffer.
There must be a sane way to do this that doesn't ever require
double copies.
We are using dma_map_single()
This allocated memory is used as the processor virtual memory required by
dma_map_single()
i.e. the "void *cpu_addr" part of dma_map_single, see DMA-API.txt:
dma_map_single(struct device *dev, void *cpu_addr, size_t size,
enum dma_data_direction direction)
Maps a piece of processor virtual memory so it can be accessed by the
device and returns the DMA address of the memory.
...
Notes: Not all memory regions in a machine can be mapped by this API.
Further, contiguous kernel virtual space may not be contiguous as
physical memory. Since this API does not provide any scatter/gather
capability, it will fail if the user tries to map a non-physically
contiguous piece of memory. For this reason, memory to be mapped by
this API should be obtained from sources which guarantee it to be
physically contiguous (like kmalloc).
I'm not fully sure I understand yout concern, are you thinking the driver should
doublecheck the dma address returned by dma_map_single() and make sure it's
within the
dma mask set for the device?
-Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 6/8] xhci: remove GFP_DMA flag from allocation
From: Mathias Nyman
> Sent: 17 May 2017 16:32
> There is no reason to restrict allocations to the first 16MB ISA DMA
> addresses.
>
> It is causing problems in a virtualization setup with enabled IOMMU
> (x86_64). The result is that USB is not working in the VM.
...
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index 12b573c..1f1687e 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -56,7 +56,7 @@ static struct xhci_segment *xhci_segment_alloc(struct
> xhci_hcd *xhci,
> }
>
> if (max_packet) {
> - seg->bounce_buf = kzalloc(max_packet, flags | GFP_DMA);
> + seg->bounce_buf = kzalloc(max_packet, flags);
This might allocate memory that the device cannot access.
So can only work if dma_map_single() itself allocates a bounce buffer.
There must be a sane way to do this that doesn't ever require
double copies.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
