[OpenPower-Firmware] [POWER8] OCC Firdata over IPMI

[Artem Senichev]

On Fri, May 24, 2019 at 9:14 PM Douglas Gilbert <dgilbert at us.ibm.com> wrote:
>
> All,
> Some additional findings on the OCC FIRDATA collection:
> Once the OCC has collected all the firdata, it iterates through the data 8 bytes at a time and adds a 9th ecc byte, Each time 256 bytes of ecc injected data has been generated, it calls whatever method that's "enabled" (HIO/IPMI) to write the chunk to PNOR.  Once the FIRDATA is exhausted, it writes all 0xffffffffffffffff + ecc to fill the remaining PNOR allocated to FIRDATA.
>
> I put stubs around the code that injects the ecc and then generated random FIRDATA to pass to it, and dumped what would have been written to PNOR.
> There is a bug in the code where if the original FIRDATA does not have a size that is a multiple of 8 bytes, it incorrectly processes the trailing data and then overwrites it, but I could not find any use case to cause it to generate incorrect ecc. This code has been around for a while and is used on mulitple platforms, but perhaps this is the first time the FIRDATA size has not been a muliple of 8 and I still don't know if this is the issue.

I assume ECC is correctly computed. Data can be corrupted during writing.
May be there are some problems in the SPI driver implementation inside
the OpenBMC (AST2400):
https://lists.ozlabs.org/pipermail/openbmc/2019-April/015953.html

Artem

> On Fri, May 24, 2019 at 10:10:52AM +0930, Andrew Jeffery wrote:
> > On Thu, 23 May 2019, at 18:58, Artem Senichev wrote:
> > > On Thu, May 23, 2019 at 10:10:38AM +0930, Andrew Jeffery wrote:
> > > > Hi Artem,
> > > >
> > > > Snipping somewhat, I'll let Doug dig into the specifics of the OCC issue as he
> > > > implemented the IPMI HIOMAP support for it.
> > > >
> > > > On Wed, 22 May 2019, at 17:13, Artem Senichev wrote:
> > > >
> > > > > We have HIOMAP support for our bundle (OpenBMC+OpenPOWER), it looks like
> > > > > the entire solution works fine except the OCC project, it is the last
> > > > > component that directly writes data to the PNOR flash.
> > > >
> > > > Great to hear the rest works for you. Just for your awareness you may need to
> > > > pick up a couple of patches depending on what firmwares you're running: [1]
> > > > to fix a data write corruption issue in skiboot and [2] to resolve a memory leak
> > > > in OpenBMC. These are the critical issues, but there are further sets of skiboot
> > > > and OpenBMC patches that I recommend to avoid lockups. I can point you to
> > > > them if necessary.
> > >
> > > Yes, please. May be these patches will solve my second problem :)
> >
> > Before I do, can you please either push your trees somewhere I can look over them?
> > That was I can minimise the set of patches I need to look over / recommend.
>
> The host-side logic is included to skiboot:
> https://github.com/open-power/skiboot/blob/76f7316bc8fc8a18fdbfcbc0e1fe1bb992d2a7d7/platforms/astbmc/vesnin.c#L298
> Actually, there are some changes in the message format (adaptation to the new
> IPMI API in OnepBMC), I will commit them as soon as I solve the sending problem.
>
> BMC-side handler of these IPMI messages:
> https://github.com/YADRO-KNS/phosphor-pci-inventory
>
> > > We have a (dead?) lock situation in skiboot. I'm not sure that
> > > this affects other skiboot's code, but I can't send PCI device list
> > > from skiboot to OpenBMC - the system freezes at ipmi_queue_msg_sync():
> > >
> > > https://github.com/open-power/skiboot/blob/76f7316bc8fc8a18fdbfcbc0e1fe1bb992d2a7d7/core/ipmi.c#L177
> > >
> > > Usually, system hangs after sending 3-5 of my messages. I have added
> > > trace to core/ipmi.c - it ends up with two calls to 'lock(&sync_lock)',
> > > one for my message and another one for HIOMAP.
> > > As workaround, I use asynchronous sending with increased IPMI message queue
> > > size, but it's not a good solution.
> >
> > Generally you shouldn't be using ipmi_queue_msg_sync(), and I'm unclear why
> > async is a bad solution here. What are your requirements that drive the use of
> > _sync()?
>
> I send one IPMI message for each end point PCI device. Device iteration is a
> fast operation, but IPMI queue is too small (10 records). As a result, the
> IPMI queue is quickly filled and we get lost messages.
> To avoid this I use synchronous sending. Or, as I
> mentioned, I can increase the IPMI queue and send the data
> asynchronously.
>
> > I don't have any immediate solutions for you. How are you sending down the PCI
> > data? Are you writing it to the PNOR or sending the PCI devices through as FRUs?
>
> I send some device information (location, vendor id, etc) via IPMI,
> it's OEM messages, not a FRU. But in the OpenBMC inventory devices look
> like FRU records.
>
> > > > On the skiboot side, master contains all the fixes we have needed so far, and
> > > > everything except for [3] is contained in the v6.3 tag. OpenBMC master has
> > > > all its respective fixes included.
> > > >
> > > > Bit of a tangent, but I hope it helps.
> > > >
> > > > Andrew
> > > >
> > > > [1] https://github.com/open-power/skiboot/commit/7f291166283f
> > > > [2] https://github.com/openbmc/hiomapd/commit/fac3689e77d3
> > > > [3] https://github.com/open-power/skiboot/commit/f01cd777adb1
> > >
> > > We already have these patches in our build, thanks.
> >
> > Out of interest, do you have this patch included?
> >
> > https://github.com/openbmc/btbridge/commit/aa5511d28ff9acee4a404c6397d09f5187812ed8
> >
> > That might eliminate some other spurious and intermittent problems.
>
> Yes, we have this patch.
>
> > Again it would be great if you can point me to your trees so I can check for
> > myself whether you've included certain patches.
>
> Unfortunately, we still use an internal git repository for OpenBMC builds.
> It based on upstream's master 2.7.0-dev-697-g5b03cfc66 (May 17).
>
> For OpenPOWER firmware we have a GitHub repo:
> https://github.com/YADRO-KNS/op-build
> But it's just an our mirror of open-power/op-build with several
> temporary workarounds.