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

[Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>]

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.

Thanks,
Jae