sbefifo userspace api

sbefifo userspace api

[Brad Bishop]

Looking to start a discussion around possible user space and kernel APIs for the POWER9 sbefifo driver.

There exists today an “alternate" sbefifo driver :-) that provides a single submit ioctl.  Applications submit a request and get a reply in single system call.

Is something like that the best approach for an upstream driver?  Or should we try something more "pipe like" with read/write interfaces?

Would the in-kernel API be the same as the user space API?

thx

-brad

Re: sbefifo userspace api

[Jeremy Kerr]

Hi Brad,

> Looking to start a discussion around possible user space and kernel
> APIs for the POWER9 sbefifo driver.
> 
> There exists today an “alternate" sbefifo driver :-) that provides a
> single submit ioctl.  Applications submit a request and get a reply in
> single system call.
> 
> Is something like that the best approach for an upstream driver?  Or
> should we try something more "pipe like" with read/write interfaces?

It probably depends on the functionality there; ioctl() is useful in
that (as you say) we can handle request and response in a single
syscall, read() / write() may be more appropriate if ordering can be
handled in userspace.

Can you add a little description about the functionality we're exposing?
That may suggest a particular API.

> Would the in-kernel API be the same as the user space API?

Probably not :)

Cheers,


Jeremy

Re: sbefifo userspace api

[Brad Bishop]

Thanks Jeremy for the reply.  I’ve added participants from this thread:

https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html

in an attempt to consolidate the whole sbefifo design discussion to a single thread.

> Hi Brad,
> 
>> Looking to start a discussion around possible user space and kernel
>> APIs for the POWER9 sbefifo driver.
>> 
>> There exists today an “alternate" sbefifo driver :-) that provides a
>> single submit ioctl.  Applications submit a request and get a reply in
>> single system call.
>> 
>> Is something like that the best approach for an upstream driver?  Or
>> should we try something more "pipe like" with read/write interfaces?
> 
> It probably depends on the functionality there; ioctl() is useful in
> that (as you say) we can handle request and response in a single
> syscall, read() / write() may be more appropriate if ordering can be
> handled in userspace.
> 
> Can you add a little description about the functionality we're exposing?
> That may suggest a particular API.

In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.

In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.

For users there seems to be two classes:

1 - user space wanting to do chip-ops (pdbg, cronus).
2 - device drivers wanting to do chip-ops (occ-hwmon).

If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.

>> Would the in-kernel API be the same as the user space API?
> 
> Probably not :)

I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...

But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
would probably not have a split read/write API or vise versa.

So to rephrase the question - would the chardev fops implementation simply be something like this:

sbe-uapi-fops(chardev)
   data = copy to/from user space;
   sbedev = from_chardev(chardev);
   kernel-api(sbedev, data);

Or are the other things to consider here?

-thx

brad

> 
> Cheers,
> 
> 
> Jeremy

Re: sbefifo userspace api

[Venkatesh Sainath]

Hi Brad,

  I am not sure why we need a user-space api for SBE FIFO driver. Even 
the pdbg and cronus apps have to go through the SBEI protocol driver in 
order to construct the packets according to the SBEI interface spec 
which is done by the SBEI protocol driver only. I think only an 
in-kernel api is sufficient for the SBE FIFO driver.

  However, for the SBEI protocol driver, we would need both in-kernel ( 
for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and 
rest apis ).

Thanks

With regards
Venkatesh

On 20/02/17 8:17 AM, Brad Bishop wrote:
> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>
> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>
> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>
>> Hi Brad,
>>
>>> Looking to start a discussion around possible user space and kernel
>>> APIs for the POWER9 sbefifo driver.
>>>
>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>> single submit ioctl.  Applications submit a request and get a reply in
>>> single system call.
>>>
>>> Is something like that the best approach for an upstream driver?  Or
>>> should we try something more "pipe like" with read/write interfaces?
>> It probably depends on the functionality there; ioctl() is useful in
>> that (as you say) we can handle request and response in a single
>> syscall, read() / write() may be more appropriate if ordering can be
>> handled in userspace.
>>
>> Can you add a little description about the functionality we're exposing?
>> That may suggest a particular API.
> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>
> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>
> For users there seems to be two classes:
>
> 1 - user space wanting to do chip-ops (pdbg, cronus).
> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>
> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>
>>> Would the in-kernel API be the same as the user space API?
>> Probably not :)
> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>
> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> would probably not have a split read/write API or vise versa.
>
> So to rephrase the question - would the chardev fops implementation simply be something like this:
>
> sbe-uapi-fops(chardev)
>     data = copy to/from user space;
>     sbedev = from_chardev(chardev);
>     kernel-api(sbedev, data);
>
> Or are the other things to consider here?
>
> -thx
>
> brad
>
>> Cheers,
>>
>>
>> Jeremy

Re: sbefifo userspace api

[Brad Bishop]

Hi Venkatesh

If we put all the chip ops in the kernel then I would agree with you.

However, a number of others have expressed concern with putting the chip ops in the kernel.

Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?

thx - brad



> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> 
> Hi Brad,
> 
> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> 
> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> 
> Thanks
> 
> With regards
> Venkatesh
> 
> On 20/02/17 8:17 AM, Brad Bishop wrote:
>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>> 
>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>> 
>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>> 
>>> Hi Brad,
>>> 
>>>> Looking to start a discussion around possible user space and kernel
>>>> APIs for the POWER9 sbefifo driver.
>>>> 
>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>> single system call.
>>>> 
>>>> Is something like that the best approach for an upstream driver?  Or
>>>> should we try something more "pipe like" with read/write interfaces?
>>> It probably depends on the functionality there; ioctl() is useful in
>>> that (as you say) we can handle request and response in a single
>>> syscall, read() / write() may be more appropriate if ordering can be
>>> handled in userspace.
>>> 
>>> Can you add a little description about the functionality we're exposing?
>>> That may suggest a particular API.
>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>> 
>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>> 
>> For users there seems to be two classes:
>> 
>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>> 
>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>> 
>>>> Would the in-kernel API be the same as the user space API?
>>> Probably not :)
>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>> 
>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>> would probably not have a split read/write API or vise versa.
>> 
>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>> 
>> sbe-uapi-fops(chardev)
>>    data = copy to/from user space;
>>    sbedev = from_chardev(chardev);
>>    kernel-api(sbedev, data);
>> 
>> Or are the other things to consider here?
>> 
>> -thx
>> 
>> brad
>> 
>>> Cheers,
>>> 
>>> 
>>> Jeremy

Re: sbefifo userspace api

[Alistair Popple]

On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
> Hi Venkatesh
>
> If we put all the chip ops in the kernel then I would agree with you.
>
> However, a number of others have expressed concern with putting the chip ops in the kernel.
>
> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?

The main concern I had was that putting the chip-ops in the kernel
makes it more complicated to update/add chip-ops. Having them in the
kernel would require a userspace API for each chip-op that would not
be able to change, or at least could only change in a way that does
not break backwards compatibility. Putting them in userpsace would
allow you to build a library and manage any API changes between the
users of that library.

I would also be interested in the answer to the inverse - why put them
in the kernel? There's the OCC hwmon driver - what chip-ops does that
need? Just get/putscom? If that's the case I thought we were already
exposing scom chip-op operations via an OpenFSI master?

Regards,

Alistair

> thx - brad
>
>
>
> > On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >
> > Hi Brad,
> >
> > I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >
> > However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >
> > Thanks
> >
> > With regards
> > Venkatesh
> >
> > On 20/02/17 8:17 AM, Brad Bishop wrote:
> >> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>
> >> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>
> >> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>
> >>> Hi Brad,
> >>>
> >>>> Looking to start a discussion around possible user space and kernel
> >>>> APIs for the POWER9 sbefifo driver.
> >>>>
> >>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>> single system call.
> >>>>
> >>>> Is something like that the best approach for an upstream driver?  Or
> >>>> should we try something more "pipe like" with read/write interfaces?
> >>> It probably depends on the functionality there; ioctl() is useful in
> >>> that (as you say) we can handle request and response in a single
> >>> syscall, read() / write() may be more appropriate if ordering can be
> >>> handled in userspace.
> >>>
> >>> Can you add a little description about the functionality we're exposing?
> >>> That may suggest a particular API.
> >> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>
> >> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>
> >> For users there seems to be two classes:
> >>
> >> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>
> >> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>
> >>>> Would the in-kernel API be the same as the user space API?
> >>> Probably not :)
> >> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>
> >> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >> would probably not have a split read/write API or vise versa.
> >>
> >> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>
> >> sbe-uapi-fops(chardev)
> >>    data = copy to/from user space;
> >>    sbedev = from_chardev(chardev);
> >>    kernel-api(sbedev, data);
> >>
> >> Or are the other things to consider here?
> >>
> >> -thx
> >>
> >> brad
> >>
> >>> Cheers,
> >>>
> >>>
> >>> Jeremy

Re: sbefifo userspace api

[Brad Bishop]

Thx Alistair!

> On Feb 20, 2017, at 10:34 AM, Alistair Popple <alistair@popple.id.au> wrote:
> 
> On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
>> Hi Venkatesh
>> 
>> If we put all the chip ops in the kernel then I would agree with you.
>> 
>> However, a number of others have expressed concern with putting the chip ops in the kernel.
>> 
>> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?
> 
> The main concern I had was that putting the chip-ops in the kernel
> makes it more complicated to update/add chip-ops. Having them in the
> kernel would require a userspace API for each chip-op that would not
> be able to change, or at least could only change in a way that does
> not break backwards compatibility. Putting them in userpsace would
> allow you to build a library and manage any API changes between the
> users of that library.

Fair point.  What makes sbescom a special exception to this?  Are we OK with a real, fixed UAPI for sbescom, but the others are implemented in a library?  That feels inconsistent to me.

> 
> I would also be interested in the answer to the inverse - why put them
> in the kernel?

Fair question.  I’m hoping Venkatesh responds with some things I may have overlooked.

The only case I have for this is the consistency argument I made above.  I would rather see all or none in the kernel.  I’d be fine with none, but I can’t see a way to implement occ-hwmon that way.  So I fall back to all.  

Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.

> There's the OCC hwmon driver - what chip-ops does that
> need? Just get/putscom?

Correct, just get/putscom.  But sbe scom, not direct scom.

> If that's the case I thought we were already
> exposing scom chip-op operations via an OpenFSI master?

I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?

> 
> Regards,
> 
> Alistair
> 
>> thx - brad
>> 
>> 
>> 
>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>> 
>>> Hi Brad,
>>> 
>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>> 
>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>> 
>>> Thanks
>>> 
>>> With regards
>>> Venkatesh
>>> 
>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>> 
>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>> 
>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>> 
>>>>> Hi Brad,
>>>>> 
>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>> 
>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>> single system call.
>>>>>> 
>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>> that (as you say) we can handle request and response in a single
>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>> handled in userspace.
>>>>> 
>>>>> Can you add a little description about the functionality we're exposing?
>>>>> That may suggest a particular API.
>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>> 
>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>> 
>>>> For users there seems to be two classes:
>>>> 
>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>> 
>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>> 
>>>>>> Would the in-kernel API be the same as the user space API?
>>>>> Probably not :)
>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>> 
>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>> would probably not have a split read/write API or vise versa.
>>>> 
>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>> 
>>>> sbe-uapi-fops(chardev)
>>>>   data = copy to/from user space;
>>>>   sbedev = from_chardev(chardev);
>>>>   kernel-api(sbedev, data);
>>>> 
>>>> Or are the other things to consider here?
>>>> 
>>>> -thx
>>>> 
>>>> brad
>>>> 
>>>>> Cheers,
>>>>> 
>>>>> 
>>>>> Jeremy

Re: sbefifo userspace api

[Venkatesh Sainath]

On 20/02/17 10:22 PM, Brad Bishop wrote:
> Thx Alistair!
>
>> On Feb 20, 2017, at 10:34 AM, Alistair Popple <alistair@popple.id.au> wrote:
>>
>> On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
>>> Hi Venkatesh
>>>
>>> If we put all the chip ops in the kernel then I would agree with you.
>>>
>>> However, a number of others have expressed concern with putting the chip ops in the kernel.
>>>
>>> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?
>> The main concern I had was that putting the chip-ops in the kernel
>> makes it more complicated to update/add chip-ops. Having them in the
>> kernel would require a userspace API for each chip-op that would not
>> be able to change, or at least could only change in a way that does
>> not break backwards compatibility. Putting them in userpsace would
>> allow you to build a library and manage any API changes between the
>> users of that library.
> Fair point.  What makes sbescom a special exception to this?  Are we OK with a real, fixed UAPI for sbescom, but the others are implemented in a library?  That feels inconsistent to me.
As the interface is itself controlled by the specification, we should 
anyway make it backward compatible. I was also hoping that the sbei 
protocol driver sits in user-space as a library (if not for the occ 
hwmon...).
>> I would also be interested in the answer to the inverse - why put them
>> in the kernel?
> Fair question.  I’m hoping Venkatesh responds with some things I may have overlooked.
>
> The only case I have for this is the consistency argument I made above.  I would rather see all or none in the kernel.  I’d be fine with none, but I can’t see a way to implement occ-hwmon that way.  So I fall back to all.
>
> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
>
>> There's the OCC hwmon driver - what chip-ops does that
>> need? Just get/putscom?
> Correct, just get/putscom.  But sbe scom, not direct scom.
occ hwmon driver should use get/put memory to communicate with OCC. One 
other option ( I am not too happy about this ) is to embed the get/put 
memory protocol construction inside occ hwmon driver and make it call 
sbe fifo driver interface directly. The sbei protocol driver can then be 
in the user-space as a library and call the sbe fifo driver interface 
via ioctl. This creates duplication of the protocol driver code and we 
have to fix it in multiple places.
>> If that's the case I thought we were already
>> exposing scom chip-op operations via an OpenFSI master?
> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
openfsi driver is not exposing chipops. The sbe fifo driver must be 
calling the openfsi driver interfaces to write to the fifo. The openfsi 
driver must also be providing a UAPI for a non-sbe scom or cfam-write 
operations via cronus, pdbg or rest api.

Net: openfsi driver need to have both intra-kernel and user-space APIs. 
sbe fifo driver also need to have both intra-kernel and user-space APIs 
if the sbei protocol driver goes to user-space.
>> Regards,
>>
>> Alistair
>>
>>> thx - brad
>>>
>>>
>>>
>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>
>>>> Hi Brad,
>>>>
>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>
>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>>>
>>>> Thanks
>>>>
>>>> With regards
>>>> Venkatesh
>>>>
>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>>>
>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>
>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>>>
>>>>>> Hi Brad,
>>>>>>
>>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>
>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>>> single system call.
>>>>>>>
>>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>>> that (as you say) we can handle request and response in a single
>>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>>> handled in userspace.
>>>>>>
>>>>>> Can you add a little description about the functionality we're exposing?
>>>>>> That may suggest a particular API.
>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>>>
>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>>>
>>>>> For users there seems to be two classes:
>>>>>
>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>
>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>>>
>>>>>>> Would the in-kernel API be the same as the user space API?
>>>>>> Probably not :)
>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>>>
>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>> would probably not have a split read/write API or vise versa.
>>>>>
>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>>>
>>>>> sbe-uapi-fops(chardev)
>>>>>    data = copy to/from user space;
>>>>>    sbedev = from_chardev(chardev);
>>>>>    kernel-api(sbedev, data);
>>>>>
>>>>> Or are the other things to consider here?
>>>>>
>>>>> -thx
>>>>>
>>>>> brad
>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>>
>>>>>> Jeremy

Re: sbefifo userspace api

[Alistair Popple]

Hi,

On Mon, 20 Feb 2017 10:35:12 PM Venkatesh Sainath wrote:
>
> On 20/02/17 10:22 PM, Brad Bishop wrote:
> > Thx Alistair!
> >
> >> On Feb 20, 2017, at 10:34 AM, Alistair Popple <alistair@popple.id.au> wrote:
> >>
> >> On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
> >>> Hi Venkatesh
> >>>
> >>> If we put all the chip ops in the kernel then I would agree with you.
> >>>
> >>> However, a number of others have expressed concern with putting the chip ops in the kernel.
> >>>
> >>> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?
> >> The main concern I had was that putting the chip-ops in the kernel
> >> makes it more complicated to update/add chip-ops. Having them in the
> >> kernel would require a userspace API for each chip-op that would not
> >> be able to change, or at least could only change in a way that does
> >> not break backwards compatibility. Putting them in userpsace would
> >> allow you to build a library and manage any API changes between the
> >> users of that library.
> > Fair point.  What makes sbescom a special exception to this?  Are we OK with a real, fixed UAPI for sbescom, but the others are implemented in a library?  That feels inconsistent to me.

The thing that I think makes sbescom special is that getscom/putscom
is a very well understood low-level and general purpose operation that
is also needed for kernel drivers. We also have existing userspace and
kernel interfaces for doing get/putscom where as many of the other
operations are quite specialised (eg. get trace array).

If we put all the chip-ops in the kernel we would need to define and
review all these interfaces.

> As the interface is itself controlled by the specification, we should
> anyway make it backward compatible. I was also hoping that the sbei
> protocol driver sits in user-space as a library (if not for the occ
> hwmon...).
> >> I would also be interested in the answer to the inverse - why put them
> >> in the kernel?
> > Fair question.  I’m hoping Venkatesh responds with some things I may have overlooked.
> >
> > The only case I have for this is the consistency argument I made above.  I would rather see all or none in the kernel.  I’d be fine with none, but I can’t see a way to implement occ-hwmon that way.  So I fall back to all.

I can understand where you are coming from with the consistency
argument but imho I don't really think it is very compelling in this
case - it seems we broadly agree that chip-ops should live in userspace
except for the problem of the OCC hwmon driver.

I don't think it is a problem having get/putscom as an exceptional
case for the reasons described above. My question about the
consistency argument is what does it get us?

I think Venkatesh was suggesting it would reduce duplication of
protocol driver code, however perhaps this isn't the case. We could
create a kernel chip-ops driver that deals with sending a buffer and
getting a response but doesn't have to know anything about the chip-op
itself. It could also implement the kernel sbe get/putscom.

What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
of the sbe interface spec. Userspace would submit command-class,
command-code & data words and the chip-ops driver would forward that
to the SBE and send the response back to userspace without having to
know what any of that data means.

In any case I think you will end up with a userspace implementation of
chip-ops anyway as it is much easier to test and develop new chip-ops
without having to also understand how to build and flash a new kernel.
Inevitably someone will want to add some kind of "debug" chip-op or
other private chip-op that they don't want published in kernel code,
and as soon as you have one userspace chip-op you may as well do all
of the ones you can from there.

> > Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
> >
> >> There's the OCC hwmon driver - what chip-ops does that
> >> need? Just get/putscom?
> > Correct, just get/putscom.  But sbe scom, not direct scom.
> occ hwmon driver should use get/put memory to communicate with OCC. One

What memory is the OCC using to store these buffer? Does it have
memory mapped onto the PowerBus into an MMIO space or something?

> other option ( I am not too happy about this ) is to embed the get/put
> memory protocol construction inside occ hwmon driver and make it call
> sbe fifo driver interface directly. The sbei protocol driver can then be
> in the user-space as a library and call the sbe fifo driver interface
> via ioctl. This creates duplication of the protocol driver code and we
> have to fix it in multiple places.
> >> If that's the case I thought we were already
> >> exposing scom chip-op operations via an OpenFSI master?
> > I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
> openfsi driver is not exposing chipops. The sbe fifo driver must be
> calling the openfsi driver interfaces to write to the fifo. The openfsi
> driver must also be providing a UAPI for a non-sbe scom or cfam-write
> operations via cronus, pdbg or rest api.
>
> Net: openfsi driver need to have both intra-kernel and user-space APIs.
> sbe fifo driver also need to have both intra-kernel and user-space APIs
> if the sbei protocol driver goes to user-space.
> >> Regards,
> >>
> >> Alistair
> >>
> >>> thx - brad
> >>>
> >>>
> >>>
> >>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>>>
> >>>> Hi Brad,
> >>>>
> >>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >>>>
> >>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >>>>
> >>>> Thanks
> >>>>
> >>>> With regards
> >>>> Venkatesh
> >>>>
> >>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
> >>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>>>>
> >>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>>>>
> >>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>>>>
> >>>>>> Hi Brad,
> >>>>>>
> >>>>>>> Looking to start a discussion around possible user space and kernel
> >>>>>>> APIs for the POWER9 sbefifo driver.
> >>>>>>>
> >>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>>>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>>>>> single system call.
> >>>>>>>
> >>>>>>> Is something like that the best approach for an upstream driver?  Or
> >>>>>>> should we try something more "pipe like" with read/write interfaces?
> >>>>>> It probably depends on the functionality there; ioctl() is useful in
> >>>>>> that (as you say) we can handle request and response in a single
> >>>>>> syscall, read() / write() may be more appropriate if ordering can be
> >>>>>> handled in userspace.
> >>>>>>
> >>>>>> Can you add a little description about the functionality we're exposing?
> >>>>>> That may suggest a particular API.
> >>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>>>>
> >>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>>>>
> >>>>> For users there seems to be two classes:
> >>>>>
> >>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>>>>
> >>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>>>>
> >>>>>>> Would the in-kernel API be the same as the user space API?
> >>>>>> Probably not :)
> >>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>>>>
> >>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >>>>> would probably not have a split read/write API or vise versa.
> >>>>>
> >>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>>>>
> >>>>> sbe-uapi-fops(chardev)
> >>>>>    data = copy to/from user space;
> >>>>>    sbedev = from_chardev(chardev);
> >>>>>    kernel-api(sbedev, data);
> >>>>>
> >>>>> Or are the other things to consider here?
> >>>>>
> >>>>> -thx
> >>>>>
> >>>>> brad
> >>>>>
> >>>>>> Cheers,
> >>>>>>
> >>>>>>
> >>>>>> Jeremy
>

Re: sbefifo userspace api

[Brad Bishop]

> On Feb 20, 2017, at 4:08 PM, Alistair Popple <alistair@popple.id.au> wrote:
> 
> Hi,
> 
> On Mon, 20 Feb 2017 10:35:12 PM Venkatesh Sainath wrote:
>> 
>> On 20/02/17 10:22 PM, Brad Bishop wrote:
>>> Thx Alistair!
>>> 
>>>> On Feb 20, 2017, at 10:34 AM, Alistair Popple <alistair@popple.id.au> wrote:
>>>> 
>>>> On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
>>>>> Hi Venkatesh
>>>>> 
>>>>> If we put all the chip ops in the kernel then I would agree with you.
>>>>> 
>>>>> However, a number of others have expressed concern with putting the chip ops in the kernel.
>>>>> 
>>>>> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?
>>>> The main concern I had was that putting the chip-ops in the kernel
>>>> makes it more complicated to update/add chip-ops. Having them in the
>>>> kernel would require a userspace API for each chip-op that would not
>>>> be able to change, or at least could only change in a way that does
>>>> not break backwards compatibility. Putting them in userpsace would
>>>> allow you to build a library and manage any API changes between the
>>>> users of that library.
>>> Fair point.  What makes sbescom a special exception to this?  Are we OK with a real, fixed UAPI for sbescom, but the others are implemented in a library?  That feels inconsistent to me.
> 
> The thing that I think makes sbescom special is that getscom/putscom
> is a very well understood low-level and general purpose operation that
> is also needed for kernel drivers. We also have existing userspace and
> kernel interfaces for doing get/putscom where as many of the other
> operations are quite specialised (eg. get trace array).
> 
> If we put all the chip-ops in the kernel we would need to define and
> review all these interfaces.
> 
>> As the interface is itself controlled by the specification, we should
>> anyway make it backward compatible. I was also hoping that the sbei
>> protocol driver sits in user-space as a library (if not for the occ
>> hwmon...).
>>>> I would also be interested in the answer to the inverse - why put them
>>>> in the kernel?
>>> Fair question.  I’m hoping Venkatesh responds with some things I may have overlooked.
>>> 
>>> The only case I have for this is the consistency argument I made above.  I would rather see all or none in the kernel.  I’d be fine with none, but I can’t see a way to implement occ-hwmon that way.  So I fall back to all.
> 
> I can understand where you are coming from with the consistency
> argument but imho I don't really think it is very compelling in this
> case - it seems we broadly agree that chip-ops should live in userspace
> except for the problem of the OCC hwmon driver.

yup.

> 
> I don't think it is a problem having get/putscom as an exceptional
> case for the reasons described above. My question about the
> consistency argument is what does it get us?
> 
> I think Venkatesh was suggesting it would reduce duplication of
> protocol driver code, however perhaps this isn't the case. We could
> create a kernel chip-ops driver that deals with sending a buffer and
> getting a response but doesn't have to know anything about the chip-op
> itself. It could also implement the kernel sbe get/putscom.
> 
> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
> of the sbe interface spec. Userspace would submit command-class,
> command-code & data words and the chip-ops driver would forward that
> to the SBE and send the response back to userspace without having to
> know what any of that data means.

I like it!

Would this ‘chip-op’ driver be distinct from the sbefifo driver or
are you proposing a possible sbefifo driver API with a tad bit more
abstraction?

If we did something like this, does a standalone sbescom driver still
have any value?

> 
> In any case I think you will end up with a userspace implementation of
> chip-ops anyway as it is much easier to test and develop new chip-ops
> without having to also understand how to build and flash a new kernel.
> Inevitably someone will want to add some kind of "debug" chip-op or
> other private chip-op that they don't want published in kernel code,
> and as soon as you have one userspace chip-op you may as well do all
> of the ones you can from there.
> 
>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
>>> 
>>>> There's the OCC hwmon driver - what chip-ops does that
>>>> need? Just get/putscom?
>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>> occ hwmon driver should use get/put memory to communicate with OCC. One
> 
> What memory is the OCC using to store these buffer? Does it have
> memory mapped onto the PowerBus into an MMIO space or something?
>> other option ( I am not too happy about this ) is to embed the get/put
>> memory protocol construction inside occ hwmon driver and make it call
>> sbe fifo driver interface directly. The sbei protocol driver can then be
>> in the user-space as a library and call the sbe fifo driver interface
>> via ioctl. This creates duplication of the protocol driver code and we
>> have to fix it in multiple places.
>>>> If that's the case I thought we were already
>>>> exposing scom chip-op operations via an OpenFSI master?
>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
>> openfsi driver is not exposing chipops. The sbe fifo driver must be
>> calling the openfsi driver interfaces to write to the fifo. The openfsi
>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
>> operations via cronus, pdbg or rest api.
>> 
>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
>> sbe fifo driver also need to have both intra-kernel and user-space APIs
>> if the sbei protocol driver goes to user-space.
>>>> Regards,
>>>> 
>>>> Alistair
>>>> 
>>>>> thx - brad
>>>>> 
>>>>> 
>>>>> 
>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>>> 
>>>>>> Hi Brad,
>>>>>> 
>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>>> 
>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>>>>> 
>>>>>> Thanks
>>>>>> 
>>>>>> With regards
>>>>>> Venkatesh
>>>>>> 
>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>>>>> 
>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>>> 
>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>>>>> 
>>>>>>>> Hi Brad,
>>>>>>>> 
>>>>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>>> 
>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>>>>> single system call.
>>>>>>>>> 
>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>>>>> that (as you say) we can handle request and response in a single
>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>>>>> handled in userspace.
>>>>>>>> 
>>>>>>>> Can you add a little description about the functionality we're exposing?
>>>>>>>> That may suggest a particular API.
>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>>>>> 
>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>>>>> 
>>>>>>> For users there seems to be two classes:
>>>>>>> 
>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>>> 
>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>>>>> 
>>>>>>>>> Would the in-kernel API be the same as the user space API?
>>>>>>>> Probably not :)
>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>>>>> 
>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>>>> would probably not have a split read/write API or vise versa.
>>>>>>> 
>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>>>>> 
>>>>>>> sbe-uapi-fops(chardev)
>>>>>>>   data = copy to/from user space;
>>>>>>>   sbedev = from_chardev(chardev);
>>>>>>>   kernel-api(sbedev, data);
>>>>>>> 
>>>>>>> Or are the other things to consider here?
>>>>>>> 
>>>>>>> -thx
>>>>>>> 
>>>>>>> brad
>>>>>>> 
>>>>>>>> Cheers,
>>>>>>>> 
>>>>>>>> 
>>>>>>>> Jeremy

Re: sbefifo userspace api

[Alistair Popple]

<snip>

On Mon, 20 Feb 2017 05:05:45 PM Brad Bishop wrote:
> >
> > I don't think it is a problem having get/putscom as an exceptional
> > case for the reasons described above. My question about the
> > consistency argument is what does it get us?
> >
> > I think Venkatesh was suggesting it would reduce duplication of
> > protocol driver code, however perhaps this isn't the case. We could
> > create a kernel chip-ops driver that deals with sending a buffer and
> > getting a response but doesn't have to know anything about the chip-op
> > itself. It could also implement the kernel sbe get/putscom.
> >
> > What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
> > of the sbe interface spec. Userspace would submit command-class,
> > command-code & data words and the chip-ops driver would forward that
> > to the SBE and send the response back to userspace without having to
> > know what any of that data means.
>
> I like it!
>
> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
> are you proposing a possible sbefifo driver API with a tad bit more
> abstraction?

From what I can tell from the FIFO documentation it looks like the
FIFO can only be meaningfully used with the SBE? If that's the case I
don't see value in having a distrinct "chip-op" driver - just call it
the sbefifo driver.

The kernel can deal with framing data (and managing concurrent access)
while userspace deals with the actual data. This is very similar to
the way the IPMI/BT interfaces work.

> If we did something like this, does a standalone sbescom driver still
> have any value?

You still need an in-kernel sbescom driver for the OCC hwmon driver
right? I guess the question is do we write a complete sbescom driver
exposing a userspace API or do we just add an OCC specific hook?

The SBE command/response buffers for scom access are pretty straight
forward so I don't care much either way. Given the simplicity I doubt
code duplication is a concern.

> >
> > In any case I think you will end up with a userspace implementation of
> > chip-ops anyway as it is much easier to test and develop new chip-ops
> > without having to also understand how to build and flash a new kernel.
> > Inevitably someone will want to add some kind of "debug" chip-op or
> > other private chip-op that they don't want published in kernel code,
> > and as soon as you have one userspace chip-op you may as well do all
> > of the ones you can from there.
> >
> >>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
> >>>
> >>>> There's the OCC hwmon driver - what chip-ops does that
> >>>> need? Just get/putscom?
> >>> Correct, just get/putscom.  But sbe scom, not direct scom.
> >> occ hwmon driver should use get/put memory to communicate with OCC. One
> >
> > What memory is the OCC using to store these buffer? Does it have
> > memory mapped onto the PowerBus into an MMIO space or something?
> >> other option ( I am not too happy about this ) is to embed the get/put
> >> memory protocol construction inside occ hwmon driver and make it call
> >> sbe fifo driver interface directly. The sbei protocol driver can then be
> >> in the user-space as a library and call the sbe fifo driver interface
> >> via ioctl. This creates duplication of the protocol driver code and we
> >> have to fix it in multiple places.
> >>>> If that's the case I thought we were already
> >>>> exposing scom chip-op operations via an OpenFSI master?
> >>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
> >> openfsi driver is not exposing chipops. The sbe fifo driver must be
> >> calling the openfsi driver interfaces to write to the fifo. The openfsi
> >> driver must also be providing a UAPI for a non-sbe scom or cfam-write
> >> operations via cronus, pdbg or rest api.
> >>
> >> Net: openfsi driver need to have both intra-kernel and user-space APIs.
> >> sbe fifo driver also need to have both intra-kernel and user-space APIs
> >> if the sbei protocol driver goes to user-space.
> >>>> Regards,
> >>>>
> >>>> Alistair
> >>>>
> >>>>> thx - brad
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>>>>>
> >>>>>> Hi Brad,
> >>>>>>
> >>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >>>>>>
> >>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >>>>>>
> >>>>>> Thanks
> >>>>>>
> >>>>>> With regards
> >>>>>> Venkatesh
> >>>>>>
> >>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
> >>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>>>>>>
> >>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>>>>>>
> >>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>>>>>>
> >>>>>>>> Hi Brad,
> >>>>>>>>
> >>>>>>>>> Looking to start a discussion around possible user space and kernel
> >>>>>>>>> APIs for the POWER9 sbefifo driver.
> >>>>>>>>>
> >>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>>>>>>> single system call.
> >>>>>>>>>
> >>>>>>>>> Is something like that the best approach for an upstream driver?  Or
> >>>>>>>>> should we try something more "pipe like" with read/write interfaces?
> >>>>>>>> It probably depends on the functionality there; ioctl() is useful in
> >>>>>>>> that (as you say) we can handle request and response in a single
> >>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
> >>>>>>>> handled in userspace.
> >>>>>>>>
> >>>>>>>> Can you add a little description about the functionality we're exposing?
> >>>>>>>> That may suggest a particular API.
> >>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>>>>>>
> >>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>>>>>>
> >>>>>>> For users there seems to be two classes:
> >>>>>>>
> >>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>>>>>>
> >>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>>>>>>
> >>>>>>>>> Would the in-kernel API be the same as the user space API?
> >>>>>>>> Probably not :)
> >>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>>>>>>
> >>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >>>>>>> would probably not have a split read/write API or vise versa.
> >>>>>>>
> >>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>>>>>>
> >>>>>>> sbe-uapi-fops(chardev)
> >>>>>>>   data = copy to/from user space;
> >>>>>>>   sbedev = from_chardev(chardev);
> >>>>>>>   kernel-api(sbedev, data);
> >>>>>>>
> >>>>>>> Or are the other things to consider here?
> >>>>>>>
> >>>>>>> -thx
> >>>>>>>
> >>>>>>> brad
> >>>>>>>
> >>>>>>>> Cheers,
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Jeremy

Re: sbefifo userspace api

[Venkatesh Sainath]

On 21/02/17 3:35 AM, Brad Bishop wrote:
>> On Feb 20, 2017, at 4:08 PM, Alistair Popple <alistair@popple.id.au> wrote:
>>
>> Hi,
>>
>> On Mon, 20 Feb 2017 10:35:12 PM Venkatesh Sainath wrote:
>>> On 20/02/17 10:22 PM, Brad Bishop wrote:
>>>> Thx Alistair!
>>>>
>>>>> On Feb 20, 2017, at 10:34 AM, Alistair Popple <alistair@popple.id.au> wrote:
>>>>>
>>>>> On Mon, 20 Feb 2017 07:20:25 AM Brad Bishop wrote:
>>>>>> Hi Venkatesh
>>>>>>
>>>>>> If we put all the chip ops in the kernel then I would agree with you.
>>>>>>
>>>>>> However, a number of others have expressed concern with putting the chip ops in the kernel.
>>>>>>
>>>>>> Joel/Alistair - can you elaborate on your concerns with this approach of putting the chip-ops in the kernel?
>>>>> The main concern I had was that putting the chip-ops in the kernel
>>>>> makes it more complicated to update/add chip-ops. Having them in the
>>>>> kernel would require a userspace API for each chip-op that would not
>>>>> be able to change, or at least could only change in a way that does
>>>>> not break backwards compatibility. Putting them in userpsace would
>>>>> allow you to build a library and manage any API changes between the
>>>>> users of that library.
>>>> Fair point.  What makes sbescom a special exception to this?  Are we OK with a real, fixed UAPI for sbescom, but the others are implemented in a library?  That feels inconsistent to me.
>> The thing that I think makes sbescom special is that getscom/putscom
>> is a very well understood low-level and general purpose operation that
>> is also needed for kernel drivers. We also have existing userspace and
>> kernel interfaces for doing get/putscom where as many of the other
>> operations are quite specialised (eg. get trace array).
>>
>> If we put all the chip-ops in the kernel we would need to define and
>> review all these interfaces.
>>
>>> As the interface is itself controlled by the specification, we should
>>> anyway make it backward compatible. I was also hoping that the sbei
>>> protocol driver sits in user-space as a library (if not for the occ
>>> hwmon...).
>>>>> I would also be interested in the answer to the inverse - why put them
>>>>> in the kernel?
>>>> Fair question.  I’m hoping Venkatesh responds with some things I may have overlooked.
>>>>
>>>> The only case I have for this is the consistency argument I made above.  I would rather see all or none in the kernel.  I’d be fine with none, but I can’t see a way to implement occ-hwmon that way.  So I fall back to all.
>> I can understand where you are coming from with the consistency
>> argument but imho I don't really think it is very compelling in this
>> case - it seems we broadly agree that chip-ops should live in userspace
>> except for the problem of the OCC hwmon driver.
> yup.
In general, I agree with this approach to contain only those interfaces 
required by hwmon within the kernel and leave the rest to the 
user-space. But, we need to interlock with the hwmon to understand 
exactly which interfaces are required.
IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b) 
Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If 
these three are the right ones, then we need to contain them within the 
kernel.
>> I don't think it is a problem having get/putscom as an exceptional
>> case for the reasons described above. My question about the
>> consistency argument is what does it get us?
>>
>> I think Venkatesh was suggesting it would reduce duplication of
>> protocol driver code, however perhaps this isn't the case. We could
>> create a kernel chip-ops driver that deals with sending a buffer and
>> getting a response but doesn't have to know anything about the chip-op
>> itself. It could also implement the kernel sbe get/putscom.
>>
>> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
>> of the sbe interface spec. Userspace would submit command-class,
>> command-code & data words and the chip-ops driver would forward that
>> to the SBE and send the response back to userspace without having to
>> know what any of that data means.
> I like it!
>
> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
> are you proposing a possible sbefifo driver API with a tad bit more
> abstraction?
>
> If we did something like this, does a standalone sbescom driver still
> have any value?
I think the proposal is to have a combined sbefifo+sbechipop driver that 
provides a user space api to submit operations to sbefifo and user space 
api for those chipops contained within ( scom, memory and sram). The 
user space library will have the other sbe interfaces and will call the 
submit operations of the chip-op driver. Is this correct? We could get 
the user-space library provide a wrapper interface for even the chip-ops 
contained within the kernel so that the caller doesnt have to know 
whether to call the library or the driver apis.
>> In any case I think you will end up with a userspace implementation of
>> chip-ops anyway as it is much easier to test and develop new chip-ops
>> without having to also understand how to build and flash a new kernel.
>> Inevitably someone will want to add some kind of "debug" chip-op or
>> other private chip-op that they don't want published in kernel code,
>> and as soon as you have one userspace chip-op you may as well do all
>> of the ones you can from there.
>>
>>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
>>>>
>>>>> There's the OCC hwmon driver - what chip-ops does that
>>>>> need? Just get/putscom?
>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>>> occ hwmon driver should use get/put memory to communicate with OCC. One
>> What memory is the OCC using to store these buffer? Does it have
>> memory mapped onto the PowerBus into an MMIO space or something?
>>> other option ( I am not too happy about this ) is to embed the get/put
>>> memory protocol construction inside occ hwmon driver and make it call
>>> sbe fifo driver interface directly. The sbei protocol driver can then be
>>> in the user-space as a library and call the sbe fifo driver interface
>>> via ioctl. This creates duplication of the protocol driver code and we
>>> have to fix it in multiple places.
>>>>> If that's the case I thought we were already
>>>>> exposing scom chip-op operations via an OpenFSI master?
>>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
>>> openfsi driver is not exposing chipops. The sbe fifo driver must be
>>> calling the openfsi driver interfaces to write to the fifo. The openfsi
>>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
>>> operations via cronus, pdbg or rest api.
>>>
>>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
>>> sbe fifo driver also need to have both intra-kernel and user-space APIs
>>> if the sbei protocol driver goes to user-space.
>>>>> Regards,
>>>>>
>>>>> Alistair
>>>>>
>>>>>> thx - brad
>>>>>>
>>>>>>
>>>>>>
>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>>>>
>>>>>>> Hi Brad,
>>>>>>>
>>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>>>>
>>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>> With regards
>>>>>>> Venkatesh
>>>>>>>
>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>>>>>>
>>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>>>>
>>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>>>>>>
>>>>>>>>> Hi Brad,
>>>>>>>>>
>>>>>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>>>>
>>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>>>>>> single system call.
>>>>>>>>>>
>>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>>>>>> that (as you say) we can handle request and response in a single
>>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>>>>>> handled in userspace.
>>>>>>>>>
>>>>>>>>> Can you add a little description about the functionality we're exposing?
>>>>>>>>> That may suggest a particular API.
>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>>>>>>
>>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>>>>>>
>>>>>>>> For users there seems to be two classes:
>>>>>>>>
>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>>>>
>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>>>>>>
>>>>>>>>>> Would the in-kernel API be the same as the user space API?
>>>>>>>>> Probably not :)
>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>>>>>>
>>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>>>>> would probably not have a split read/write API or vise versa.
>>>>>>>>
>>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>>>>>>
>>>>>>>> sbe-uapi-fops(chardev)
>>>>>>>>    data = copy to/from user space;
>>>>>>>>    sbedev = from_chardev(chardev);
>>>>>>>>    kernel-api(sbedev, data);
>>>>>>>>
>>>>>>>> Or are the other things to consider here?
>>>>>>>>
>>>>>>>> -thx
>>>>>>>>
>>>>>>>> brad
>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Jeremy

Re: sbefifo userspace api

[Alistair Popple]

On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:

> In general, I agree with this approach to contain only those interfaces
> required by hwmon within the kernel and leave the rest to the
> user-space. But, we need to interlock with the hwmon to understand
> exactly which interfaces are required.
> IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b)
> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If
> these three are the right ones, then we need to contain them within the
> kernel.

Where are the OCC interfaces documented? We need to get a better
understanding of these.

Regardless I think you need a way for userspace to do chip-ops (for
debug/testing/development/etc). So I guess the question is then which
ones need a kernel implementation, and if there is a kernel
implementation should userspace use that or it's own implementation?

I think it's easiest to just have the userspace library contain all
the neccessary chip-ops and use those. Formatting of chip-ops seems
like it should be a trival amount of straight-forward code so the
minor duplication shouldn't be a concern (especially if we leave
framing up to the kernel). It's simpler than maintaining a long list
of ioctls which would still need a userspace library imho.

> >> I don't think it is a problem having get/putscom as an exceptional
> >> case for the reasons described above. My question about the
> >> consistency argument is what does it get us?
> >>
> >> I think Venkatesh was suggesting it would reduce duplication of
> >> protocol driver code, however perhaps this isn't the case. We could
> >> create a kernel chip-ops driver that deals with sending a buffer and
> >> getting a response but doesn't have to know anything about the chip-op
> >> itself. It could also implement the kernel sbe get/putscom.
> >>
> >> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
> >> of the sbe interface spec. Userspace would submit command-class,
> >> command-code & data words and the chip-ops driver would forward that
> >> to the SBE and send the response back to userspace without having to
> >> know what any of that data means.
> > I like it!
> >
> > Would this ‘chip-op’ driver be distinct from the sbefifo driver or
> > are you proposing a possible sbefifo driver API with a tad bit more
> > abstraction?
> >
> > If we did something like this, does a standalone sbescom driver still
> > have any value?
> I think the proposal is to have a combined sbefifo+sbechipop driver that
> provides a user space api to submit operations to sbefifo and user space
> api for those chipops contained within ( scom, memory and sram). The
> user space library will have the other sbe interfaces and will call the
> submit operations of the chip-op driver. Is this correct? We could get
> the user-space library provide a wrapper interface for even the chip-ops
> contained within the kernel so that the caller doesnt have to know
> whether to call the library or the driver apis.
> >> In any case I think you will end up with a userspace implementation of
> >> chip-ops anyway as it is much easier to test and develop new chip-ops
> >> without having to also understand how to build and flash a new kernel.
> >> Inevitably someone will want to add some kind of "debug" chip-op or
> >> other private chip-op that they don't want published in kernel code,
> >> and as soon as you have one userspace chip-op you may as well do all
> >> of the ones you can from there.
> >>
> >>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
> >>>>
> >>>>> There's the OCC hwmon driver - what chip-ops does that
> >>>>> need? Just get/putscom?
> >>>> Correct, just get/putscom.  But sbe scom, not direct scom.
> >>> occ hwmon driver should use get/put memory to communicate with OCC. One
> >> What memory is the OCC using to store these buffer? Does it have
> >> memory mapped onto the PowerBus into an MMIO space or something?
> >>> other option ( I am not too happy about this ) is to embed the get/put
> >>> memory protocol construction inside occ hwmon driver and make it call
> >>> sbe fifo driver interface directly. The sbei protocol driver can then be
> >>> in the user-space as a library and call the sbe fifo driver interface
> >>> via ioctl. This creates duplication of the protocol driver code and we
> >>> have to fix it in multiple places.
> >>>>> If that's the case I thought we were already
> >>>>> exposing scom chip-op operations via an OpenFSI master?
> >>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
> >>> openfsi driver is not exposing chipops. The sbe fifo driver must be
> >>> calling the openfsi driver interfaces to write to the fifo. The openfsi
> >>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
> >>> operations via cronus, pdbg or rest api.
> >>>
> >>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
> >>> sbe fifo driver also need to have both intra-kernel and user-space APIs
> >>> if the sbei protocol driver goes to user-space.
> >>>>> Regards,
> >>>>>
> >>>>> Alistair
> >>>>>
> >>>>>> thx - brad
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>>>>>>
> >>>>>>> Hi Brad,
> >>>>>>>
> >>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >>>>>>>
> >>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >>>>>>>
> >>>>>>> Thanks
> >>>>>>>
> >>>>>>> With regards
> >>>>>>> Venkatesh
> >>>>>>>
> >>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
> >>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>>>>>>>
> >>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>>>>>>>
> >>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>>>>>>>
> >>>>>>>>> Hi Brad,
> >>>>>>>>>
> >>>>>>>>>> Looking to start a discussion around possible user space and kernel
> >>>>>>>>>> APIs for the POWER9 sbefifo driver.
> >>>>>>>>>>
> >>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>>>>>>>> single system call.
> >>>>>>>>>>
> >>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
> >>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
> >>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
> >>>>>>>>> that (as you say) we can handle request and response in a single
> >>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
> >>>>>>>>> handled in userspace.
> >>>>>>>>>
> >>>>>>>>> Can you add a little description about the functionality we're exposing?
> >>>>>>>>> That may suggest a particular API.
> >>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>>>>>>>
> >>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>>>>>>>
> >>>>>>>> For users there seems to be two classes:
> >>>>>>>>
> >>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>>>>>>>
> >>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>>>>>>>
> >>>>>>>>>> Would the in-kernel API be the same as the user space API?
> >>>>>>>>> Probably not :)
> >>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>>>>>>>
> >>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >>>>>>>> would probably not have a split read/write API or vise versa.
> >>>>>>>>
> >>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>>>>>>>
> >>>>>>>> sbe-uapi-fops(chardev)
> >>>>>>>>    data = copy to/from user space;
> >>>>>>>>    sbedev = from_chardev(chardev);
> >>>>>>>>    kernel-api(sbedev, data);
> >>>>>>>>
> >>>>>>>> Or are the other things to consider here?
> >>>>>>>>
> >>>>>>>> -thx
> >>>>>>>>
> >>>>>>>> brad
> >>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Jeremy
>

Re: sbefifo userspace api

[Brad Bishop]

Based on what we’ve discussed here and a couple offline discussions,
I’m making some assertions below.  I’d like for this note to be a
“last call for discussion” (as much as that is possible) so we can
start hacking; please try and raise any concerns by sometime next week.

- sbefifo DD will implement read and write system calls.  A write 
will put data in the upstream fifo.  A read will pull data out of
the downstream fifo.

- sbefifo DD will _not_ do any framing of data flowing into/out of the
kernel (in other words, exactly what is found in the sbei interface spec).

- sbefifo will provide an in-kernel API which again, will not do any
framing of data.

- occ-hwmon DD will assemble requests and parse responses for whatever
chip-ops it requires and use the sbefifo in-kernel api to access the sbe.

- any userspace implementation of any part of the sbei spec will
use the sbefifo uapi.

Below are not assertions but relevant items yet to be resolved, and
should not prevent moving forward with implementation of the above.

- Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
assemble requests, parse responses for whatever chip-ops it requires
and use the sbefifo in-kernel api in the same manner as the occ-hwmon DD.
imho, this feels like convenience code since we have a real hw scom
CFAM engine with UAPI.  A wrapper that hides this distinction could
just as easily be done in userspace without another DD.

Thanks to everyone participating in the discussion.

-brad

> On Feb 22, 2017, at 12:59 PM, Alistair Popple <alistair@popple.id.au> wrote:
> 
> On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
> 
>> In general, I agree with this approach to contain only those interfaces
>> required by hwmon within the kernel and leave the rest to the
>> user-space. But, we need to interlock with the hwmon to understand
>> exactly which interfaces are required.
>> IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b)
>> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If
>> these three are the right ones, then we need to contain them within the
>> kernel.
> 
> Where are the OCC interfaces documented? We need to get a better
> understanding of these.
> 
> Regardless I think you need a way for userspace to do chip-ops (for
> debug/testing/development/etc). So I guess the question is then which
> ones need a kernel implementation, and if there is a kernel
> implementation should userspace use that or it's own implementation?
> 
> I think it's easiest to just have the userspace library contain all
> the neccessary chip-ops and use those. Formatting of chip-ops seems
> like it should be a trival amount of straight-forward code so the
> minor duplication shouldn't be a concern (especially if we leave
> framing up to the kernel). It's simpler than maintaining a long list
> of ioctls which would still need a userspace library imho.
> 
>>>> I don't think it is a problem having get/putscom as an exceptional
>>>> case for the reasons described above. My question about the
>>>> consistency argument is what does it get us?
>>>> 
>>>> I think Venkatesh was suggesting it would reduce duplication of
>>>> protocol driver code, however perhaps this isn't the case. We could
>>>> create a kernel chip-ops driver that deals with sending a buffer and
>>>> getting a response but doesn't have to know anything about the chip-op
>>>> itself. It could also implement the kernel sbe get/putscom.
>>>> 
>>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
>>>> of the sbe interface spec. Userspace would submit command-class,
>>>> command-code & data words and the chip-ops driver would forward that
>>>> to the SBE and send the response back to userspace without having to
>>>> know what any of that data means.
>>> I like it!
>>> 
>>> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
>>> are you proposing a possible sbefifo driver API with a tad bit more
>>> abstraction?
>>> 
>>> If we did something like this, does a standalone sbescom driver still
>>> have any value?
>> I think the proposal is to have a combined sbefifo+sbechipop driver that
>> provides a user space api to submit operations to sbefifo and user space
>> api for those chipops contained within ( scom, memory and sram). The
>> user space library will have the other sbe interfaces and will call the
>> submit operations of the chip-op driver. Is this correct? We could get
>> the user-space library provide a wrapper interface for even the chip-ops
>> contained within the kernel so that the caller doesnt have to know
>> whether to call the library or the driver apis.
>>>> In any case I think you will end up with a userspace implementation of
>>>> chip-ops anyway as it is much easier to test and develop new chip-ops
>>>> without having to also understand how to build and flash a new kernel.
>>>> Inevitably someone will want to add some kind of "debug" chip-op or
>>>> other private chip-op that they don't want published in kernel code,
>>>> and as soon as you have one userspace chip-op you may as well do all
>>>> of the ones you can from there.
>>>> 
>>>>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
>>>>>> 
>>>>>>> There's the OCC hwmon driver - what chip-ops does that
>>>>>>> need? Just get/putscom?
>>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>>>>> occ hwmon driver should use get/put memory to communicate with OCC. One
>>>> What memory is the OCC using to store these buffer? Does it have
>>>> memory mapped onto the PowerBus into an MMIO space or something?
>>>>> other option ( I am not too happy about this ) is to embed the get/put
>>>>> memory protocol construction inside occ hwmon driver and make it call
>>>>> sbe fifo driver interface directly. The sbei protocol driver can then be
>>>>> in the user-space as a library and call the sbe fifo driver interface
>>>>> via ioctl. This creates duplication of the protocol driver code and we
>>>>> have to fix it in multiple places.
>>>>>>> If that's the case I thought we were already
>>>>>>> exposing scom chip-op operations via an OpenFSI master?
>>>>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
>>>>> openfsi driver is not exposing chipops. The sbe fifo driver must be
>>>>> calling the openfsi driver interfaces to write to the fifo. The openfsi
>>>>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
>>>>> operations via cronus, pdbg or rest api.
>>>>> 
>>>>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
>>>>> sbe fifo driver also need to have both intra-kernel and user-space APIs
>>>>> if the sbei protocol driver goes to user-space.
>>>>>>> Regards,
>>>>>>> 
>>>>>>> Alistair
>>>>>>> 
>>>>>>>> thx - brad
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>>>>>> 
>>>>>>>>> Hi Brad,
>>>>>>>>> 
>>>>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>>>>>> 
>>>>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>>>>>>>> 
>>>>>>>>> Thanks
>>>>>>>>> 
>>>>>>>>> With regards
>>>>>>>>> Venkatesh
>>>>>>>>> 
>>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>>>>>>>> 
>>>>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>>>>>> 
>>>>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>>>>>>>> 
>>>>>>>>>>> Hi Brad,
>>>>>>>>>>> 
>>>>>>>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>>>>>> 
>>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>>>>>>>> single system call.
>>>>>>>>>>>> 
>>>>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>>>>>>>> that (as you say) we can handle request and response in a single
>>>>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>>>>>>>> handled in userspace.
>>>>>>>>>>> 
>>>>>>>>>>> Can you add a little description about the functionality we're exposing?
>>>>>>>>>>> That may suggest a particular API.
>>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>>>>>>>> 
>>>>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>>>>>>>> 
>>>>>>>>>> For users there seems to be two classes:
>>>>>>>>>> 
>>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>>>>>> 
>>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>>>>>>>> 
>>>>>>>>>>>> Would the in-kernel API be the same as the user space API?
>>>>>>>>>>> Probably not :)
>>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>>>>>>>> 
>>>>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>>>>>>> would probably not have a split read/write API or vise versa.
>>>>>>>>>> 
>>>>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>>>>>>>> 
>>>>>>>>>> sbe-uapi-fops(chardev)
>>>>>>>>>>   data = copy to/from user space;
>>>>>>>>>>   sbedev = from_chardev(chardev);
>>>>>>>>>>   kernel-api(sbedev, data);
>>>>>>>>>> 
>>>>>>>>>> Or are the other things to consider here?
>>>>>>>>>> 
>>>>>>>>>> -thx
>>>>>>>>>> 
>>>>>>>>>> brad
>>>>>>>>>> 
>>>>>>>>>>> Cheers,
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> Jeremy
>> 

Re: sbefifo userspace api

[Alistair Popple]

On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
> Based on what we’ve discussed here and a couple offline discussions,
> I’m making some assertions below.  I’d like for this note to be a
> “last call for discussion” (as much as that is possible) so we can
> start hacking; please try and raise any concerns by sometime next week.
>
> - sbefifo DD will implement read and write system calls.  A write
> will put data in the upstream fifo.  A read will pull data out of
> the downstream fifo.
>
> - sbefifo DD will _not_ do any framing of data flowing into/out of the
> kernel (in other words, exactly what is found in the sbei interface spec).
>
> - sbefifo will provide an in-kernel API which again, will not do any
> framing of data.
>
> - occ-hwmon DD will assemble requests and parse responses for whatever
> chip-ops it requires and use the sbefifo in-kernel api to access the sbe.
>
> - any userspace implementation of any part of the sbei spec will
> use the sbefifo uapi.
>
> Below are not assertions but relevant items yet to be resolved, and
> should not prevent moving forward with implementation of the above.
>
> - Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
> assemble requests, parse responses for whatever chip-ops it requires
> and use the sbefifo in-kernel api in the same manner as the occ-hwmon DD.
> imho, this feels like convenience code since we have a real hw scom
> CFAM engine with UAPI.  A wrapper that hides this distinction could
> just as easily be done in userspace without another DD.

Thanks for taking the time to write this up! I don't have any concerns
with the above summary. I agree the sbescom driver seems unneccesary
as whether to use chip-ops or real HW scom is something that userspace
(rather than the kernel) should decide anyway, so I'm not sure what
value having that wrapper in the kernel would have.

- Alistair

> Thanks to everyone participating in the discussion.
>
> -brad
>
> > On Feb 22, 2017, at 12:59 PM, Alistair Popple <alistair@popple.id.au> wrote:
> >
> > On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
> >
> >> In general, I agree with this approach to contain only those interfaces
> >> required by hwmon within the kernel and leave the rest to the
> >> user-space. But, we need to interlock with the hwmon to understand
> >> exactly which interfaces are required.
> >> IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b)
> >> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If
> >> these three are the right ones, then we need to contain them within the
> >> kernel.
> >
> > Where are the OCC interfaces documented? We need to get a better
> > understanding of these.
> >
> > Regardless I think you need a way for userspace to do chip-ops (for
> > debug/testing/development/etc). So I guess the question is then which
> > ones need a kernel implementation, and if there is a kernel
> > implementation should userspace use that or it's own implementation?
> >
> > I think it's easiest to just have the userspace library contain all
> > the neccessary chip-ops and use those. Formatting of chip-ops seems
> > like it should be a trival amount of straight-forward code so the
> > minor duplication shouldn't be a concern (especially if we leave
> > framing up to the kernel). It's simpler than maintaining a long list
> > of ioctls which would still need a userspace library imho.
> >
> >>>> I don't think it is a problem having get/putscom as an exceptional
> >>>> case for the reasons described above. My question about the
> >>>> consistency argument is what does it get us?
> >>>>
> >>>> I think Venkatesh was suggesting it would reduce duplication of
> >>>> protocol driver code, however perhaps this isn't the case. We could
> >>>> create a kernel chip-ops driver that deals with sending a buffer and
> >>>> getting a response but doesn't have to know anything about the chip-op
> >>>> itself. It could also implement the kernel sbe get/putscom.
> >>>>
> >>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
> >>>> of the sbe interface spec. Userspace would submit command-class,
> >>>> command-code & data words and the chip-ops driver would forward that
> >>>> to the SBE and send the response back to userspace without having to
> >>>> know what any of that data means.
> >>> I like it!
> >>>
> >>> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
> >>> are you proposing a possible sbefifo driver API with a tad bit more
> >>> abstraction?
> >>>
> >>> If we did something like this, does a standalone sbescom driver still
> >>> have any value?
> >> I think the proposal is to have a combined sbefifo+sbechipop driver that
> >> provides a user space api to submit operations to sbefifo and user space
> >> api for those chipops contained within ( scom, memory and sram). The
> >> user space library will have the other sbe interfaces and will call the
> >> submit operations of the chip-op driver. Is this correct? We could get
> >> the user-space library provide a wrapper interface for even the chip-ops
> >> contained within the kernel so that the caller doesnt have to know
> >> whether to call the library or the driver apis.
> >>>> In any case I think you will end up with a userspace implementation of
> >>>> chip-ops anyway as it is much easier to test and develop new chip-ops
> >>>> without having to also understand how to build and flash a new kernel.
> >>>> Inevitably someone will want to add some kind of "debug" chip-op or
> >>>> other private chip-op that they don't want published in kernel code,
> >>>> and as soon as you have one userspace chip-op you may as well do all
> >>>> of the ones you can from there.
> >>>>
> >>>>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
> >>>>>>
> >>>>>>> There's the OCC hwmon driver - what chip-ops does that
> >>>>>>> need? Just get/putscom?
> >>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
> >>>>> occ hwmon driver should use get/put memory to communicate with OCC. One
> >>>> What memory is the OCC using to store these buffer? Does it have
> >>>> memory mapped onto the PowerBus into an MMIO space or something?
> >>>>> other option ( I am not too happy about this ) is to embed the get/put
> >>>>> memory protocol construction inside occ hwmon driver and make it call
> >>>>> sbe fifo driver interface directly. The sbei protocol driver can then be
> >>>>> in the user-space as a library and call the sbe fifo driver interface
> >>>>> via ioctl. This creates duplication of the protocol driver code and we
> >>>>> have to fix it in multiple places.
> >>>>>>> If that's the case I thought we were already
> >>>>>>> exposing scom chip-op operations via an OpenFSI master?
> >>>>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
> >>>>> openfsi driver is not exposing chipops. The sbe fifo driver must be
> >>>>> calling the openfsi driver interfaces to write to the fifo. The openfsi
> >>>>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
> >>>>> operations via cronus, pdbg or rest api.
> >>>>>
> >>>>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
> >>>>> sbe fifo driver also need to have both intra-kernel and user-space APIs
> >>>>> if the sbei protocol driver goes to user-space.
> >>>>>>> Regards,
> >>>>>>>
> >>>>>>> Alistair
> >>>>>>>
> >>>>>>>> thx - brad
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>>>>>>>>
> >>>>>>>>> Hi Brad,
> >>>>>>>>>
> >>>>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >>>>>>>>>
> >>>>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >>>>>>>>>
> >>>>>>>>> Thanks
> >>>>>>>>>
> >>>>>>>>> With regards
> >>>>>>>>> Venkatesh
> >>>>>>>>>
> >>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
> >>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>>>>>>>>>
> >>>>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>>>>>>>>>
> >>>>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>>>>>>>>>
> >>>>>>>>>>> Hi Brad,
> >>>>>>>>>>>
> >>>>>>>>>>>> Looking to start a discussion around possible user space and kernel
> >>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
> >>>>>>>>>>>>
> >>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>>>>>>>>>> single system call.
> >>>>>>>>>>>>
> >>>>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
> >>>>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
> >>>>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
> >>>>>>>>>>> that (as you say) we can handle request and response in a single
> >>>>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
> >>>>>>>>>>> handled in userspace.
> >>>>>>>>>>>
> >>>>>>>>>>> Can you add a little description about the functionality we're exposing?
> >>>>>>>>>>> That may suggest a particular API.
> >>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>>>>>>>>>
> >>>>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>>>>>>>>>
> >>>>>>>>>> For users there seems to be two classes:
> >>>>>>>>>>
> >>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>>>>>>>>>
> >>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>>>>>>>>>
> >>>>>>>>>>>> Would the in-kernel API be the same as the user space API?
> >>>>>>>>>>> Probably not :)
> >>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>>>>>>>>>
> >>>>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >>>>>>>>>> would probably not have a split read/write API or vise versa.
> >>>>>>>>>>
> >>>>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>>>>>>>>>
> >>>>>>>>>> sbe-uapi-fops(chardev)
> >>>>>>>>>>   data = copy to/from user space;
> >>>>>>>>>>   sbedev = from_chardev(chardev);
> >>>>>>>>>>   kernel-api(sbedev, data);
> >>>>>>>>>>
> >>>>>>>>>> Or are the other things to consider here?
> >>>>>>>>>>
> >>>>>>>>>> -thx
> >>>>>>>>>>
> >>>>>>>>>> brad
> >>>>>>>>>>
> >>>>>>>>>>> Cheers,
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Jeremy
> >>

Re: sbefifo userspace api

[Venkatesh Sainath]

On 24/02/17 11:44 AM, Alistair Popple wrote:
> On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
>> Based on what we’ve discussed here and a couple offline discussions,
>> I’m making some assertions below.  I’d like for this note to be a
>> “last call for discussion” (as much as that is possible) so we can
>> start hacking; please try and raise any concerns by sometime next week.
>>
>> - sbefifo DD will implement read and write system calls.  A write
>> will put data in the upstream fifo.  A read will pull data out of
>> the downstream fifo.
Brad,
  Thanks for summarising the discussion. Instead of independent 
read/write calls, I was looking for a single ioctl(submit) operation 
which would always do a write of the incoming data packet to the 
upstream fifo followed by a read of a byte stream from the downstream 
fifo. There can be multiple users who open file descriptors to the sbe 
fifo in write mode. However, each ioctl(fd,submit,data) operation will 
be protected such that no two submit operations will happen in parallel. 
This way (a) the users dont have to know the semantics of the sbe fifo 
status bits ( data valid / end-of-transmission) and (b) there wont be 
any collision of write and read operations between two users. I am 
hoping this does not violate any linux DD norms.

-Venkatesh
>> - sbefifo DD will _not_ do any framing of data flowing into/out of the
>> kernel (in other words, exactly what is found in the sbei interface spec).
Agreed.
>> - sbefifo will provide an in-kernel API which again, will not do any
>> framing of data.
Agreed.
>> - occ-hwmon DD will assemble requests and parse responses for whatever
>> chip-ops it requires and use the sbefifo in-kernel api to access the sbe.
Agreed.
>> - any userspace implementation of any part of the sbei spec will
>> use the sbefifo uapi.
Agreed.
>> Below are not assertions but relevant items yet to be resolved, and
>> should not prevent moving forward with implementation of the above.
>>
>> - Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
>> assemble requests, parse responses for whatever chip-ops it requires
>> and use the sbefifo in-kernel api in the same manner as the occ-hwmon DD.
>> imho, this feels like convenience code since we have a real hw scom
>> CFAM engine with UAPI.  A wrapper that hides this distinction could
>> just as easily be done in userspace without another DD.
I dont see a need for sbescom DD. A wrapper to the user-space sbe 
protocol driver lib can handle sbe scom vs fsi scom. For all practical 
purposes, we should ask users to use the sbe path only.
> Thanks for taking the time to write this up! I don't have any concerns
> with the above summary. I agree the sbescom driver seems unneccesary
> as whether to use chip-ops or real HW scom is something that userspace
> (rather than the kernel) should decide anyway, so I'm not sure what
> value having that wrapper in the kernel would have.
>
> - Alistair
>
>> Thanks to everyone participating in the discussion.
>>
>> -brad
>>
>>> On Feb 22, 2017, at 12:59 PM, Alistair Popple <alistair@popple.id.au> wrote:
>>>
>>> On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
>>>
>>>> In general, I agree with this approach to contain only those interfaces
>>>> required by hwmon within the kernel and leave the rest to the
>>>> user-space. But, we need to interlock with the hwmon to understand
>>>> exactly which interfaces are required.
>>>> IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b)
>>>> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If
>>>> these three are the right ones, then we need to contain them within the
>>>> kernel.
>>> Where are the OCC interfaces documented? We need to get a better
>>> understanding of these.
>>>
>>> Regardless I think you need a way for userspace to do chip-ops (for
>>> debug/testing/development/etc). So I guess the question is then which
>>> ones need a kernel implementation, and if there is a kernel
>>> implementation should userspace use that or it's own implementation?
>>>
>>> I think it's easiest to just have the userspace library contain all
>>> the neccessary chip-ops and use those. Formatting of chip-ops seems
>>> like it should be a trival amount of straight-forward code so the
>>> minor duplication shouldn't be a concern (especially if we leave
>>> framing up to the kernel). It's simpler than maintaining a long list
>>> of ioctls which would still need a userspace library imho.
>>>
>>>>>> I don't think it is a problem having get/putscom as an exceptional
>>>>>> case for the reasons described above. My question about the
>>>>>> consistency argument is what does it get us?
>>>>>>
>>>>>> I think Venkatesh was suggesting it would reduce duplication of
>>>>>> protocol driver code, however perhaps this isn't the case. We could
>>>>>> create a kernel chip-ops driver that deals with sending a buffer and
>>>>>> getting a response but doesn't have to know anything about the chip-op
>>>>>> itself. It could also implement the kernel sbe get/putscom.
>>>>>>
>>>>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
>>>>>> of the sbe interface spec. Userspace would submit command-class,
>>>>>> command-code & data words and the chip-ops driver would forward that
>>>>>> to the SBE and send the response back to userspace without having to
>>>>>> know what any of that data means.
>>>>> I like it!
>>>>>
>>>>> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
>>>>> are you proposing a possible sbefifo driver API with a tad bit more
>>>>> abstraction?
>>>>>
>>>>> If we did something like this, does a standalone sbescom driver still
>>>>> have any value?
>>>> I think the proposal is to have a combined sbefifo+sbechipop driver that
>>>> provides a user space api to submit operations to sbefifo and user space
>>>> api for those chipops contained within ( scom, memory and sram). The
>>>> user space library will have the other sbe interfaces and will call the
>>>> submit operations of the chip-op driver. Is this correct? We could get
>>>> the user-space library provide a wrapper interface for even the chip-ops
>>>> contained within the kernel so that the caller doesnt have to know
>>>> whether to call the library or the driver apis.
>>>>>> In any case I think you will end up with a userspace implementation of
>>>>>> chip-ops anyway as it is much easier to test and develop new chip-ops
>>>>>> without having to also understand how to build and flash a new kernel.
>>>>>> Inevitably someone will want to add some kind of "debug" chip-op or
>>>>>> other private chip-op that they don't want published in kernel code,
>>>>>> and as soon as you have one userspace chip-op you may as well do all
>>>>>> of the ones you can from there.
>>>>>>
>>>>>>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
>>>>>>>>
>>>>>>>>> There's the OCC hwmon driver - what chip-ops does that
>>>>>>>>> need? Just get/putscom?
>>>>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>>>>>>> occ hwmon driver should use get/put memory to communicate with OCC. One
>>>>>> What memory is the OCC using to store these buffer? Does it have
>>>>>> memory mapped onto the PowerBus into an MMIO space or something?
>>>>>>> other option ( I am not too happy about this ) is to embed the get/put
>>>>>>> memory protocol construction inside occ hwmon driver and make it call
>>>>>>> sbe fifo driver interface directly. The sbei protocol driver can then be
>>>>>>> in the user-space as a library and call the sbe fifo driver interface
>>>>>>> via ioctl. This creates duplication of the protocol driver code and we
>>>>>>> have to fix it in multiple places.
>>>>>>>>> If that's the case I thought we were already
>>>>>>>>> exposing scom chip-op operations via an OpenFSI master?
>>>>>>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
>>>>>>> openfsi driver is not exposing chipops. The sbe fifo driver must be
>>>>>>> calling the openfsi driver interfaces to write to the fifo. The openfsi
>>>>>>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
>>>>>>> operations via cronus, pdbg or rest api.
>>>>>>>
>>>>>>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
>>>>>>> sbe fifo driver also need to have both intra-kernel and user-space APIs
>>>>>>> if the sbei protocol driver goes to user-space.
>>>>>>>>> Regards,
>>>>>>>>>
>>>>>>>>> Alistair
>>>>>>>>>
>>>>>>>>>> thx - brad
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>>>>>>>>
>>>>>>>>>>> Hi Brad,
>>>>>>>>>>>
>>>>>>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>>>>>>>>
>>>>>>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
>>>>>>>>>>>
>>>>>>>>>>> Thanks
>>>>>>>>>>>
>>>>>>>>>>> With regards
>>>>>>>>>>> Venkatesh
>>>>>>>>>>>
>>>>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
>>>>>>>>>>>>
>>>>>>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>>>>>>>>
>>>>>>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Brad,
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Looking to start a discussion around possible user space and kernel
>>>>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
>>>>>>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
>>>>>>>>>>>>>> single system call.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
>>>>>>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
>>>>>>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
>>>>>>>>>>>>> that (as you say) we can handle request and response in a single
>>>>>>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
>>>>>>>>>>>>> handled in userspace.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Can you add a little description about the functionality we're exposing?
>>>>>>>>>>>>> That may suggest a particular API.
>>>>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
>>>>>>>>>>>>
>>>>>>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
>>>>>>>>>>>>
>>>>>>>>>>>> For users there seems to be two classes:
>>>>>>>>>>>>
>>>>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>>>>>>>>
>>>>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
>>>>>>>>>>>>
>>>>>>>>>>>>>> Would the in-kernel API be the same as the user space API?
>>>>>>>>>>>>> Probably not :)
>>>>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
>>>>>>>>>>>>
>>>>>>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>>>>>>>>> would probably not have a split read/write API or vise versa.
>>>>>>>>>>>>
>>>>>>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
>>>>>>>>>>>>
>>>>>>>>>>>> sbe-uapi-fops(chardev)
>>>>>>>>>>>>    data = copy to/from user space;
>>>>>>>>>>>>    sbedev = from_chardev(chardev);
>>>>>>>>>>>>    kernel-api(sbedev, data);
>>>>>>>>>>>>
>>>>>>>>>>>> Or are the other things to consider here?
>>>>>>>>>>>>
>>>>>>>>>>>> -thx
>>>>>>>>>>>>
>>>>>>>>>>>> brad
>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>> Jeremy

Re: sbefifo userspace api

[Brad Bishop]

> On Feb 25, 2017, at 12:14 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> 
> 
> 
> On 24/02/17 11:44 AM, Alistair Popple wrote:
>> On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
>>> Based on what we’ve discussed here and a couple offline discussions,
>>> I’m making some assertions below.  I’d like for this note to be a
>>> “last call for discussion” (as much as that is possible) so we can
>>> start hacking; please try and raise any concerns by sometime next week.
>>> 
>>> - sbefifo DD will implement read and write system calls.  A write
>>> will put data in the upstream fifo.  A read will pull data out of
>>> the downstream fifo.
> Brad,
> Thanks for summarising the discussion. Instead of independent read/write calls, I was looking for a single ioctl(submit) operation which would always do a write of the incoming data packet to the upstream fifo followed by a read of a byte stream from the downstream fifo. There can be multiple users who open file descriptors to the sbe fifo in write mode. However, each ioctl(fd,submit,data) operation will be protected such that no two submit operations will happen in parallel.

The driver can/will still provide this protection, even with a read/write
interface.

> This way (a) the users dont have to know the semantics of the sbe fifo status bits ( data valid / end-of-transmission)

I don’t think a read/write interface necessitates this.

> and (b) there wont be any collision of write and read operations between two users.

If a user writes to an fd, the sbe response will only come out with
a read on that same fd.  If some other fd is read from (before or after
read is called on the first fd), that read is just going to block (until
_that_ fd is written to).

> I am hoping this does not violate any linux DD norms.

I’ve been told by multiple folks that read/write is the “interface of least
surprise” in the general Linux sense.  Given that, I chose read/write over
an ioctl to maximize our chances of getting this driver incorporated into
upstream Linux.

A wrapper to get a submit-like interface could look like this:

int submit(const char* in, size_t len, char** out)
{
    char buf[1024];
    ssize_t read = 0;

    int fd = open(“/dev/sbefifo”);
    int rc = write(fd, in, len);
    
    while((rc = read(fd, &buf, sizeof(buf))) > 0)
    {
       read += rc;
       /* copy out… */
    }

    return read;
}

That should be workable, right?

> 
> -Venkatesh

-brad

Re: sbefifo userspace api

[Venkatesh Sainath]

On 25/02/17 11:13 AM, Brad Bishop wrote:
>> On Feb 25, 2017, at 12:14 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>
>>
>>
>> On 24/02/17 11:44 AM, Alistair Popple wrote:
>>> On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
>>>> Based on what we’ve discussed here and a couple offline discussions,
>>>> I’m making some assertions below.  I’d like for this note to be a
>>>> “last call for discussion” (as much as that is possible) so we can
>>>> start hacking; please try and raise any concerns by sometime next week.
>>>>
>>>> - sbefifo DD will implement read and write system calls.  A write
>>>> will put data in the upstream fifo.  A read will pull data out of
>>>> the downstream fifo.
>> Brad,
>> Thanks for summarising the discussion. Instead of independent read/write calls, I was looking for a single ioctl(submit) operation which would always do a write of the incoming data packet to the upstream fifo followed by a read of a byte stream from the downstream fifo. There can be multiple users who open file descriptors to the sbe fifo in write mode. However, each ioctl(fd,submit,data) operation will be protected such that no two submit operations will happen in parallel.
> The driver can/will still provide this protection, even with a read/write
> interface.
>
>> This way (a) the users dont have to know the semantics of the sbe fifo status bits ( data valid / end-of-transmission)
> I don’t think a read/write interface necessitates this.
>
>> and (b) there wont be any collision of write and read operations between two users.
> If a user writes to an fd, the sbe response will only come out with
> a read on that same fd.  If some other fd is read from (before or after
> read is called on the first fd), that read is just going to block (until
> _that_ fd is written to).
>
>> I am hoping this does not violate any linux DD norms.
> I’ve been told by multiple folks that read/write is the “interface of least
> surprise” in the general Linux sense.  Given that, I chose read/write over
> an ioctl to maximize our chances of getting this driver incorporated into
> upstream Linux.
>
> A wrapper to get a submit-like interface could look like this:
>
> int submit(const char* in, size_t len, char** out)
> {
>      char buf[1024];
>      ssize_t read = 0;
>
>      int fd = open(“/dev/sbefifo”);
>      int rc = write(fd, in, len);
>      
>      while((rc = read(fd, &buf, sizeof(buf))) > 0)
>      {
>         read += rc;
>         /* copy out… */
>      }
>
>      return read;
> }
>
> That should be workable, right?
Agreed. Also, a write(fd1), write(fd2), read(fd1) should not be allowed 
as it can confuse the SBE which is waiting for its downstream fifo to be 
pulled by BMC and it receives more data on its upstream fifo. A write on 
a given fd should always be followed by a read on the same fd. Two 
writes or two reads should not be followed in sequence. Two back-to-back 
reads can result in sbefifo DD experiencing a false sbe response timeout 
as there is nothing to be sent by SBE and sbefifo DD is waiting on the 
downstream fifo.
>> -Venkatesh
> -brad

Re: sbefifo userspace api

[Alistair Popple]

Hi,

On Sat, 25 Feb 2017 11:22:00 AM Venkatesh Sainath wrote:
>
> On 25/02/17 11:13 AM, Brad Bishop wrote:
> >> On Feb 25, 2017, at 12:14 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>
> >>
> >>
> >> On 24/02/17 11:44 AM, Alistair Popple wrote:
> >>> On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
> >>>> Based on what we’ve discussed here and a couple offline discussions,
> >>>> I’m making some assertions below.  I’d like for this note to be a
> >>>> “last call for discussion” (as much as that is possible) so we can
> >>>> start hacking; please try and raise any concerns by sometime next week.
> >>>>
> >>>> - sbefifo DD will implement read and write system calls.  A write
> >>>> will put data in the upstream fifo.  A read will pull data out of
> >>>> the downstream fifo.
> >> Brad,
> >> Thanks for summarising the discussion. Instead of independent read/write calls, I was looking for a single ioctl(submit) operation which would always do a write of the incoming data packet to the upstream fifo followed by a read of a byte stream from the downstream fifo. There can be multiple users who open file descriptors to the sbe fifo in write mode. However, each ioctl(fd,submit,data) operation will be protected such that no two submit operations will happen in parallel.
> > The driver can/will still provide this protection, even with a read/write
> > interface.
> >
> >> This way (a) the users dont have to know the semantics of the sbe fifo status bits ( data valid / end-of-transmission)
> > I don’t think a read/write interface necessitates this.
> >
> >> and (b) there wont be any collision of write and read operations between two users.
> > If a user writes to an fd, the sbe response will only come out with
> > a read on that same fd.  If some other fd is read from (before or after
> > read is called on the first fd), that read is just going to block (until
> > _that_ fd is written to).
> >
> >> I am hoping this does not violate any linux DD norms.
> > I’ve been told by multiple folks that read/write is the “interface of least
> > surprise” in the general Linux sense.  Given that, I chose read/write over
> > an ioctl to maximize our chances of getting this driver incorporated into
> > upstream Linux.
> >
> > A wrapper to get a submit-like interface could look like this:
> >
> > int submit(const char* in, size_t len, char** out)
> > {
> >      char buf[1024];
> >      ssize_t read = 0;
> >
> >      int fd = open(“/dev/sbefifo”);
> >      int rc = write(fd, in, len);
> >
> >      while((rc = read(fd, &buf, sizeof(buf))) > 0)
> >      {
> >         read += rc;
> >         /* copy out… */
> >      }
> >
> >      return read;
> > }
> >
> > That should be workable, right?
> Agreed. Also, a write(fd1), write(fd2), read(fd1) should not be allowed
> as it can confuse the SBE which is waiting for its downstream fifo to be
> pulled by BMC and it receives more data on its upstream fifo. A write on
> a given fd should always be followed by a read on the same fd. Two
> writes or two reads should not be followed in sequence. Two back-to-back
> reads can result in sbefifo DD experiencing a false sbe response timeout
> as there is nothing to be sent by SBE and sbefifo DD is waiting on the
> downstream fifo.

I think you are confusing the read/write system calls with the
read/write calls to the fifo. The kernel driver is responsible for
ensuring a sequence of write(fd1), write(fd2), read(fd1) syscalls
results in a sequence of write(fd1), read(fd1), write(fd2) calls to
the SBE FIFO.

- Alistair

> >> -Venkatesh
> > -brad
>

Re: sbefifo userspace api

[Venkatesh Sainath]

On 25/02/17 11:26 AM, Alistair Popple wrote:
> Hi,
>
> On Sat, 25 Feb 2017 11:22:00 AM Venkatesh Sainath wrote:
>> On 25/02/17 11:13 AM, Brad Bishop wrote:
>>>> On Feb 25, 2017, at 12:14 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
>>>>
>>>>
>>>>
>>>> On 24/02/17 11:44 AM, Alistair Popple wrote:
>>>>> On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
>>>>>> Based on what we’ve discussed here and a couple offline discussions,
>>>>>> I’m making some assertions below.  I’d like for this note to be a
>>>>>> “last call for discussion” (as much as that is possible) so we can
>>>>>> start hacking; please try and raise any concerns by sometime next week.
>>>>>>
>>>>>> - sbefifo DD will implement read and write system calls.  A write
>>>>>> will put data in the upstream fifo.  A read will pull data out of
>>>>>> the downstream fifo.
>>>> Brad,
>>>> Thanks for summarising the discussion. Instead of independent read/write calls, I was looking for a single ioctl(submit) operation which would always do a write of the incoming data packet to the upstream fifo followed by a read of a byte stream from the downstream fifo. There can be multiple users who open file descriptors to the sbe fifo in write mode. However, each ioctl(fd,submit,data) operation will be protected such that no two submit operations will happen in parallel.
>>> The driver can/will still provide this protection, even with a read/write
>>> interface.
>>>
>>>> This way (a) the users dont have to know the semantics of the sbe fifo status bits ( data valid / end-of-transmission)
>>> I don’t think a read/write interface necessitates this.
>>>
>>>> and (b) there wont be any collision of write and read operations between two users.
>>> If a user writes to an fd, the sbe response will only come out with
>>> a read on that same fd.  If some other fd is read from (before or after
>>> read is called on the first fd), that read is just going to block (until
>>> _that_ fd is written to).
>>>
>>>> I am hoping this does not violate any linux DD norms.
>>> I’ve been told by multiple folks that read/write is the “interface of least
>>> surprise” in the general Linux sense.  Given that, I chose read/write over
>>> an ioctl to maximize our chances of getting this driver incorporated into
>>> upstream Linux.
>>>
>>> A wrapper to get a submit-like interface could look like this:
>>>
>>> int submit(const char* in, size_t len, char** out)
>>> {
>>>       char buf[1024];
>>>       ssize_t read = 0;
>>>
>>>       int fd = open(“/dev/sbefifo”);
>>>       int rc = write(fd, in, len);
>>>
>>>       while((rc = read(fd, &buf, sizeof(buf))) > 0)
>>>       {
>>>          read += rc;
>>>          /* copy out… */
>>>       }
>>>
>>>       return read;
>>> }
>>>
>>> That should be workable, right?
>> Agreed. Also, a write(fd1), write(fd2), read(fd1) should not be allowed
>> as it can confuse the SBE which is waiting for its downstream fifo to be
>> pulled by BMC and it receives more data on its upstream fifo. A write on
>> a given fd should always be followed by a read on the same fd. Two
>> writes or two reads should not be followed in sequence. Two back-to-back
>> reads can result in sbefifo DD experiencing a false sbe response timeout
>> as there is nothing to be sent by SBE and sbefifo DD is waiting on the
>> downstream fifo.
> I think you are confusing the read/write system calls with the
> read/write calls to the fifo. The kernel driver is responsible for
> ensuring a sequence of write(fd1), write(fd2), read(fd1) syscalls
> results in a sequence of write(fd1), read(fd1), write(fd2) calls to
> the SBE FIFO.
Thanks for correcting me :-) Also, the kernel driver should return error 
if a read(fd) is done without a preceding write(fd). This error must be 
returned by kernel driver before touching the fifo itself. One more 
use-case to cover --- If a read(fd) is done with a not-so-large-enough 
buffer, then kernel driver should flush the downstream fifo until it is 
empty and return buffer-too-small error to the caller. User cannot 
allocate a bigger buffer and retry read(fd) as it will end up in two 
consecutive reads without a write.

-Venkatesh
> - Alistair
>
>>>> -Venkatesh
>>> -brad

Re: sbefifo userspace api

[Brad Bishop]

> Thanks for correcting me :-) Also, the kernel driver should return error if a read(fd) is done without a preceding write(fd).
> This error must be returned by kernel driver before touching the fifo itself.

Again, it is the job of the driver to make sure that the fifo is ‘touched’ in the correct way.

The read system call will likely never touch the fifo in any case.

Consider this sample implementation of the read system call:

size_t sbefifo_read(fd, buf)
{
    struct *sbefifo sbe = dev_from_fd(fd);

    if(response_received(sbe->responses[fd])) {
        copy_user(sbe->responses[fd]);
        return len(sbe->data[fd]);
    }

    wait_interuptible(sbe->queues[fd]);
    copy_user(sbe->data[fd]);
    return len(sbe->data[fd]);
}

If a single threaded application calls this function without doing a write first,
that is a bug in the application, because it has just deadlocked itself.  This function
is never going to return from the system call because the wait event associated
with this fd is never going to occur.

It isn’t appropriate to make this an error, because that would prevent something
like a multithreaded application from doing a read in one thread, and then later
doing a write from another thread, which, as far as the kernel is concerned, is
a perfect valid thing for user space to do.

> One more use-case to cover --- If a read(fd) is done with a not-so-large-enough buffer, then kernel driver should flush the downstream fifo until it is empty and return buffer-too-small error to the caller. User cannot allocate a bigger buffer and retry read(fd) as it will end up in two consecutive reads without a write.

It won’t though.  A read system call != reading from the fifo.  A read system
call means give me back the data that came out of the fifo after I did a write.

> 
> -Venkatesh

Re: sbefifo userspace api

[Alistair Popple]

On Sat, 25 Feb 2017 10:44:03 AM Venkatesh Sainath wrote:
>
> On 24/02/17 11:44 AM, Alistair Popple wrote:
> > On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
> >> Based on what we’ve discussed here and a couple offline discussions,
> >> I’m making some assertions below.  I’d like for this note to be a
> >> “last call for discussion” (as much as that is possible) so we can
> >> start hacking; please try and raise any concerns by sometime next week.
> >>
> >> - sbefifo DD will implement read and write system calls.  A write
> >> will put data in the upstream fifo.  A read will pull data out of
> >> the downstream fifo.
> Brad,
>   Thanks for summarising the discussion. Instead of independent
> read/write calls, I was looking for a single ioctl(submit) operation
> which would always do a write of the incoming data packet to the
> upstream fifo followed by a read of a byte stream from the downstream
> fifo. There can be multiple users who open file descriptors to the sbe
> fifo in write mode. However, each ioctl(fd,submit,data) operation will
> be protected such that no two submit operations will happen in parallel.
> This way (a) the users dont have to know the semantics of the sbe fifo
> status bits ( data valid / end-of-transmission) and (b) there wont be
> any collision of write and read operations between two users. I am
> hoping this does not violate any linux DD norms.

It kind of does imho :-)

There is no need for an ioctl() to submit a data packet.

A write to a fd is the submission and can block until the fifo is free
and a read can block until a response is recieved for that fd. If
blocking is a problem we can implement async io
(ie. poll/select). Obviously this will require the kernel driver to
queue up requests to allow the resource to be shared.

Userspace does not and should not be exposed to any of the data
valid/end of transmission status bits - these should be dealt with by
the kernel.

This is very similar to the BMC BT driver (which is also just a fifo
with status flags). Main difference is you need to support multiple
open fd's.

- Alistair

> -Venkatesh
> >> - sbefifo DD will _not_ do any framing of data flowing into/out of the
> >> kernel (in other words, exactly what is found in the sbei interface spec).
> Agreed.
> >> - sbefifo will provide an in-kernel API which again, will not do any
> >> framing of data.
> Agreed.
> >> - occ-hwmon DD will assemble requests and parse responses for whatever
> >> chip-ops it requires and use the sbefifo in-kernel api to access the sbe.
> Agreed.
> >> - any userspace implementation of any part of the sbei spec will
> >> use the sbefifo uapi.
> Agreed.
> >> Below are not assertions but relevant items yet to be resolved, and
> >> should not prevent moving forward with implementation of the above.
> >>
> >> - Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
> >> assemble requests, parse responses for whatever chip-ops it requires
> >> and use the sbefifo in-kernel api in the same manner as the occ-hwmon DD.
> >> imho, this feels like convenience code since we have a real hw scom
> >> CFAM engine with UAPI.  A wrapper that hides this distinction could
> >> just as easily be done in userspace without another DD.
> I dont see a need for sbescom DD. A wrapper to the user-space sbe
> protocol driver lib can handle sbe scom vs fsi scom. For all practical
> purposes, we should ask users to use the sbe path only.
> > Thanks for taking the time to write this up! I don't have any concerns
> > with the above summary. I agree the sbescom driver seems unneccesary
> > as whether to use chip-ops or real HW scom is something that userspace
> > (rather than the kernel) should decide anyway, so I'm not sure what
> > value having that wrapper in the kernel would have.
> >
> > - Alistair
> >
> >> Thanks to everyone participating in the discussion.
> >>
> >> -brad
> >>
> >>> On Feb 22, 2017, at 12:59 PM, Alistair Popple <alistair@popple.id.au> wrote:
> >>>
> >>> On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
> >>>
> >>>> In general, I agree with this approach to contain only those interfaces
> >>>> required by hwmon within the kernel and leave the rest to the
> >>>> user-space. But, we need to interlock with the hwmon to understand
> >>>> exactly which interfaces are required.
> >>>> IMO, we need (a) Get/Put Memory for tunneling data to OCC and back (b)
> >>>> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages. If
> >>>> these three are the right ones, then we need to contain them within the
> >>>> kernel.
> >>> Where are the OCC interfaces documented? We need to get a better
> >>> understanding of these.
> >>>
> >>> Regardless I think you need a way for userspace to do chip-ops (for
> >>> debug/testing/development/etc). So I guess the question is then which
> >>> ones need a kernel implementation, and if there is a kernel
> >>> implementation should userspace use that or it's own implementation?
> >>>
> >>> I think it's easiest to just have the userspace library contain all
> >>> the neccessary chip-ops and use those. Formatting of chip-ops seems
> >>> like it should be a trival amount of straight-forward code so the
> >>> minor duplication shouldn't be a concern (especially if we leave
> >>> framing up to the kernel). It's simpler than maintaining a long list
> >>> of ioctls which would still need a userspace library imho.
> >>>
> >>>>>> I don't think it is a problem having get/putscom as an exceptional
> >>>>>> case for the reasons described above. My question about the
> >>>>>> consistency argument is what does it get us?
> >>>>>>
> >>>>>> I think Venkatesh was suggesting it would reduce duplication of
> >>>>>> protocol driver code, however perhaps this isn't the case. We could
> >>>>>> create a kernel chip-ops driver that deals with sending a buffer and
> >>>>>> getting a response but doesn't have to know anything about the chip-op
> >>>>>> itself. It could also implement the kernel sbe get/putscom.
> >>>>>>
> >>>>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 & 1.1.2.5
> >>>>>> of the sbe interface spec. Userspace would submit command-class,
> >>>>>> command-code & data words and the chip-ops driver would forward that
> >>>>>> to the SBE and send the response back to userspace without having to
> >>>>>> know what any of that data means.
> >>>>> I like it!
> >>>>>
> >>>>> Would this ‘chip-op’ driver be distinct from the sbefifo driver or
> >>>>> are you proposing a possible sbefifo driver API with a tad bit more
> >>>>> abstraction?
> >>>>>
> >>>>> If we did something like this, does a standalone sbescom driver still
> >>>>> have any value?
> >>>> I think the proposal is to have a combined sbefifo+sbechipop driver that
> >>>> provides a user space api to submit operations to sbefifo and user space
> >>>> api for those chipops contained within ( scom, memory and sram). The
> >>>> user space library will have the other sbe interfaces and will call the
> >>>> submit operations of the chip-op driver. Is this correct? We could get
> >>>> the user-space library provide a wrapper interface for even the chip-ops
> >>>> contained within the kernel so that the caller doesnt have to know
> >>>> whether to call the library or the driver apis.
> >>>>>> In any case I think you will end up with a userspace implementation of
> >>>>>> chip-ops anyway as it is much easier to test and develop new chip-ops
> >>>>>> without having to also understand how to build and flash a new kernel.
> >>>>>> Inevitably someone will want to add some kind of "debug" chip-op or
> >>>>>> other private chip-op that they don't want published in kernel code,
> >>>>>> and as soon as you have one userspace chip-op you may as well do all
> >>>>>> of the ones you can from there.
> >>>>>>
> >>>>>>>> Of course we can implement just the get/put scom chip ops in kernel and the others in user space.  I just want to make sure everyone understands exactly what we’d be doing there and would be OK with that approach.
> >>>>>>>>
> >>>>>>>>> There's the OCC hwmon driver - what chip-ops does that
> >>>>>>>>> need? Just get/putscom?
> >>>>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
> >>>>>>> occ hwmon driver should use get/put memory to communicate with OCC. One
> >>>>>> What memory is the OCC using to store these buffer? Does it have
> >>>>>> memory mapped onto the PowerBus into an MMIO space or something?
> >>>>>>> other option ( I am not too happy about this ) is to embed the get/put
> >>>>>>> memory protocol construction inside occ hwmon driver and make it call
> >>>>>>> sbe fifo driver interface directly. The sbei protocol driver can then be
> >>>>>>> in the user-space as a library and call the sbe fifo driver interface
> >>>>>>> via ioctl. This creates duplication of the protocol driver code and we
> >>>>>>> have to fix it in multiple places.
> >>>>>>>>> If that's the case I thought we were already
> >>>>>>>>> exposing scom chip-op operations via an OpenFSI master?
> >>>>>>>> I have the same consistency question here.  Why is it preferable to have a UAPI for these chip-ops but not the others?
> >>>>>>> openfsi driver is not exposing chipops. The sbe fifo driver must be
> >>>>>>> calling the openfsi driver interfaces to write to the fifo. The openfsi
> >>>>>>> driver must also be providing a UAPI for a non-sbe scom or cfam-write
> >>>>>>> operations via cronus, pdbg or rest api.
> >>>>>>>
> >>>>>>> Net: openfsi driver need to have both intra-kernel and user-space APIs.
> >>>>>>> sbe fifo driver also need to have both intra-kernel and user-space APIs
> >>>>>>> if the sbei protocol driver goes to user-space.
> >>>>>>>>> Regards,
> >>>>>>>>>
> >>>>>>>>> Alistair
> >>>>>>>>>
> >>>>>>>>>> thx - brad
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath <vsainath@linux.vnet.ibm.com> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> Hi Brad,
> >>>>>>>>>>>
> >>>>>>>>>>> I am not sure why we need a user-space api for SBE FIFO driver. Even the pdbg and cronus apps have to go through the SBEI protocol driver in order to construct the packets according to the SBEI interface spec which is done by the SBEI protocol driver only. I think only an in-kernel api is sufficient for the SBE FIFO driver.
> >>>>>>>>>>>
> >>>>>>>>>>> However, for the SBEI protocol driver, we would need both in-kernel ( for use by OCC hwmon ) and user-space api ( for use by pdbg, cronus and rest apis ).
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks
> >>>>>>>>>>>
> >>>>>>>>>>> With regards
> >>>>>>>>>>> Venkatesh
> >>>>>>>>>>>
> >>>>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
> >>>>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants from this thread:
> >>>>>>>>>>>>
> >>>>>>>>>>>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
> >>>>>>>>>>>>
> >>>>>>>>>>>> in an attempt to consolidate the whole sbefifo design discussion to a single thread.
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Hi Brad,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Looking to start a discussion around possible user space and kernel
> >>>>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-) that provides a
> >>>>>>>>>>>>>> single submit ioctl.  Applications submit a request and get a reply in
> >>>>>>>>>>>>>> single system call.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> Is something like that the best approach for an upstream driver?  Or
> >>>>>>>>>>>>>> should we try something more "pipe like" with read/write interfaces?
> >>>>>>>>>>>>> It probably depends on the functionality there; ioctl() is useful in
> >>>>>>>>>>>>> that (as you say) we can handle request and response in a single
> >>>>>>>>>>>>> syscall, read() / write() may be more appropriate if ordering can be
> >>>>>>>>>>>>> handled in userspace.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Can you add a little description about the functionality we're exposing?
> >>>>>>>>>>>>> That may suggest a particular API.
> >>>>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just two 8-word queues for sending/receiving messages to/from the SBE.  Each queue entry has a single ‘end of transfer’ flag to let the other side know the message is done.
> >>>>>>>>>>>>
> >>>>>>>>>>>> In terms of data flowing through it, there is an SBEI protocol that covers encoding operations (like getscom, getmem, etc.. aka chip-ops) and the SBE response.
> >>>>>>>>>>>>
> >>>>>>>>>>>> For users there seems to be two classes:
> >>>>>>>>>>>>
> >>>>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
> >>>>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
> >>>>>>>>>>>>
> >>>>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there would be any need for the kernel to have any knowledge of the data flowing through the fifo (at the moment anyway).  An sbe-scom driver has been suggested but I wonder what the point of that driver would be, if userspace could simply encode a get/putscom chip-op and use the fifo directly.
> >>>>>>>>>>>>
> >>>>>>>>>>>>>> Would the in-kernel API be the same as the user space API?
> >>>>>>>>>>>>> Probably not :)
> >>>>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats why I got the smiley face :-) ...
> >>>>>>>>>>>>
> >>>>>>>>>>>> But I would have figured they’d at least be similar - meaning if we were go the ‘submit’ route for a UAPI..the kernel
> >>>>>>>>>>>> would probably not have a split read/write API or vise versa.
> >>>>>>>>>>>>
> >>>>>>>>>>>> So to rephrase the question - would the chardev fops implementation simply be something like this:
> >>>>>>>>>>>>
> >>>>>>>>>>>> sbe-uapi-fops(chardev)
> >>>>>>>>>>>>    data = copy to/from user space;
> >>>>>>>>>>>>    sbedev = from_chardev(chardev);
> >>>>>>>>>>>>    kernel-api(sbedev, data);
> >>>>>>>>>>>>
> >>>>>>>>>>>> Or are the other things to consider here?
> >>>>>>>>>>>>
> >>>>>>>>>>>> -thx
> >>>>>>>>>>>>
> >>>>>>>>>>>> brad
> >>>>>>>>>>>>
> >>>>>>>>>>>>> Cheers,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Jeremy
>

Re: sbefifo userspace api

[Milton Miller II]

Alistair Popple wrote on or about 02/24/2017 12:16AM in some timezone:

>On Thu, 23 Feb 2017 10:01:15 PM Brad Bishop wrote:
>> Based on what we’ve discussed here and a couple offline
>discussions,
>> I’m making some assertions below.  I’d like for this note to be a
>> “last call for discussion” (as much as that is possible) so we can
>> start hacking; please try and raise any concerns by sometime next
>week.
>>
>> - sbefifo DD will implement read and write system calls.  A write
>> will put data in the upstream fifo.  A read will pull data out of
>> the downstream fifo.
>>
>> - sbefifo DD will _not_ do any framing of data flowing into/out of
>the
>> kernel (in other words, exactly what is found in the sbei interface
>spec).
>>
>> - sbefifo will provide an in-kernel API which again, will not do
>any
>> framing of data.
>>
>> - occ-hwmon DD will assemble requests and parse responses for
>whatever
>> chip-ops it requires and use the sbefifo in-kernel api to access
>the sbe.
>>
>> - any userspace implementation of any part of the sbei spec will
>> use the sbefifo uapi.


To me, one key thing a kernel driver should be providing is resource 
sharing.  Without finding the sbei spec, I have heard the description 
is one outstanding op at a time, send the request and the sbe responds 
with a response; asynchronous notifications are done by issuing a poll 
request.

For this interface to be shared by the kernel and user programs, it 
would seem that the write data would have to be submitted in one 
system call and this file descriptor would be the only one that 
could satisfy a read, and the data should be read in one system call.


If the data is just a byte stream then how can multiple users or
sources share the fifo?  Each agent would get a random response
and even with exclusive open the occ driver would have to be
stopped or paused or unbound for the user operation to occur.

Am I missing something fundamental?

milton

>>
>> Below are not assertions but relevant items yet to be resolved, and
>> should not prevent moving forward with implementation of the above.
>>
>> - Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
>> assemble requests, parse responses for whatever chip-ops it
>requires
>> and use the sbefifo in-kernel api in the same manner as the
>occ-hwmon DD.
>> imho, this feels like convenience code since we have a real hw scom
>> CFAM engine with UAPI.  A wrapper that hides this distinction could
>> just as easily be done in userspace without another DD.
>
>Thanks for taking the time to write this up! I don't have any
>concerns
>with the above summary. I agree the sbescom driver seems unneccesary
>as whether to use chip-ops or real HW scom is something that
>userspace
>(rather than the kernel) should decide anyway, so I'm not sure what
>value having that wrapper in the kernel would have.
>
>- Alistair
>
>> Thanks to everyone participating in the discussion.
>>
>> -brad
>>
>> > On Feb 22, 2017, at 12:59 PM, Alistair Popple
><alistair@popple.id.au> wrote:
>> >
>> > On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
>> >
>> >> In general, I agree with this approach to contain only those
>interfaces
>> >> required by hwmon within the kernel and leave the rest to the
>> >> user-space. But, we need to interlock with the hwmon to
>understand
>> >> exactly which interfaces are required.
>> >> IMO, we need (a) Get/Put Memory for tunneling data to OCC and
>back (b)
>> >> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages.
>If
>> >> these three are the right ones, then we need to contain them
>within the
>> >> kernel.
>> >
>> > Where are the OCC interfaces documented? We need to get a better
>> > understanding of these.
>> >
>> > Regardless I think you need a way for userspace to do chip-ops
>(for
>> > debug/testing/development/etc). So I guess the question is then
>which
>> > ones need a kernel implementation, and if there is a kernel
>> > implementation should userspace use that or it's own
>implementation?
>> >
>> > I think it's easiest to just have the userspace library contain
>all
>> > the neccessary chip-ops and use those. Formatting of chip-ops
>seems
>> > like it should be a trival amount of straight-forward code so the
>> > minor duplication shouldn't be a concern (especially if we leave
>> > framing up to the kernel). It's simpler than maintaining a long
>list
>> > of ioctls which would still need a userspace library imho.
>> >
>> >>>> I don't think it is a problem having get/putscom as an
>exceptional
>> >>>> case for the reasons described above. My question about the
>> >>>> consistency argument is what does it get us?
>> >>>>
>> >>>> I think Venkatesh was suggesting it would reduce duplication
>of
>> >>>> protocol driver code, however perhaps this isn't the case. We
>could
>> >>>> create a kernel chip-ops driver that deals with sending a
>buffer and
>> >>>> getting a response but doesn't have to know anything about the
>chip-op
>> >>>> itself. It could also implement the kernel sbe get/putscom.
>> >>>>
>> >>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 &
>1.1.2.5
>> >>>> of the sbe interface spec. Userspace would submit
>command-class,
>> >>>> command-code & data words and the chip-ops driver would
>forward that
>> >>>> to the SBE and send the response back to userspace without
>having to
>> >>>> know what any of that data means.
>> >>> I like it!
>> >>>
>> >>> Would this ‘chip-op’ driver be distinct from the sbefifo driver
>or
>> >>> are you proposing a possible sbefifo driver API with a tad bit
>more
>> >>> abstraction?
>> >>>
>> >>> If we did something like this, does a standalone sbescom driver
>still
>> >>> have any value?
>> >> I think the proposal is to have a combined sbefifo+sbechipop
>driver that
>> >> provides a user space api to submit operations to sbefifo and
>user space
>> >> api for those chipops contained within ( scom, memory and sram).
>The
>> >> user space library will have the other sbe interfaces and will
>call the
>> >> submit operations of the chip-op driver. Is this correct? We
>could get
>> >> the user-space library provide a wrapper interface for even the
>chip-ops
>> >> contained within the kernel so that the caller doesnt have to
>know
>> >> whether to call the library or the driver apis.
>> >>>> In any case I think you will end up with a userspace
>implementation of
>> >>>> chip-ops anyway as it is much easier to test and develop new
>chip-ops
>> >>>> without having to also understand how to build and flash a new
>kernel.
>> >>>> Inevitably someone will want to add some kind of "debug"
>chip-op or
>> >>>> other private chip-op that they don't want published in kernel
>code,
>> >>>> and as soon as you have one userspace chip-op you may as well
>do all
>> >>>> of the ones you can from there.
>> >>>>
>> >>>>>> Of course we can implement just the get/put scom chip ops in
>kernel and the others in user space.  I just want to make sure
>everyone understands exactly what we’d be doing there and would be OK
>with that approach.
>> >>>>>>
>> >>>>>>> There's the OCC hwmon driver - what chip-ops does that
>> >>>>>>> need? Just get/putscom?
>> >>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>> >>>>> occ hwmon driver should use get/put memory to communicate
>with OCC. One
>> >>>> What memory is the OCC using to store these buffer? Does it
>have
>> >>>> memory mapped onto the PowerBus into an MMIO space or
>something?
>> >>>>> other option ( I am not too happy about this ) is to embed
>the get/put
>> >>>>> memory protocol construction inside occ hwmon driver and make
>it call
>> >>>>> sbe fifo driver interface directly. The sbei protocol driver
>can then be
>> >>>>> in the user-space as a library and call the sbe fifo driver
>interface
>> >>>>> via ioctl. This creates duplication of the protocol driver
>code and we
>> >>>>> have to fix it in multiple places.
>> >>>>>>> If that's the case I thought we were already
>> >>>>>>> exposing scom chip-op operations via an OpenFSI master?
>> >>>>>> I have the same consistency question here.  Why is it
>preferable to have a UAPI for these chip-ops but not the others?
>> >>>>> openfsi driver is not exposing chipops. The sbe fifo driver
>must be
>> >>>>> calling the openfsi driver interfaces to write to the fifo.
>The openfsi
>> >>>>> driver must also be providing a UAPI for a non-sbe scom or
>cfam-write
>> >>>>> operations via cronus, pdbg or rest api.
>> >>>>>
>> >>>>> Net: openfsi driver need to have both intra-kernel and
>user-space APIs.
>> >>>>> sbe fifo driver also need to have both intra-kernel and
>user-space APIs
>> >>>>> if the sbei protocol driver goes to user-space.
>> >>>>>>> Regards,
>> >>>>>>>
>> >>>>>>> Alistair
>> >>>>>>>
>> >>>>>>>> thx - brad
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath
><vsainath@linux.vnet.ibm.com> wrote:
>> >>>>>>>>>
>> >>>>>>>>> Hi Brad,
>> >>>>>>>>>
>> >>>>>>>>> I am not sure why we need a user-space api for SBE FIFO
>driver. Even the pdbg and cronus apps have to go through the SBEI
>protocol driver in order to construct the packets according to the
>SBEI interface spec which is done by the SBEI protocol driver only. I
>think only an in-kernel api is sufficient for the SBE FIFO driver.
>> >>>>>>>>>
>> >>>>>>>>> However, for the SBEI protocol driver, we would need both
>in-kernel ( for use by OCC hwmon ) and user-space api ( for use by
>pdbg, cronus and rest apis ).
>> >>>>>>>>>
>> >>>>>>>>> Thanks
>> >>>>>>>>>
>> >>>>>>>>> With regards
>> >>>>>>>>> Venkatesh
>> >>>>>>>>>
>> >>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>> >>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants
>from this thread:
>> >>>>>>>>>>
>> >>>>>>>>>>
>https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>> >>>>>>>>>>
>> >>>>>>>>>> in an attempt to consolidate the whole sbefifo design
>discussion to a single thread.
>> >>>>>>>>>>
>> >>>>>>>>>>> Hi Brad,
>> >>>>>>>>>>>
>> >>>>>>>>>>>> Looking to start a discussion around possible user
>space and kernel
>> >>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-)
>that provides a
>> >>>>>>>>>>>> single submit ioctl.  Applications submit a request
>and get a reply in
>> >>>>>>>>>>>> single system call.
>> >>>>>>>>>>>>
>> >>>>>>>>>>>> Is something like that the best approach for an
>upstream driver?  Or
>> >>>>>>>>>>>> should we try something more "pipe like" with
>read/write interfaces?
>> >>>>>>>>>>> It probably depends on the functionality there; ioctl()
>is useful in
>> >>>>>>>>>>> that (as you say) we can handle request and response in
>a single
>> >>>>>>>>>>> syscall, read() / write() may be more appropriate if
>ordering can be
>> >>>>>>>>>>> handled in userspace.
>> >>>>>>>>>>>
>> >>>>>>>>>>> Can you add a little description about the
>functionality we're exposing?
>> >>>>>>>>>>> That may suggest a particular API.
>> >>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just
>two 8-word queues for sending/receiving messages to/from the SBE.
>Each queue entry has a single ‘end of transfer’ flag to let the other
>side know the message is done.
>> >>>>>>>>>>
>> >>>>>>>>>> In terms of data flowing through it, there is an SBEI
>protocol that covers encoding operations (like getscom, getmem, etc..
>aka chip-ops) and the SBE response.
>> >>>>>>>>>>
>> >>>>>>>>>> For users there seems to be two classes:
>> >>>>>>>>>>
>> >>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>> >>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>> >>>>>>>>>>
>> >>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there
>would be any need for the kernel to have any knowledge of the data
>flowing through the fifo (at the moment anyway).  An sbe-scom driver
>has been suggested but I wonder what the point of that driver would
>be, if userspace could simply encode a get/putscom chip-op and use
>the fifo directly.
>> >>>>>>>>>>
>> >>>>>>>>>>>> Would the in-kernel API be the same as the user space
>API?
>> >>>>>>>>>>> Probably not :)
>> >>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats
>why I got the smiley face :-) ...
>> >>>>>>>>>>
>> >>>>>>>>>> But I would have figured they’d at least be similar -
>meaning if we were go the ‘submit’ route for a UAPI..the kernel
>> >>>>>>>>>> would probably not have a split read/write API or vise
>versa.
>> >>>>>>>>>>
>> >>>>>>>>>> So to rephrase the question - would the chardev fops
>implementation simply be something like this:
>> >>>>>>>>>>
>> >>>>>>>>>> sbe-uapi-fops(chardev)
>> >>>>>>>>>>   data = copy to/from user space;
>> >>>>>>>>>>   sbedev = from_chardev(chardev);
>> >>>>>>>>>>   kernel-api(sbedev, data);
>> >>>>>>>>>>
>> >>>>>>>>>> Or are the other things to consider here?
>> >>>>>>>>>>
>> >>>>>>>>>> -thx
>> >>>>>>>>>>
>> >>>>>>>>>> brad
>> >>>>>>>>>>
>> >>>>>>>>>>> Cheers,
>> >>>>>>>>>>>
>> >>>>>>>>>>>
>> >>>>>>>>>>> Jeremy
>> >>
>
>

Re: sbefifo userspace api

[Brad Bishop]

[snip]
> 
> To me, one key thing a kernel driver should be providing is resource 
> sharing.  Without finding the sbei spec, I have heard the description 
> is one outstanding op at a time, send the request and the sbe responds 
> with a response; asynchronous notifications are done by issuing a poll 
> request.

That is my understanding as well.

> 
> For this interface to be shared by the kernel and user programs, it 
> would seem that the write data would have to be submitted in one 
> system call and this file descriptor would be the only one that 
> could satisfy a read, and the data should be read in one system call.
> 
> 
> If the data is just a byte stream then how can multiple users or
> sources share the fifo?  

By treating it as fifo-like and not storage-like.  If a write hasn’t
been done to an fd and a read occurs on that fd, the read would block
until a write happens (on that same fd).

> Each agent would get a random response
> and even with exclusive open the occ driver would have to be
> stopped or paused or unbound for the user operation to occur.
> 
> Am I missing something fundamental?
> 
> milton
> 
>>> 
>>> Below are not assertions but relevant items yet to be resolved, and
>>> should not prevent moving forward with implementation of the above.
>>> 
>>> - Whether or not an ‘sbescom’ DD is valuable.  If it is, it will
>>> assemble requests, parse responses for whatever chip-ops it
>> requires
>>> and use the sbefifo in-kernel api in the same manner as the
>> occ-hwmon DD.
>>> imho, this feels like convenience code since we have a real hw scom
>>> CFAM engine with UAPI.  A wrapper that hides this distinction could
>>> just as easily be done in userspace without another DD.
>> 
>> Thanks for taking the time to write this up! I don't have any
>> concerns
>> with the above summary. I agree the sbescom driver seems unneccesary
>> as whether to use chip-ops or real HW scom is something that
>> userspace
>> (rather than the kernel) should decide anyway, so I'm not sure what
>> value having that wrapper in the kernel would have.
>> 
>> - Alistair
>> 
>>> Thanks to everyone participating in the discussion.
>>> 
>>> -brad
>>> 
>>>> On Feb 22, 2017, at 12:59 PM, Alistair Popple
>> <alistair@popple.id.au> wrote:
>>>> 
>>>> On Wed, 22 Feb 2017 10:53:12 PM Venkatesh Sainath wrote:
>>>> 
>>>>> In general, I agree with this approach to contain only those
>> interfaces
>>>>> required by hwmon within the kernel and leave the rest to the
>>>>> user-space. But, we need to interlock with the hwmon to
>> understand
>>>>> exactly which interfaces are required.
>>>>> IMO, we need (a) Get/Put Memory for tunneling data to OCC and
>> back (b)
>>>>> Get/Put Scom? (c) Get/Put SRAM (?) for getting nominal voltages.
>> If
>>>>> these three are the right ones, then we need to contain them
>> within the
>>>>> kernel.
>>>> 
>>>> Where are the OCC interfaces documented? We need to get a better
>>>> understanding of these.
>>>> 
>>>> Regardless I think you need a way for userspace to do chip-ops
>> (for
>>>> debug/testing/development/etc). So I guess the question is then
>> which
>>>> ones need a kernel implementation, and if there is a kernel
>>>> implementation should userspace use that or it's own
>> implementation?
>>>> 
>>>> I think it's easiest to just have the userspace library contain
>> all
>>>> the neccessary chip-ops and use those. Formatting of chip-ops
>> seems
>>>> like it should be a trival amount of straight-forward code so the
>>>> minor duplication shouldn't be a concern (especially if we leave
>>>> framing up to the kernel). It's simpler than maintaining a long
>> list
>>>> of ioctls which would still need a userspace library imho.
>>>> 
>>>>>>> I don't think it is a problem having get/putscom as an
>> exceptional
>>>>>>> case for the reasons described above. My question about the
>>>>>>> consistency argument is what does it get us?
>>>>>>> 
>>>>>>> I think Venkatesh was suggesting it would reduce duplication
>> of
>>>>>>> protocol driver code, however perhaps this isn't the case. We
>> could
>>>>>>> create a kernel chip-ops driver that deals with sending a
>> buffer and
>>>>>>> getting a response but doesn't have to know anything about the
>> chip-op
>>>>>>> itself. It could also implement the kernel sbe get/putscom.
>>>>>>> 
>>>>>>> What I am suggesting is the chip-op driver deal with 1.1.2.4 &
>> 1.1.2.5
>>>>>>> of the sbe interface spec. Userspace would submit
>> command-class,
>>>>>>> command-code & data words and the chip-ops driver would
>> forward that
>>>>>>> to the SBE and send the response back to userspace without
>> having to
>>>>>>> know what any of that data means.
>>>>>> I like it!
>>>>>> 
>>>>>> Would this ‘chip-op’ driver be distinct from the sbefifo driver
>> or
>>>>>> are you proposing a possible sbefifo driver API with a tad bit
>> more
>>>>>> abstraction?
>>>>>> 
>>>>>> If we did something like this, does a standalone sbescom driver
>> still
>>>>>> have any value?
>>>>> I think the proposal is to have a combined sbefifo+sbechipop
>> driver that
>>>>> provides a user space api to submit operations to sbefifo and
>> user space
>>>>> api for those chipops contained within ( scom, memory and sram).
>> The
>>>>> user space library will have the other sbe interfaces and will
>> call the
>>>>> submit operations of the chip-op driver. Is this correct? We
>> could get
>>>>> the user-space library provide a wrapper interface for even the
>> chip-ops
>>>>> contained within the kernel so that the caller doesnt have to
>> know
>>>>> whether to call the library or the driver apis.
>>>>>>> In any case I think you will end up with a userspace
>> implementation of
>>>>>>> chip-ops anyway as it is much easier to test and develop new
>> chip-ops
>>>>>>> without having to also understand how to build and flash a new
>> kernel.
>>>>>>> Inevitably someone will want to add some kind of "debug"
>> chip-op or
>>>>>>> other private chip-op that they don't want published in kernel
>> code,
>>>>>>> and as soon as you have one userspace chip-op you may as well
>> do all
>>>>>>> of the ones you can from there.
>>>>>>> 
>>>>>>>>> Of course we can implement just the get/put scom chip ops in
>> kernel and the others in user space.  I just want to make sure
>> everyone understands exactly what we’d be doing there and would be OK
>> with that approach.
>>>>>>>>> 
>>>>>>>>>> There's the OCC hwmon driver - what chip-ops does that
>>>>>>>>>> need? Just get/putscom?
>>>>>>>>> Correct, just get/putscom.  But sbe scom, not direct scom.
>>>>>>>> occ hwmon driver should use get/put memory to communicate
>> with OCC. One
>>>>>>> What memory is the OCC using to store these buffer? Does it
>> have
>>>>>>> memory mapped onto the PowerBus into an MMIO space or
>> something?
>>>>>>>> other option ( I am not too happy about this ) is to embed
>> the get/put
>>>>>>>> memory protocol construction inside occ hwmon driver and make
>> it call
>>>>>>>> sbe fifo driver interface directly. The sbei protocol driver
>> can then be
>>>>>>>> in the user-space as a library and call the sbe fifo driver
>> interface
>>>>>>>> via ioctl. This creates duplication of the protocol driver
>> code and we
>>>>>>>> have to fix it in multiple places.
>>>>>>>>>> If that's the case I thought we were already
>>>>>>>>>> exposing scom chip-op operations via an OpenFSI master?
>>>>>>>>> I have the same consistency question here.  Why is it
>> preferable to have a UAPI for these chip-ops but not the others?
>>>>>>>> openfsi driver is not exposing chipops. The sbe fifo driver
>> must be
>>>>>>>> calling the openfsi driver interfaces to write to the fifo.
>> The openfsi
>>>>>>>> driver must also be providing a UAPI for a non-sbe scom or
>> cfam-write
>>>>>>>> operations via cronus, pdbg or rest api.
>>>>>>>> 
>>>>>>>> Net: openfsi driver need to have both intra-kernel and
>> user-space APIs.
>>>>>>>> sbe fifo driver also need to have both intra-kernel and
>> user-space APIs
>>>>>>>> if the sbei protocol driver goes to user-space.
>>>>>>>>>> Regards,
>>>>>>>>>> 
>>>>>>>>>> Alistair
>>>>>>>>>> 
>>>>>>>>>>> thx - brad
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>>>> On Feb 20, 2017, at 12:05 AM, Venkatesh Sainath
>> <vsainath@linux.vnet.ibm.com> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> Hi Brad,
>>>>>>>>>>>> 
>>>>>>>>>>>> I am not sure why we need a user-space api for SBE FIFO
>> driver. Even the pdbg and cronus apps have to go through the SBEI
>> protocol driver in order to construct the packets according to the
>> SBEI interface spec which is done by the SBEI protocol driver only. I
>> think only an in-kernel api is sufficient for the SBE FIFO driver.
>>>>>>>>>>>> 
>>>>>>>>>>>> However, for the SBEI protocol driver, we would need both
>> in-kernel ( for use by OCC hwmon ) and user-space api ( for use by
>> pdbg, cronus and rest apis ).
>>>>>>>>>>>> 
>>>>>>>>>>>> Thanks
>>>>>>>>>>>> 
>>>>>>>>>>>> With regards
>>>>>>>>>>>> Venkatesh
>>>>>>>>>>>> 
>>>>>>>>>>>> On 20/02/17 8:17 AM, Brad Bishop wrote:
>>>>>>>>>>>>> Thanks Jeremy for the reply.  I’ve added participants
>> from this thread:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>> https://lists.ozlabs.org/pipermail/openbmc/2017-February/006563.html
>>>>>>>>>>>>> 
>>>>>>>>>>>>> in an attempt to consolidate the whole sbefifo design
>> discussion to a single thread.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Hi Brad,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Looking to start a discussion around possible user
>> space and kernel
>>>>>>>>>>>>>>> APIs for the POWER9 sbefifo driver.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> There exists today an “alternate" sbefifo driver :-)
>> that provides a
>>>>>>>>>>>>>>> single submit ioctl.  Applications submit a request
>> and get a reply in
>>>>>>>>>>>>>>> single system call.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Is something like that the best approach for an
>> upstream driver?  Or
>>>>>>>>>>>>>>> should we try something more "pipe like" with
>> read/write interfaces?
>>>>>>>>>>>>>> It probably depends on the functionality there; ioctl()
>> is useful in
>>>>>>>>>>>>>> that (as you say) we can handle request and response in
>> a single
>>>>>>>>>>>>>> syscall, read() / write() may be more appropriate if
>> ordering can be
>>>>>>>>>>>>>> handled in userspace.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Can you add a little description about the
>> functionality we're exposing?
>>>>>>>>>>>>>> That may suggest a particular API.
>>>>>>>>>>>>> In terms of hardware its pretty simple.  sbefifo is just
>> two 8-word queues for sending/receiving messages to/from the SBE.
>> Each queue entry has a single ‘end of transfer’ flag to let the other
>> side know the message is done.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> In terms of data flowing through it, there is an SBEI
>> protocol that covers encoding operations (like getscom, getmem, etc..
>> aka chip-ops) and the SBE response.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> For users there seems to be two classes:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 1 - user space wanting to do chip-ops (pdbg, cronus).
>>>>>>>>>>>>> 2 - device drivers wanting to do chip-ops (occ-hwmon).
>>>>>>>>>>>>> 
>>>>>>>>>>>>> If it weren’t for occ-hwmon, it doesn’t seem like there
>> would be any need for the kernel to have any knowledge of the data
>> flowing through the fifo (at the moment anyway).  An sbe-scom driver
>> has been suggested but I wonder what the point of that driver would
>> be, if userspace could simply encode a get/putscom chip-op and use
>> the fifo directly.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Would the in-kernel API be the same as the user space
>> API?
>>>>>>>>>>>>>> Probably not :)
>>>>>>>>>>>>> I realize they wouldn’t be _exactly_ the same if thats
>> why I got the smiley face :-) ...
>>>>>>>>>>>>> 
>>>>>>>>>>>>> But I would have figured they’d at least be similar -
>> meaning if we were go the ‘submit’ route for a UAPI..the kernel
>>>>>>>>>>>>> would probably not have a split read/write API or vise
>> versa.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> So to rephrase the question - would the chardev fops
>> implementation simply be something like this:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> sbe-uapi-fops(chardev)
>>>>>>>>>>>>>  data = copy to/from user space;
>>>>>>>>>>>>>  sbedev = from_chardev(chardev);
>>>>>>>>>>>>>  kernel-api(sbedev, data);
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Or are the other things to consider here?
>>>>>>>>>>>>> 
>>>>>>>>>>>>> -thx
>>>>>>>>>>>>> 
>>>>>>>>>>>>> brad
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Jeremy

Re: sbefifo userspace api

[Patrick Williams]

[-- Attachment #1: Type: text/plain, Size: 658 bytes --]

On Tue, Feb 21, 2017 at 02:34:18AM +1100, Alistair Popple wrote:
> I would also be interested in the answer to the inverse - why put them
> in the kernel? There's the OCC hwmon driver - what chip-ops does that
> need? Just get/putscom? If that's the case I thought we were already
> exposing scom chip-op operations via an OpenFSI master?

When the P9 chip has security enabled, you cannot even issue SCOM
operations without going through the SBE FIFO.  The SBE provides a
white-list filtering system that prevents the majority of the SCOM space
from being accessed.  We will need a "SCOM over FIFO" driver on top of
FSI.

-- 
Patrick Williams

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: sbefifo userspace api

[Brad Bishop]

> On Feb 21, 2017, at 4:49 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
> 
> On Tue, Feb 21, 2017 at 02:34:18AM +1100, Alistair Popple wrote:
>> I would also be interested in the answer to the inverse - why put them
>> in the kernel? There's the OCC hwmon driver - what chip-ops does that
>> need? Just get/putscom? If that's the case I thought we were already
>> exposing scom chip-op operations via an OpenFSI master?
> 
> When the P9 chip has security enabled, you cannot even issue SCOM
> operations without going through the SBE FIFO.  The SBE provides a
> white-list filtering system that prevents the majority of the SCOM space
> from being accessed.  We will need a "SCOM over FIFO" driver on top of
> FSI.

Well, we need a way to do scoms over sbe.  Not sure that equals being
a device driver.  As it stands if it were a driver, it would just be
very thin wrapper around the sbefifo driver.

> 
> -- 
> Patrick Williams

Re: sbefifo userspace api

[Stewart Smith]

Brad Bishop <bradleyb@fuzziesquirrel.com> writes:
>> On Feb 21, 2017, at 4:49 PM, Patrick Williams <patrick@stwcx.xyz> wrote:
>> 
>> On Tue, Feb 21, 2017 at 02:34:18AM +1100, Alistair Popple wrote:
>>> I would also be interested in the answer to the inverse - why put them
>>> in the kernel? There's the OCC hwmon driver - what chip-ops does that
>>> need? Just get/putscom? If that's the case I thought we were already
>>> exposing scom chip-op operations via an OpenFSI master?
>> 
>> When the P9 chip has security enabled, you cannot even issue SCOM
>> operations without going through the SBE FIFO.  The SBE provides a
>> white-list filtering system that prevents the majority of the SCOM space
>> from being accessed.  We will need a "SCOM over FIFO" driver on top of
>> FSI.
>
> Well, we need a way to do scoms over sbe.  Not sure that equals being
> a device driver.  As it stands if it were a driver, it would just be
> very thin wrapper around the sbefifo driver.

it'd be nice if getscom/putscom userspace could be the same on BMC and
host though.

-- 
Stewart Smith
OPAL Architect, IBM.