vhci doesn’t do DMA for remote device. Actually, the real DMA
operation is done by network card driver. vhci just passes virtual
address of the buffer to the network stack, so vhci doesn’t use and
need dma address of the buffer of the URB.
But HCD provides DMA mapping and unmapping function by default.
Moreover, it causes unnecessary DMA mapping and unmapping which
will be done again at the NIC driver and it wastes CPU cycles.
So, implement map_urb_for_dma and unmap_urb_for_dma function for
vhci in order to skip the DMA mapping and unmapping procedure.
When it comes to supporting SG for vhci, it is useful to use native
SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
fnuction can adjust the number of SG list (urb->num_mapped_sgs).
And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
hcd_map_urb_for_dma() does.
Signed-off-by: Suwan Kim <[email protected]>
---
drivers/usb/usbip/vhci_hcd.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index 000ab7225717..c62f7fa8118c 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
return 0;
}
+static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
+ gfp_t mem_flags)
+{
+ if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
+ WARN_ON(1);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void vhci_unmap_urb_for_dma(struct usb_hcd *hcd, struct urb *urb)
+{
+ dev_dbg(hcd->self.controller, "vhci does not unmap urb for dma\n");
+}
+
static const struct hc_driver vhci_hc_driver = {
.description = driver_name,
.product_desc = driver_desc,
@@ -1304,6 +1320,9 @@ static const struct hc_driver vhci_hc_driver = {
.get_frame_number = vhci_get_frame_number,
+ .map_urb_for_dma = vhci_map_urb_for_dma,
+ .unmap_urb_for_dma = vhci_unmap_urb_for_dma,
+
.hub_status_data = vhci_hub_status,
.hub_control = vhci_hub_control,
.bus_suspend = vhci_bus_suspend,
--
2.20.1
On 8/2/19 11:36 AM, Suwan Kim wrote:
> vhci doesn’t do DMA for remote device. Actually, the real DMA
> operation is done by network card driver. vhci just passes virtual
> address of the buffer to the network stack, so vhci doesn’t use and
> need dma address of the buffer of the URB.
>
> But HCD provides DMA mapping and unmapping function by default.
> Moreover, it causes unnecessary DMA mapping and unmapping which
> will be done again at the NIC driver and it wastes CPU cycles.
> So, implement map_urb_for_dma and unmap_urb_for_dma function for
> vhci in order to skip the DMA mapping and unmapping procedure.
>
> When it comes to supporting SG for vhci, it is useful to use native
> SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
> fnuction can adjust the number of SG list (urb->num_mapped_sgs).
> And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
> hcd_map_urb_for_dma() does.
>
> Signed-off-by: Suwan Kim <[email protected]>
> ---
> drivers/usb/usbip/vhci_hcd.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 000ab7225717..c62f7fa8118c 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
> return 0;
> }
>
> +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> + gfp_t mem_flags)
> +{
> + if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
> + WARN_ON(1);
Don't add WARN_ON. I cleaned them all up recently and don't want new
ones added.
thanks,
-- Shuah
On Fri, Aug 02, 2019 at 04:22:27PM -0600, shuah wrote:
> On 8/2/19 11:36 AM, Suwan Kim wrote:
> > vhci doesn’t do DMA for remote device. Actually, the real DMA
> > operation is done by network card driver. vhci just passes virtual
> > address of the buffer to the network stack, so vhci doesn’t use and
> > need dma address of the buffer of the URB.
> >
> > But HCD provides DMA mapping and unmapping function by default.
> > Moreover, it causes unnecessary DMA mapping and unmapping which
> > will be done again at the NIC driver and it wastes CPU cycles.
> > So, implement map_urb_for_dma and unmap_urb_for_dma function for
> > vhci in order to skip the DMA mapping and unmapping procedure.
> >
> > When it comes to supporting SG for vhci, it is useful to use native
> > SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
> > fnuction can adjust the number of SG list (urb->num_mapped_sgs).
> > And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
> > hcd_map_urb_for_dma() does.
> >
> > Signed-off-by: Suwan Kim <[email protected]>
> > ---
> > drivers/usb/usbip/vhci_hcd.c | 19 +++++++++++++++++++
> > 1 file changed, 19 insertions(+)
> >
> > diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> > index 000ab7225717..c62f7fa8118c 100644
> > --- a/drivers/usb/usbip/vhci_hcd.c
> > +++ b/drivers/usb/usbip/vhci_hcd.c
> > @@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
> > return 0;
> > }
> > +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
> > + gfp_t mem_flags)
> > +{
> > + if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
> > + WARN_ON(1);
>
> Don't add WARN_ON. I cleaned them all up recently and don't want new
> ones added.
Ok. I will remove it and resend v4.
Regards
Suwan Kim
On 8/4/19 11:23 PM, Suwan Kim wrote:
> On Fri, Aug 02, 2019 at 04:22:27PM -0600, shuah wrote:
>> On 8/2/19 11:36 AM, Suwan Kim wrote:
>>> vhci doesn’t do DMA for remote device. Actually, the real DMA
>>> operation is done by network card driver. vhci just passes virtual
>>> address of the buffer to the network stack, so vhci doesn’t use and
>>> need dma address of the buffer of the URB.
>>>
>>> But HCD provides DMA mapping and unmapping function by default.
>>> Moreover, it causes unnecessary DMA mapping and unmapping which
>>> will be done again at the NIC driver and it wastes CPU cycles.
>>> So, implement map_urb_for_dma and unmap_urb_for_dma function for
>>> vhci in order to skip the DMA mapping and unmapping procedure.
>>>
>>> When it comes to supporting SG for vhci, it is useful to use native
>>> SG list (urb->num_sgs) instead of mapped SG list because DMA mapping
>>> fnuction can adjust the number of SG list (urb->num_mapped_sgs).
>>> And vhci_map_urb_for_dma() prevents isoc pipe from using SG as
>>> hcd_map_urb_for_dma() does.
>>>
>>> Signed-off-by: Suwan Kim <[email protected]>
>>> ---
>>> drivers/usb/usbip/vhci_hcd.c | 19 +++++++++++++++++++
>>> 1 file changed, 19 insertions(+)
>>>
>>> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
>>> index 000ab7225717..c62f7fa8118c 100644
>>> --- a/drivers/usb/usbip/vhci_hcd.c
>>> +++ b/drivers/usb/usbip/vhci_hcd.c
>>> @@ -1288,6 +1288,22 @@ static int vhci_free_streams(struct usb_hcd *hcd, struct usb_device *udev,
>>> return 0;
>>> }
>>> +static int vhci_map_urb_for_dma(struct usb_hcd *hcd, struct urb *urb,
>>> + gfp_t mem_flags)
>>> +{
>>> + if (usb_endpoint_xfer_isoc(&urb->ep->desc) && urb->num_sgs) {
>>> + WARN_ON(1);
>>
>> Don't add WARN_ON. I cleaned them all up recently and don't want new
>> ones added.
>
> Ok. I will remove it and resend v4.
>
Please add appropriate error message in place of WARN_ON
thanks,
-- Shuah