From: Mathias Nyman
> Sent: 19 May 2017 10:49
> To: David Laight; gregkh@xxxxxxxxxxxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx; Matthias Lange; stable@xxxxxxxxxxxxxxx
> 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