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

Cyril Bur cyril.bur at au1.ibm.com
Fri Jun 30 09:04:41 AEST 2017


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

Thanks for letting us know. Admittedly Suraj and I did silo this a bit
- although I'm not sure where we should all agree to announce/discuss
ideas and proposals, something to think about.

> 
> 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.

>From skiboot point of view it doesn't need to always use all the
features of the version it declares to know. So if you sent your work
to apply on top of this series I don't see a problem.

mboxd may not be as smooth, I suppose both Surajs work and your work
could go in separately but only once its all merged mboxd would expose 
v3.

Most of the discussion would have to be around your new command, from
eyeballing the link, I don't see any problems.

I suppose the question is how ready are you to start upstreaming?

Thanks,

Cyril

> 
> 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



More information about the Skiboot mailing list