[Skiboot] [PATCH 00/11] MBOX Protocol: Onwards to V3

[Suraj Jitindar Singh]

Hi William,

Great that you want to contribute. Please see below :)

On Thu, 2017-06-29 at 16:05 +0000, William Kennington wrote:
> This is somewhat tangential to the patch, but we haven't seen any
> tracking for the effort to define a V3 protocol and already started
> making our own changes to accommodate our own needs for the mailbox
> daemon. I wrote up our initial proposal for the changes to the
> protocol. So far it is just a simple addition of the ability to use
> multiple flash devices.
> https://github.com/wak-
> google/mboxbridge/commit/e9b7c482fcff5278ee1ad46ecd8d6240d5b8886e

Given there is no mboxbridge mailing list we can either discuss this
kind of think in the gerrit for mboxbridge (https://gerrit.openbmc-proj
ect.xyz/#/q/project:openbmc/mboxbridge) or on the openbmc mailing list
(which is kinda high traffic but an option).

If you do have features you would like to see included in the V3 spec
then my suggestion would be to send a patch to the main protocol repo
so that we can atleast start reviewing it and given it looks like there
aren't huge time constraints on a V3 we could probably merge this all
into a single version bump. The repo is managed through a gerrit
instance (https://gerrit.openbmc-project.xyz/#/q/project:openbmc/mboxbr
idge) which I think just requires a github account to contribute to.

Given the mboxbridge repo (https://github.com/openbmc/mboxbridge) is
the reference implementation this is were eventually any changes would
need to end up for them to be accepted as a part of the protocol.

> 
> We already have prototype skiboot and mboxd code implementing V3. We
> would like to get this into the current proposal for V3 but it is
> clearly late in the cycle and could jump on an iteration for V4.
> 
> On Thu, Jun 29, 2017 at 5:50 AM Cyril Bur <cyril.bur at au1.ibm.com>
> wrote:
> > Hello,
> > 
> > Firstly, let me apologise for the amount of rework in this series.
> > Ultimately the goal here was really the last three patches.
> > Unfortunately in the process of writing them I started thinking
> > about
> > some of the problems in V2.
> > 
> > The easiest to address is a bug that came up in testing V3 which is
> > that if opening a window fails, skiboot will assume that the
> > previously
> > opened window is still valid this is incorrect. Patch 2 address
> > this
> > problem.
> > 
> > Then I got to thinking about errors and timeouts. It turns out that
> > currently if mbox-flash decides a message times out, it will not be
> > able to send anymore due to the lpc-mbox driver needing a response
> > for
> > every message it sent out - patch 4 deals with this. However after
> > writing a significantly less invasive fix I realised that the
> > entire
> > handling of timeouts/retries/resends/late responses was racey. So a
> > much simpler version of patch 4 became patches 3-6.
> > 
> > Then there was some extra V2 stuff that wasn't necessary but is
> > nice
> > to have - patch 7.
> > 
> > Then on to actually implementing V3 - at the time of writing V2
> > Suraj
> > did suggest what has become patch 8. This actually makes the V3
> > patch
> > - patch 9 very easy.
> > 
> > The final patch is an extended, reworked and even more improved
> > version of the tests I sent with V2 (which weren't merged at the
> > time)
> > 
> > One final note: this series passes all the tests I can throw at it
> > however I do not have reliable access to a MBOX V3 Daemon running
> > on
> > real hardware. Suraj has been kind enough to use (very) similar
> > versions of this series in his testing and he informs me that there
> > are no problems, however this exact code has never run on real
> > hardware.
> > 
> > 
> > Thanks,
> > 
> > Cyril
> > 
> > Cyril Bur (11):
> >   libflash/mbox-flash: Add v2 error codes
> >   libflash/mbox-flash: Always close windows before opening a new
> > window
> >   libflash/mbox-flash: Move sequence handling to driver level
> >   libflash/mbox-flash: Allow mbox-flash to tell the driver msg
> > timeouts
> >   hw/lpc-mbox: Simplify message bookkeeping and timeouts
> >   libflash/mbox-flash: Simplify message sending
> >   libflash/mbox-flash: Use BMC suggested timeout value
> >   libflash/mbox-flash: Use static arrays of function pointers
> >   libflash/mbox-flash: Understand v3
> >   libflash/mbox-flash: Add the ability to lock flash
> >   libflash/test: Add tests for mbox-flash
> > 
> >  .gitignore                   |   1 +
> >  hw/lpc-mbox.c                |  53 +++--
> >  include/lpc-mbox.h           |   9 +-
> >  include/skiboot.h            |   2 +
> >  libflash/mbox-flash.c        | 404 +++++++++++++++++++++--------
> > ------
> >  libflash/mbox-flash.h        |   1 +
> >  libflash/test/Makefile.check |  17 +-
> >  libflash/test/mbox-server.c  | 499
> > +++++++++++++++++++++++++++++++++++++++++++
> >  libflash/test/mbox-server.h  |  10 +
> >  libflash/test/stubs.c        |  78 ++++++-
> >  libflash/test/stubs.h        |  27 +++
> >  libflash/test/test-mbox.c    | 341 +++++++++++++++++++++++++++++
> >  12 files changed, 1254 insertions(+), 188 deletions(-)
> >  create mode 100644 libflash/test/mbox-server.c
> >  create mode 100644 libflash/test/mbox-server.h
> >  create mode 100644 libflash/test/stubs.h
> >  create mode 100644 libflash/test/test-mbox.c
> > 
> > --
> > 2.13.2
> > 
> > _______________________________________________
> > Skiboot mailing list
> > Skiboot at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/skiboot
> >