linux-kernel - Re: [PATCH v2 3/3] usb: host: add the xhci offload hooks implementations

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:   Thu, 24 Nov 2022 10:08:00 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Albert Wang <albertccwang@...gle.com>
Cc:     mathias.nyman@...el.com, badhri@...gle.com, howardyen@...gle.com,
        linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org
Subject: Re: [PATCH v2 3/3] usb: host: add the xhci offload hooks
 implementations

On Thu, Nov 24, 2022 at 02:47:22PM +0800, Albert Wang wrote:
> > > +/*
> > > + * This is the driver call to co-processor for offload operations.
> > > + */
> > > +int offload_driver_call(enum usb_offload_msg msg, void *ptr)
> > > +{
> > > +     enum usb_offload_msg offload_msg;
> > > +     void *argptr;
> > > +
> > > +     offload_msg = msg;
> > > +     argptr = ptr;
> >
> > Don't just silence compiler warnings for no reason.
> >
> > Again, this does not actually do anything at all.  So how can we accept
> > this code?
> >
> 
> This is the driver call to our co-processor which is a specific
> hardware, so I don't submit it
> and make it silent here.

"specific hardware" is what Linux is all about!  Please submit your
actual drivers for this hardware, otherwise there is no way we can even
review properly this type of code, let alone accept it.

You all know this in great detail, I've been saying this for many years
now.  It is very frustrating on my end to constantly have to reject this
type of change all the time.

What would you do if you were on the reviewer's side?  Would you accept
this type of submission after constantly saying "I will only accept this
if you do X" and you get another patch that does NOT do "X"?

> We define and use those hook apis in the common xhci driver to offload
> some memory
> manipulation to a co-processor. So these apis will be executed to
> allocate or free memory,
> like dcbaa, transfer ring, in the co-processor memory space when
> offlooffload_driver_callad enabled. The file,
> xhci-offload-impl.c, shows how we use those xHCI hook apis for the
> offload implementation.
> 
> Here is the flow diagram:
> xHCI common driver        xHCI offload implement driver
> co-processor driver
> hooks
>                     offload_driver_call()
> ----------------------------
> ----------------------------------------
> --------------------------------------------------------------
> offload_init                         usb_audio_offload_init
> offload_cleanup                 usb_audio_offload_cleanup
> is_offload_enabled             is_offload_enabled
> alloc_dcbaa                        alloc_dcbaa
>        offload_driver_call(SET_DCBAA_PTR, &dcbaa_ptr);
> 
>                        offload_driver_call(SETUP_DONE, NULL);
> free_dcbaa                         free_dcbaa
> alloc_transfer_ring             alloc_transfer_ring
>    offload_driver_call(SET_ISOC_TR_INFO, &tr_info);
> free_transfer_ring              free_transfer_ring
> usb_offload_skip_urb        offload_skip_urb


This does not make any sense, sorry.  Perhaps your lines got wrapped
incorrectly?

thanks,

greg k-h