linux-kernel - Re: [RFC v2] usb: xhci: add Immediate Data Transfer support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 06 Feb 2019 12:54:50 +0200
From:   Felipe Balbi <felipe.balbi@...ux.intel.com>
To:     Nicolas Saenz Julienne <nsaenzjulienne@...e.de>, oneukum@...e.com,
        Mathias Nyman <mathias.nyman@...el.com>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC v2] usb: xhci: add Immediate Data Transfer support


Hi,

Nicolas Saenz Julienne <nsaenzjulienne@...e.de> writes:
>> > diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> > index 005e65922608..dec62f7f5dc8 100644
>> > --- a/drivers/usb/host/xhci.c
>> > +++ b/drivers/usb/host/xhci.c
>> > @@ -1238,6 +1238,21 @@ EXPORT_SYMBOL_GPL(xhci_resume);
>> >  
>> >  /*-------------------------------------------------------------------------
>> > */
>> >  
>> > +/*
>> > + * Bypass the DMA mapping if URB is suitable for Immediate Transfer (IDT),
>> > + * we'll copy the actual data into the TRB address register. This is
>> > limited to
>> > + * transfers up to 8 bytes on output endpoints of any kind with
>> > wMaxPacketSize
>> > + * >= 8 bytes.
>> > + */
>> > +static int xhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
>> > +				gfp_t mem_flags)
>> > +{
>> > +	if (xhci_urb_suitable_for_idt(urb))
>> > +		return 0;
>> > +
>> > +	return usb_hcd_map_urb_for_dma(hcd, urb, mem_flags);
>> > +}
>> 
>> don't you need a matching unmap_urb_for_dma()??
>
> Not really as every DMA mapping sets a matching URB flag to track it. For
> example when usb_hcd_map_urb_for_dma() uses dma_map_single() it will set
> URB_DMA_MAP_SINGLE in urb->transfer_flags, later on unmap_urb_for_dma() will
> catch it and unmap it. As I bypass the mapping altogether there are no
> flags set, so unmap_urb_for_dma() won't have any effect.
>
> I could still add it for clarity, and well, I guess it'll save some
> instructions on the IDT suitable side.

thanks for the clarification.

>> > diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
>> > index 652dc36e3012..9d77b0901ab7 100644
>> > --- a/drivers/usb/host/xhci.h
>> > +++ b/drivers/usb/host/xhci.h
>> > @@ -1295,6 +1295,8 @@ enum xhci_setup_dev {
>> >  #define TRB_IOC			(1<<5)
>> >  /* The buffer pointer contains immediate data */
>> >  #define TRB_IDT			(1<<6)
>> > +/* TDs smaller than this might use IDT */
>> 
>> Technically, "TDs at most this" since you're 8 itself is an allowed

heh, I made a mess on this sentence, but I guess you got the gist of it.

cheers

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (833 bytes)