[PATCH v10 03/12] peci: Add support for PECI bus driver core

[Greg Kroah-Hartman]

On Fri, Jan 25, 2019 at 10:51:54AM -0800, Jae Hyun Yoo wrote:
> Hi Greg,
> 
> On 1/24/2019 11:18 PM, Greg Kroah-Hartman wrote:
> > On Thu, Jan 24, 2019 at 02:01:10PM -0800, Jae Hyun Yoo wrote:
> > > On 1/23/2019 10:57 PM, Greg Kroah-Hartman wrote:
> > > > On Wed, Jan 23, 2019 at 01:38:24PM -0800, Jae Hyun Yoo wrote:
> > > > > > What about the un-initialized fields in this structure?  Can you
> > > > > > properly handle that, and also, is this ok to be on the stack?
> > > > > 
> > > > > It's fully initialized at here because the peci_ping_msg struct has only
> > > > > one member:
> > > > > 
> > > > > struct peci_ping_msg {
> > > > > 	__u8 addr;
> > > > > };
> > > > 
> > > > Ok.  But my question about "can you do this off the stack" remains.
> > > 
> > > I'll add 3 bytes of dummy padding into this structure. Also, I'll check
> > > again u32 boundary alignment for all struct defines in peci_ioctl.h.
> > > Would it be okay to be on stack then?
> > 
> > The issue of being on the stack has nothing to do with alignment, and
> > everything to do with, "can your controller handle data from the stack".
> > Lots of busses and controllers can not (i.e. all USB devices), so you
> > have to properly allocate all memory that is used for data transfers
> > from areas that are able to do DMA properly (i.e. by using kmalloc).
> > 
> > That is why I asked here about that, if this is a USB driver, having the
> > data you wish to send from a stack variable is not allowed.  I don't
> > know how your hardware works, which is why I was asking this.
> > 
> > Note, some architectures (like x86), hide this fact as their stack
> > memory is able to be DMA, so you do not run into any errors.  Other
> > arches that Linux supports are not like that, which is why we have those
> > types of restrictions.
> 
> Thanks for your detailed explanation.
> 
> In this core driver, all PECI command messages will be translated into
> the raw PECI message structure (peci_xfer_msg) and it is the actual
> data which will be delivered to an adapter driver through
> adapter->xfer().
> 
> I'll fix the translation logic to use heap instead of stack for handling
> the peci_xfer_msg structure.

Great!  You might want to add the checks that we have in the USB core
for this type of thing to catch users who don't remember not to send
stack-data.  Look at usb_hcd_map_urb_for_dma() for specifics.

thanks,

greg k-h